Skip to content

[BD-04] Convert PollModule to PollBlock and CustomTagModule to CustomTagBlock#26873

Merged
ormsbee merged 2 commits intoopenedx:masterfrom
open-craft:symbolist/convert-to-xblock
Mar 9, 2021
Merged

[BD-04] Convert PollModule to PollBlock and CustomTagModule to CustomTagBlock#26873
ormsbee merged 2 commits intoopenedx:masterfrom
open-craft:symbolist/convert-to-xblock

Conversation

@symbolist
Copy link
Contributor

@symbolist symbolist commented Mar 5, 2021 •
edited
Loading

Description

Converts the Poll XModule and CustomTagXModule into XBlocks.

Part of XModule to XBlock Conversion work.

  • Which edX user roles will this change impact? "Developer"

Testing Instructions

  • Checkout the master branch.
  • Create a course and import this archive: course-xmodules.tar.gz
  • The course has an example Poll XBlock and an example CustomTag XBlock. These XBlocks cannot be created in Studio so we need to create them in XML and import.
  • View and interact with both of them in Studio and LMS.
  • Checkout this branch and run pip install -e common/lib/xmodule/ and NO_PREREQ_INSTALL=1 paver update_assets and then make lms-restart + make studio-restart.
  • View and interact with both of them in Studio and LMS. They should look and behave the same (see screenshots below).
  • Export the course and check that the OLX of Poll and CustomTag is the same as the OLX in the original archive.
  • Import the exported course and check that the two XBlocks are still a part of the course.

Screenshots

All the views should look and work the same before and after the conversion.

CustomTag author view

image

CustomTag studio view

image

CustomTag student view

image

Poll author view

image

Poll studio view (It does not exist so we get this error)

image

Poll student view

image

@openedx-webhooks openedx-webhooks added open-source-contribution PR author is not from Axim or 2U waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Mar 5, 2021
@openedx-webhooks
Copy link

openedx-webhooks commented Mar 5, 2021 •
edited
Loading

Thanks for the pull request, @symbolist! I've created BLENDED-786 to keep track of it in Jira. More details are on the BD-04 project page.

When this pull request is ready, tag your edX technical lead.

@symbolist symbolist force-pushed the symbolist/convert-to-xblock branch from 1529251 to 856e58a Compare March 5, 2021 16:17
@natabene
Copy link
Contributor

natabene commented Mar 5, 2021

@symbolist Thank you for your contribution. Please let me know once it is ready for our review.

@symbolist symbolist changed the title Convert PollModule to PollBlock and CustomTagModule to CustomTagBlock [BD-04] Convert PollModule to PollBlock and CustomTagModule to CustomTagBlock Mar 5, 2021
@openedx-webhooks openedx-webhooks added blended PR is managed through 2U's blended developmnt program and removed open-source-contribution PR author is not from Axim or 2U labels Mar 5, 2021
symbolist
symbolist commented Mar 5, 2021
View reviewed changes
common/lib/xmodule/xmodule/poll_module.py Outdated
Copy link
Contributor Author

@symbolist symbolist Mar 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be possible to fix this but I am assuming it is better to leave deprecated code broken.

common/lib/xmodule/xmodule/template_module.py Outdated
Copy link
Contributor Author

@symbolist symbolist Mar 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copied over from XMLEditingDescriptor.

common/lib/xmodule/xmodule/tests/__init__.py Outdated
Copy link
Contributor Author

@symbolist symbolist Mar 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class is no longer being used.

common/lib/xmodule/xmodule/tests/test_poll.py Outdated
Copy link
Contributor Author

@symbolist symbolist Mar 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two methods have been moved over with changes for XBlocks (instead of XModules) from LogicTest.

@symbolist
Copy link
Contributor Author

symbolist commented Mar 5, 2021

@natabene Thanks! This is part of BD-04 so @kdmccormick or @ormsbee will be doing the review.

@symbolist
Copy link
Contributor Author

symbolist commented Mar 5, 2021

@s0b0lev I do not think the failures are related to the changes in this PR. So hopefully they will go away once I do the final rebase.

@symbolist symbolist force-pushed the symbolist/convert-to-xblock branch from 856e58a to 357951e Compare March 8, 2021 11:13
@symbolist symbolist marked this pull request as ready for review March 8, 2021 11:47
@openedx-webhooks openedx-webhooks added needs triage and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Mar 8, 2021
@edx-status-bot
Copy link

edx-status-bot commented Mar 8, 2021

Your PR has finished running tests. There were no failures.

s0b0lev
s0b0lev approved these changes Mar 8, 2021
View reviewed changes
Copy link
Contributor

@s0b0lev s0b0lev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@symbolist 👍

  • I tested this: follow testing instruction
  • I read through the code
  • Includes tests for bugfixes and/or features added.

@symbolist
Copy link
Contributor Author

symbolist commented Mar 8, 2021

@kdmccormick @ormsbee This is ready for a second round. 🙂

@ormsbee ormsbee merged commit e624a8f into openedx:master Mar 9, 2021
@edx-pipeline-bot
Copy link
Contributor

edx-pipeline-bot commented Mar 9, 2021

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

edx-pipeline-bot commented Mar 9, 2021

EdX Release Notice: This PR has been deployed to the production environment.

@edx-pipeline-bot
Copy link
Contributor

edx-pipeline-bot commented Mar 9, 2021

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

edx-pipeline-bot commented Mar 9, 2021

EdX Release Notice: This PR has been deployed to the production environment.

@Agrendalath Agrendalath mentioned this pull request Oct 27, 2022
Merged
@Agrendalath Agrendalath mentioned this pull request Aug 1, 2023
Closed
@kdmccormick kdmccormick mentioned this pull request Nov 12, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ormsbee ormsbee ormsbee approved these changes

+1 more reviewer

@s0b0lev s0b0lev s0b0lev approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

blended PR is managed through 2U's blended developmnt program merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@symbolist @openedx-webhooks @natabene @edx-status-bot @edx-pipeline-bot @ormsbee @s0b0lev

Comments