fix: support the pointer tag OLX parsing of XBlocks residing outside the edx-platform #37133
Conversation
|
Thanks for the pull request, @ttqureshi! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
openedx-webhooks
added
the
open-source-contribution
ttqureshi
force-pushed
the
ttqureshi/pointer-tag
branch
from
8b24406 to
e587f30
Compare
ttqureshi
changed the title
ttqureshi
force-pushed
the
ttqureshi/pointer-tag
branch
from
e587f30 to
a7237b3
Compare
|
Hey @ttqureshi , do you want feedback on this or are you still working at it? |
|
@kdmccormick |
ttqureshi
force-pushed
the
ttqureshi/pointer-tag
branch
2 times, most recently
from
a7237b3 to
49dd8c0
Compare
There was a problem hiding this comment.
Yes, I think this is the right direction!
I think one of the causes of the test failures might be that you're passing in None for def_id. def_id is the slug which (along with the block type) uniquely identifies the block within the course, so this code might be trying to give all the pointed-to blocks the same slug (None). I am curious how this works with import/export in your manual testing... perhaps all the XML you are manual testing with redundantly defines url_slug in the pointed-to file, whereas the unit test XML doesn't?
I recommend taking a look at the other parts of the platform which call is_pointer_tag. Those calls might shed light on how to correctly load a block definition from a pointed-to file.
Once you've gotten that working, I think we should add this same logic to all the edx-platform blocks with children (ItemBankBlock, ConditionalBlock, RandomizeBlock, SplitTestBlock) and then remove the logic from XmlMixin and VideoBlock. That way, pointer-tag handling would always be handled by the parent instead of by the pointed-to block itself.
|
Thanks @kdmccormick for sharing your thoughts. Couple of good points to pay attention to. |
|
Update: I will update further if there's any update. If you've any thoughts on it please share. cc: @farhan |
|
There's one weird behavior which I've noticed. I made two similar courses one with multiple |
|
@ttqureshi Were you using master or this branch?
Can you expand on what you mean by this? It'll be hard for me to know for sure what's wrong without looking at the export. Would you be able to send along the .tar.gz? |
I was using this branch. LTI block is parsed and initialized correctly but HTML doesn't.
On studio, I created HTML blocks like this: ACTUAL
But when I imported the course after exporting, the HTML blocks were initialized like this: AFTER COURSE IMPORT
|
|
I had a discussion with @irtazaakram regarding this issue. I’m investigating it and will update here if I find any clues. |
|
That is strange, and it seems like a bug. Could you send the .tar.gz? EDIT: I misread your message, I thought you had said that this happened on master. |
|
sure I'm attaching course_with_multiple_html_blocks.tar.gz |
ttqureshi
force-pushed
the
ttqureshi/pointer-tag
branch
from
6c07c3b to
edb3d6d
Compare
|
@kdmccormick with this change, the course import issue with HTML blocks is resolved, HTML blocks are now importing correctly. I’m still investigating another issue. My current suspicion is that when a course contains draft blocks, some blocks are not being initialized as expected. |
|
@ttqureshi , do you understand why that works, and why the issue is only for HTML blocks? We cannot merge code that handles the HTML block as a special case like that. |
|
@kdmccormick So, setting the |
ttqureshi
force-pushed
the
ttqureshi/pointer-tag
branch
2 times, most recently
from
dcde83f to
bc3d13a
Compare
farhan
left a comment
farhan
left a comment
There was a problem hiding this comment.
PR looks good to me.
Kudos for the great effort in analyzing and fixing these complex issues!
🌟 🌟 🌟
|
Sandbox deployment successful 🚀 |
|
@kdmccormick |
|
@ttqureshi Before merging of the PR we need to do an announcement in the risky-changes channel |
|
Sandbox deployment successful 🚀 |
xmodule/modulestore/xml.py
Outdated
| try: | ||
| xml_data = etree.fromstring(xml) | ||
| make_name_unique(xml_data) | ||
| if not kwargs.get('def_loaded', False): |
There was a problem hiding this comment.
Please make this an actual keyword argument in the method definition
xmodule/item_bank_block.py
Outdated
| def_id = system.id_generator.create_definition(child.tag, child.get('url_name')) | ||
| child, _ = cls.load_definition_xml(child, system, def_id) | ||
|
|
||
| # Exported ItemBank blocks may have some "null" attributes as per the current |
There was a problem hiding this comment.
Is this existing ItemBank block behavior, or does this PR introduce the null attributes?
There was a problem hiding this comment.
This is the existing ItemBank block behavior. This PR is not modifying any OLX export functionality.
the example XML in the comment is taken form the course export on master branch.
There was a problem hiding this comment.
how were the "null" attributes being handled before this PR @ttqureshi ?
There was a problem hiding this comment.
@kdmccormick
In the case of BuiltIn XBlocks, parse_xml method from the XmlMixin is called. During the function call, the metadata attributes are deserialized like this:
try:
deserialized = json.loads(value)
if deserialized is None:
return deserialized
...
So in the case when the value is null, json.loads returns a None python object.
parse_xml() --> load_metadata() --> deserialize_field()
Now for the extracted XBlocks, XBlock.core's parse_xml method is called which parses the metadata attributes in a different way like this:
value = (block.fields[name]).from_string(value)
When the value is null, it doesn't get deserialized to None python object like in the Builtin case, instead it remains "null" string object.
parse_xml() --> _set_field_if_present
That is why we get "null" rendered in the XBlock.
There was a problem hiding this comment.
Ah, weird! Thanks for the explanation. @ttqureshi , does this issue also apply to other extracted block types, and to blocks which are not children of itembank?
For example, when we flip the USE_EXTRACTED flags on, will there be "null" attributes which need to be handled for:
- a Problem which is a child of an ItemBank ?
- a WordCloud which is a child of a Vertical ?
- a Problem which is a child of a Vertical ?
The reason I ask is that: if this is a general issue, then we shouldn't we handle it for all extracted block types, not just for ItemBank children?
There was a problem hiding this comment.
No problem.
I have an example of <conditional> working with pointer tags in my personal test course:
- XML file: https://github.com/kdmccormick/openedx-test-course/blob/master/test-course/course/conditional/5c70268dca294eb28701330f1c74f140.xml
- Full tar.gz course download: https://github.com/kdmccormick/openedx-test-course/tree/master/dist
I could not get the legacy <randomize> block working correctly. Don't worry about this one, I'll write a DEPR ticket for it.
As per my testing on the sandbox, the Randomized block from the Demo Course is being imported correctly in the Sandbox.
Glad to hear <library_content> works, thanks!
There was a problem hiding this comment.
TESTING NOTES FOR THE RANDOMIZED CONTENT BLOCK (<library_content>)
@kdmccormick I should have added these testing notes in the previous comment but adding it here for future reference
Why the Randomized Content Block is being parsed correctly?
In order to verify that the Randomized Content Block (library_content) is being parsed correctly during course import and specifically that the ItemBank’s definition_from_xml method is called for Randomized Content Blocks, I have done the following exercise:
I manually modified the demo course XML by replacing all <problem> tags with <lti> tags inside the library_content tag children.
I then created .tar.gz file, set the flag USE_EXTRACTED_LTI_BLOCK to True, and imported the course.
At this stage, the itembank pointer tag definition loading logic inside theItemBank.definition_from_xml was commented out in order to see how the LTI Blocks are initialized. In this case, the LTI Blocks inside the Randomized Content block weren't initialized correctly (met the expectation).
Next, I uncommented the pointer tag definition logic inside ItemBank.definition_from_xml and re-imported the same course. This time, all the LTI Blocks inside the Randomized Content Block were initialized correctly based on their XML definitions.
This confirmed that the Randomized Content Block is parsed properly during the import. This exercise confirmed that its behavior is not coincidental—initialization is happening exactly because definition_from_xml is being called from ItemBank as intended.
In case if you are wondering why I replaced the <problem> tags with <lti> tags. The reason is:
The extracted LTI Block does not override the parse_xml method. Therefore, in its case, the parse_xml method from xblock.core is invoked. In contrast, the extracted Problem Block does override parse_xml. By replacing the <problem> tags with <lti> tags, I made sure the pointer tag definition loading actually depended on the ItemBank’s pointer tag parsing logic.
If I had kept <problem> tags, the ProblemBlock’s own parse_xml method would have taken care of parsing the XML correctly, and the testing wouldn’t really prove anything about the Randomized Content Block.
Using <lti> tags ensured that only the xblock.core parse_xml from xblock was used, so the initialization of blocks inside Randomized Content could only work if the ItemBank’s pointer tag parsing was happening.
--
P.S.: Thanks for sharing the test course to test the conditional Block. On it's testing now!
There was a problem hiding this comment.
Your testing process sounds great, thank you!
There was a problem hiding this comment.
@ttqureshi , while you're testing <conditional>, I recommend falling back to the legacy studio Unit editor (with waffle legacy_studio.unit_editor). The new Unit editor has a bug where it doesn't let you add new children to Conditional or SplitTest, which would make testing harder: openedx/frontend-app-authoring#2499 EDIT fixed link: openedx/frontend-app-authoring#2497
There was a problem hiding this comment.
@kdmccormick I've added the logic to load pointer-tag definitions inside the ConditionalBlock's definition_from_xml method.
I have followed a similar testing approach as I did for the Randomized block. I kept the LTIs as the Conditional children, kept the relevant flag (USE_EXTRACTED_LTI_BLOCK) ON and then tested the import functionality. The changes are working fine.
I recommend falling back to the legacy studio Unit editor (with waffle legacy_studio.unit_editor)
Thanks for this, it made the testing easier, otherwise I would have to manually edit the XML files.
Now I believe the PR is up for the final review.
ttqureshi
force-pushed
the
ttqureshi/pointer-tag
branch
from
bc3d13a to
0ce2eb9
Compare
|
Sandbox deployment successful 🚀 |
|
@kdmccormick |
|
Sorry, I'm still working on local testing, and I had some issues with my dev env yesterday. I hope to be able to finish validation in the next couple days. |
ttqureshi
force-pushed
the
ttqureshi/pointer-tag
branch
from
0ce2eb9 to
e9ec6c7
Compare
|
Sandbox deployment successful 🚀 |
ttqureshi
force-pushed
the
ttqureshi/pointer-tag
branch
from
e9ec6c7 to
716753e
Compare
|
Sandbox deployment successful 🚀 |
ttqureshi
force-pushed
the
ttqureshi/pointer-tag
branch
from
716753e to
16b11c5
Compare
|
Sandbox deployment successful 🚀 |
|
Reverted by #37426 |
feat: removed spaces check for spam urls (openedx#37238) fix: remove CSRF_TRUSTED_ORIGINS_WITH_SCHEME variable (openedx#37195) fix: Passing unsaved model instances to related filters is no longer … (openedx#37227) * fix: Passing unsaved model instances to related filters is no longer allowed. fix: sidebar completion for completable XBlocks with children It is possible to create a completable XBlock with children. An example is the Library Content Block with the `MARK_LIBRARY_CONTENT_BLOCK_COMPLETE_ON_VIEW` feature toggle. The sidebar should use the same mechanism as the `BlockCompletionTransformer` and the `edx-completion` library. It means that we should treat: 1. An aggregator XBlock as completed only when all its children are completed. 2. A completable XBlock as completed when it is directly marked as completed (without checking the completion of its children). fix: show correct icons in the sidebar for units with custom XBlocks Currently, the sidebar relies only on the XBlock's `category` class attribute (called `type` in the transformers). This behavior is inconsistent with the legacy subsection navigation, which relies on the `XModuleMixin.get_icon_class` method. This commit adds the `icon_class` to the fields collected by the transformers and uses it to determine whether the "problem" or "video" icon should be displayed for a unit in the sidebar. feat: Use dropdown for units when more then 15 When dealing with subsections that have a lot of units, show a dropdown for unit selection to simplify navigation. feat: add a POST endpoint for listing courses (openedx#35586) * feat: add a POST endpoint for listing courses feat: reindex course and recreate upstream links post import and course re-run (openedx#37237) Index the newly created course after import and course re-run. Also recreate upstream links after course re-run. We make use of newly created COURSE_RERUN_COMPLETED signal to run post re-run processes. feat: Add the top-level parent logic to the ready_to_sync value in the UpstreamLink (openedx#37217) - Updates `UpstreamLink.ready_to_sync` with the top-level parent logic: In a container, `ready_to_sync` is `True` if the container or any children have changes. - Updates `decline_sync` to decline children recursively. fix!: session ID stability while maintaining inactivity timeout (openedx#36896) This change modifies the SessionInactivityTimeout middleware to prevent the session ID from changing on every request while still enforcing the inactivity timeout. Key improvements: - Store datetime values as ISO strings for proper serialization - Implement hybrid session save approach that only allows full session saves periodically (controlled by SESSION_SAVE_FREQUENCY_SECONDS) - Preserve session ID between requests while still tracking user activity This resolves the issue where lms_sessionid was changing on every user interaction, which caused problems. BREAKING CHANGE: The breaking change is that SESSION_ACTIVITY_SAVE_DELAY_SECONDS was introduced with a 15 minute default, which will change the current behavior. It is not necessarily breaking (since it actually fixes an issue), but this is to bring more attention to the new setting because the default is reasonable, but also somewhat arbitrary. feat: Add DOI-specific headers for link validation (openedx#37246) feat: Upgrade Python dependency edx-enterprise (openedx#37261) fix: bump edx-enterprise version to bump django-simple-history Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` Co-authored-by: UsamaSadiq <41958659+UsamaSadiq@users.noreply.github.com> chore: Upgrade Python requirements (openedx#37229) chore: Upgrade Python requirements (openedx#37263) chore!: remove Proctortrack dependency Remove all Proctortrack dependencies from the project, including: - Python package dependencies in requirements files - JavaScript package dependency in package.json - Translation configuration in conf/locale/config.yaml BREAKING CHANGE: Proctortrack proctoring provider is no longer available as a dependency. This change removes the Proctortrack Dependency while preserving code references. feat: Update the package lock chore: Updating the requirements fix: Discussion xblock error when loading for the first time This PR fixes the user not being created when accessing the discussion xblock as the course_key was missing in some of the api calls and that lead to the forum backend not being fetched correctly. feat!: Drop import_from_modulestore app (1/3 -- code only) (openedx#37239) Part of: openedx#37242 feat: Allow adding custom LTI parameters via LTI_CUSTOM_PARAMS django setting Adds a new Django setting called `LTI_CUSTOM_PARAMS` that allows extending the list of optional LTI parameters processed by the platform. These parameters can be used by plugins for deeper platform integration. fix: intermittent "utils attribute not found" issue in webpack_loader (openedx#37109) chore: Upgrade Python requirements feat: Remove the `SEND_CATALOG_INFO_SIGNAL` This setting controls an additive feature to send a signal on catalog changes. The signal is documented as a part of the list of supported events in our openedx-events reference. It has also been running live in the edx.org deployment for some time. This change removes the signal and defaults to the behavior as if it is net to true. OPERATORS NOTE: If you override the `SEND_CATALOG_INFO_SIGNAL` in your settings overrides for the edx-platform, you can remove that override. This signal will always fire on catalog changes now. The performance impact of this change should be negligible. chore: Updating the requirements2 fix: django52 tightened consistency around delete ordering and signal/delete() method invocation. (openedx#37250) fix: added thread id in comment/response reported event (openedx#37276) Co-authored-by: Ayesha Waris <ayesha.waris@A006-01000.local> feat: added rate limit on one click unsubscribe api (openedx#37272) * feat: added rate limit on one click unsubscribe api * fix: fixed failing test * chore: raise 400 error on invalid username * fix: fixed pylint chore: remove edx.notifications.preferences.viewed event (openedx#37285) fix: Updated tuple unpacking of signal receivers to match current structure in django52. (openedx#37249) fix: ensure _get_user returns saved user (with id) (openedx#37290) Django 5.2 tightened model handling: unsaved User objects (no primary key) no longer pass through in some checks. feat: implement course access role history table (openedx#37260) Co-authored-by: Muhammad Faraz Maqsood <faraz.maqsood@A006-01130.local> chore: Upgrade Python requirements (openedx#37292) fix: fix timezone for Django 5.2 (openedx#37293) fix: fix test_html_tags_removal (openedx#37209) chore: Clean up comments in github.in Remove comment about django42 support pending release. feat!: Drop import_from_modulestore app (2/3 -- models) (openedx#37240) Part of: openedx#37242 fix: fix script tag quot escaped (openedx#37296) feat: Add `unlinkable` to xblock actions and update `top_level_parent_key` on unlink [FC-0097] (openedx#37215) - Adds the `unlinkable` action to the XBlock object sent to the frontend - Updates the `top_level_parent_key` reference when unlinking containers. If you unlink a Section with Subsections and Units, this updates the `top_level_parent_key` for the Subsections to `None` (they are the top level now), and the `top_level_parent_key` for the Units to the corresponding parent Subsection. fix: exception while trying to check sync status of deleted upstream (openedx#37298) Use `Upstream.try_get_for_block` instead of `Upstream.get_for_block` which raises `BadUpstream` if upstream block is deleted. test: Disable the Django Debug Toolbar by default. Disable the toolbar by default but make it easy to turn back on as needed. This is in response to the discussion here: https://discuss.openedx.org/t/lets-remove-django-debug-toolbar/16847 fix: added info in notification preferences (openedx#37295) feat: Added pluggable override on financial_assistance (openedx#37303) chore: Upgrade Python requirements chore: removing ENABLE_V2_CERT_DISPLAY_SETTINGS (openedx#37302) the remnants of the logic that used this setting has been gone for a long time. This removes the toggle. FIXES: APER-1405 feat: Add support for using LTI data to populate user profile Currently the LTI provider implementation auto-creates a random user when logging in, however, the LTI launch can include relevant user details such as their email, full name and even a username. This change makes the LTI code use the provided details if the "Use lti pii" setting is set in the Django admin. chore: upgrade social-auth-core chore: Management command to update preferences (openedx#37297) feat: tpa automatic logout with a single redirect feat: enhance Course Optimizer to update previous course links via API (openedx#37206) * feat: API to update previous-run course links * feat: handle edge cases and update tests for prev-run links API chore: Upgrade Python requirements feat!: Drop import_from_modulestore app (3/3 -- uninstall app) (openedx#37241) openedx#37242 feat: Upgrade Python dependency edx-enterprise fix: [edx-enterprise] upgrade djangorestframework and skip automatic validators Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` chore: geoip2: update maxmind geolite country database feat: add custom attributes to moniter export course task (openedx#37314) fix: Update SAMLProviderConfig for site-specific configurations (openedx#37294) Fixes minor bugs in new SAMLProviderConfig signal handlers. chore: Update openedx-forum version (openedx#37320) feat: added api for mobile configs (openedx#37323) feat: extend courseware api with new fields refactor: add some fields to serailizer for FE & update API docstring refactor: fix prerequisites source refactor: add missing docstrings, rename * remove some fields fix: remove redundant test which breaks everything docs: update docstring for prerequisite course serializer docs: update docstring for course info serializer refactor: move import inside testcase refactor: fix unused imports docs: add comment for skip_unless_lms fix: remove unused fields 'prerequisites', 'ocw_links' chore: remove deprecated DEFAULT_FILE_STORAGE and STATICFILES_STORAGE settings (openedx#37002) Revert "feat: added api for mobile configs (openedx#37323)" (openedx#37332) This reverts commit 3d46524. fix: Invalid read param in forum thread api This PR fixes the invalid read parameter being passed in forum update_thread api. this was somehow introduced in this commit openedx@6f522f3#diff-b9d08209a02c673887f6ca72b0018758174e95508539adbcb6885f22c7844eb8L292 fix: unlink broken link (openedx#37329) This fixes a bug that occurs when unlinking a block from a downstream block that has a broken link fix: program details dashboard (openedx#37203) Co-authored-by: Deborah Kaplan <deborahgu@users.noreply.github.com> fix: fix bad serialization of courseware metadata (openedx#37338) * Formats single_paid_mode as a dict, rather than named tuple which couldn't be serialized to DictField * Allow ecommerce_checkout_link to be empty Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> fix: [FC-0092] fix 500 when return_type is list (openedx#36969) There was problem in filter_discussion_xblocks_from_response(). This function was breaking the list response for the BlocksInCourseView by returning a dict instead of list. chore: Upgrade Python requirements build: Upgrade to Node 24 (openedx#37166) Upgrade node and re-build package-lock.json feat: Upgrade Python dependency edx-enterprise (openedx#37346) New version adds a missed migration for Django 5.2 support Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` Co-authored-by: UsamaSadiq <41958659+UsamaSadiq@users.noreply.github.com> chore!: removing of pyjwkest dependency from edx-platform (openedx#37159) feat: automatically follow post when creating comments/responses (openedx#37318) * feat: automatically follow post while createing comments/responses * feat: follow post when a comment or response is created * test: updated tests --------- Co-authored-by: Ayesha Waris <ayesha.waris@192.168.10.28> Co-authored-by: Muhammad Adeel Tajamul <muhammadadeeltajamul@hotmail.com> feat: added configurable param to rate limit email change in account settings (openedx#37349) chore: Upgrade to django-pipeline to its latest. It has django52 support. (openedx#37355) feat: added feature to rate limit secondary email change (openedx#37356) feat!: remove cs_comments_service support for forum's content APIs - This will force the use of the new v2 forum's APIs for Threads & Comment. - Update params for get_user_subscription function. It uses the same structure as we have in the get_user_threads. feat: Upgrade Python dependency edx-enterprise gracefully decouples enterprise from ecommerce Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` feat: Upgrade Python dependency enterprise-integrated-channels Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` Fix: CORS issues in third-party auth disconnect by adding JSON endpoint (openedx#37100) Add a json auth endpoint where previously there was only an HTML redirect version. This will make it easier to work with MFEs. --------- Co-authored-by: Feanil Patel <feanil@axim.org> docs: Fix the docs build. With the settings re-organization the docs settings file was missing a setting needed to properly run django. This wasn't caught in CI because the docs build is slow enough that we don't run it on every PR. docs: Update the checked-in lms-openapi docs. These are checked-in because there are some downstream users that depend on them being checked-in. They are also used to generate sphinx docs based on the openapi spec. build: Parallize the docs build. There is no technical issue that prevents us from running this build in parallel as far as I can tell. We can see if this will sufficiently speed up the build for us to turn it on for PRs. test: running tests with django52. (openedx#37357) feat: Upgrade Python dependency edx-enterprise (openedx#37360) fix: Moodle configuration updates were not being saved correctly Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` Co-authored-by: zamanafzal <11922730+zamanafzal@users.noreply.github.com> Co-authored-by: Zaman Afzal <zamanafzal@gmail.com> chore: Upgrade Python requirements feat: allow editing imported text blocks (openedx#37124) * feat: allow editing html block imported from upstream The modified field is left untouched in future sync while storing the upstream values in hidden fields to allow authors to revert to upstream version at any point. * fix: sync downstream_customized field for copy-pasted modified block * test: add more tests * fix: lint issues * test: copy paste * feat: skip sync if html data is modified * feat: update upstream fields only when modified * refactor: use version_synced field to skip sync * feat: edit title inplace for library source components * fixup! feat: edit title inplace for library source components * fix: edit title button style * fix: test case * fix: lint issue * refactor: don't show different icon for modified upstream blocks * Revert "refactor: use version_synced field to skip sync" This reverts commit 8b784ff. * feat: only skip sync for modified blocks if updated as part of container * refactor: update sync behaviour when synced individually and as part of parent * feat: include ready to sync children info in downstream link get api * test: fix failing tests * fix: lint issues * feat: new tests and update api to allow overriding modified fields in sync * test: api changes * refactor: edit options should be visible for individual imports * docs: update api docs * chore: remove old comments chore: update @edx/brand dependency to new package @openedx/brand-openedx (openedx#37244) feat: update saml management command (openedx#37330) The SAML management command has been refactored from an auto-update tool to a comprehensive report-only audit system. The changes introduce a new --run-checks option that provides detailed reporting on SAML configuration issues without making any automatic changes. feat: add ability to override middlewares for recurring nudges * feat: add ability to override middlewares for recurring nudges * feat: add ability to run command for all sites fix: styles for share video functionality of the video xblock fix: visual and progress score display enhancement fix: add missing styles build: Unpin pip and pip-tools. Test with unpinning both pip and pip-tools. The latest versions have fixed the pathing issues with `make upgrade` and the dependency resolution issues reported in openedx/edx-lint#458 chore: Run `make upgrade` refactor: flatten FEATURES dictionary with backward compatible proxy feat: [AXM-2398] Add short label to assignment (openedx#36970) Rationale: The instructor may create short labels that are longer than 3 characters, and they can be hard to work with in the mobile UI. Thus, on mobile, it was decided to add short labels to the api response by getting them from section breakdown, which ensures they are consistent with the labels the user sees in the Grading section. fix(deps): update dependency webpack-bundle-tracker to v3 fix: Upgrade django-webpack-loader to match webpack-bundle-tracker The versions of these two tools is now kept in sync. As a part of the migration, the method signature for render_bundle changes which we needed to update in our Mako templates where it doesn't automatically pass context. test: Fix test monkeypatches for render_bundle Now that the signature has changed, update the testing monkeypatches as well. fix: Use CI oriented FakeWebpackLoader This was added in a newer version of django-webpack loader to be used in CI. Previously we had some monkey patches to deal with this but they could break when changes happen to the upstream django-webpack-loader library. This should be a more robust solution to keep CI working and to reduce complexity on future upgrades. docs: Document what MFEs are on by default Document which MFEs are currently on by default in the edx-platform code and where the code expects those MFEs to be running including the settings that can be overridden to change those default locations. Consolidate common settings to platform wide settings module (openedx#37337) * refactor: lift same settings up to openedx common module feat!: Remove Proctortrack dependency (openedx#37284) * chore!: remove Proctortrack dependency Remove all Proctortrack dependencies from the project, including: - Python package dependencies in requirements files - JavaScript package dependency in package.json - Translation configuration in conf/locale/config.yaml BREAKING CHANGE: Proctortrack proctoring provider is no longer available as a dependency. This change removes the Proctortrack Dependency while preserving code references. * feat: Update the package lock * chore: Updating the requirements * chore: Updating the requirements2 * chore: Clean up comments in github.in Remove comment about django42 support pending release. feat: Upgrade Python dependency edx-enterprise (openedx#37372) * feat: Upgrade Python dependency edx-enterprise Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` * fix: typo fix to trigger tests --------- Co-authored-by: kiram15 <31229189+kiram15@users.noreply.github.com> Co-authored-by: Kira Miller <kira.miller15@yahoo.com> chore: remove outdated JWT auth related settings fix(deps): update dependency webpack-merge to v6 fix: Use the default merge for webpack config. The smart merge feature has been dropped from the webpack-merge library that we were using. Use the basic merge feature instead since we're not actually doing anything too complicated. Also don't return null from the WorkerConfig as it can't be correctly merged. fix: Inconsistent update state in Course Optimizer API (openedx#37369) * fix: Inconsistent update state in Course Optimizer API * fix: update re-run link issue docs: add DEPR ticket for some mfe_config settings (openedx#37370) refactor: course container children api [FC-0097] (openedx#37375) * feat: course container children view * refactor: rename * refactor: include children info in upstream info of container children * fix: tests * fix: test * refactor: children check chore: remove unused mockprock devDependency feat!: modulestore_migrator (openedx#36873) This introduces the modulestore_migrator app, which can be used to copy content (courses and libraries) from modulestore into Learning Core. It is currently aimed to work on the legacy library -> v2 library migration, but it will be used in the future for course->library and course->course migrations. This includes an initial REST API, Django admin interface, and Python API. Closes: openedx#37211 Requires some follow-up work before this is production-ready: openedx#37259 Co-authored-by: Andrii <andrii.hantkovskyi@raccoongang.com> Co-authored-by: Maksim Sokolskiy <maksim.sokolskiy@raccoongang.com> feat: updates legacy libraries list API to include migration info [FC-0097] (openedx#37286) Adds migration info like `migrated_to_title`, `migrated_to_key` and `is_migrated` fields indicating whether the legacy library was migrated to library v2. If yes, it includes the new library name and key. Users can also filter by migration status using `is_migrated` query param. fix: update Course Highlights help URL (openedx#37388) build(deps): bump codecov/codecov-action from 4 to 5 Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 4 to 5. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](codecov/codecov-action@v4...v5) --- updated-dependencies: - dependency-name: codecov/codecov-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> build(deps): bump actions/checkout from 4 to 5 Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 5. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v4...v5) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> build(deps): bump actions/download-artifact from 4 to 5 Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 4 to 5. - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@v4...v5) --- updated-dependencies: - dependency-name: actions/download-artifact dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> chore(deps): bump peter-evans/create-pull-request from 6 to 7 Bumps [peter-evans/create-pull-request](https://github.com/peter-evans/create-pull-request) from 6 to 7. - [Release notes](https://github.com/peter-evans/create-pull-request/releases) - [Commits](peter-evans/create-pull-request@v6...v7) --- updated-dependencies: - dependency-name: peter-evans/create-pull-request dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> fix: always generate block structure from "published-only" branch (openedx#37335) Block structures are meant to be an optimization for the LMS, meaning that they should always be collecting from the published branch of modulestore. This is what happens by default when it's run from the LMS celery process, but this code is sometimes invoked from a Studio worker (e.g. development mode celery, running in immediate in-proc mode). --------- Co-authored-by: Peter Pinch <pdpinch@mit.edu> docs: ADR for Visually Configuring Course Components (openedx#36856) --------- Co-authored-by: Maksim Sokolskiy <maksim.sokolskiy@raccoongang.com> build: use ubuntu-lts-latest in readthedocs build (openedx#37392) feat: Upgrade Python dependency edx-enterprise (openedx#37390) Upgrade python requirements Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` Co-authored-by: brobro10000 <82611798+brobro10000@users.noreply.github.com> Co-authored-by: Hamzah Ullah <hamzahullah@yahoo.com> feat: update course_about & catalog link generation refactor: switch to global mfe state check chore: updated settings for mfes fix: fix gettings ENABLE_CATALOG_MICROFRONTEND FEATURE fix: append /catalog to CATALOG_MICROFRONTEND_URL test: fix ddt in test_link_with_new_catalog_page fix: put CATALOG_MICROFRONTEND_URL with the other mfe urls fix: look up ENABLE_CATALOG_MICROFRONTEND in settings directly, adjust tests refactor: move filter out of if, extract about_base_url test: fix ENABLE_CATALOG_MICROFRONTEND look up in tests fix: use getattr in use_catalog_mfe test: fix test overrides feat: Add `blockType` to xblockPreview & title to xblock_iframe [FC-0097] (openedx#37362) - Adds `blockType` and `is_modified` to the `showXBlockLibraryChangesPreview` iframe message. - Add title to the `xblock_iframe` - Add `is-modified` to `studio_xblock_wrapper` - Add `disable_staff_debug_info` as a query param in `render_xblock` - `downstream_is_modified` added to ComponentLink and ContainerLink feat: add redirects to mfe for catalog-related legacy pages refactor: switch to global mfe state check refactor: use get_link_for_about_page, remove duplicate function test: use ENABLE_CATALOG_MICROFRONTEND from settings feat: extend program dash endpoint to b2c (openedx#37387) chore: geoip2: update maxmind geolite country database chore: Upgrade Django to 4.2.25 to pick up security patch (openedx#37418) This is from running `make upgrade-package package=Django` feat: adds new fork migration strategy (openedx#37408) Implements the `fork` strategy, allowing the user to create new copies while migrating courses/legacy libraries to v2 libraries. fix: reindex_studio could crash if an XBlock child isn't loadable (openedx#37177) Co-authored-by: Chris Chávez <xnpiochv@gmail.com> fix: survey questions options alignment in mobile view fix: Fix the docs build. The docs settings file needed an update for a new djangoapp that was added. Long term we need to probably think more deeply about how we want the LMS/CMS docs builds to work. Do we want to separate them, or make it easier to have a django settings file that will work with both. For now, we have a single settings file that is referenced for the docs build so it needs to have all the installed apps from both. feat: add unified certificate task API with toggle, generate, and regenerate support fix: support parsing the pointer-tag OLX for external XBlocks (openedx#37133) Previously, the built-in XBlocks (problem, video, etc) could be parsed from pointer tag syntax, but externally-defined XBlocks (drag-and-drop, ORA, etc.) could only be parsed from inline syntax. This is because the built-in blocks have special parsing logic, defined in XmlMixin, which is not available to external blocks. This PR shifts the pointer tag parsing "up a level" such that the parent blocks parse the pointer tag, regardless of whether the child is built-in or external: * vertical (aka unit) * split_test (aka content experiment) * itembank (aka problem bank) * library_content (aka randomized legacy library) The following parent blocks still lack support for external pointer-tag children; we will fix this in a follow-up PR: * randomize * all externally defined container blocks Part of: openedx/XBlock#823 revert: fix: support parsing the pointer-tag OLX for external XBlocks (openedx#37426) This reverts commit d382721, which seems to break edx-platform's ability to load the content of externally- defined xblocks (e.g. lti_consumer) from course exports. fix: make ALLOWED_HOSTS configurable through YAML refactor: move ALLOWED_HOSTS to openedx/envs/common fix: add '*' wild card in common ALLOWED_HOSTS refactor: Move user_util library to edx-platform The library consisted of this set of utilities and a cli and was only being used in the edx-platform repo. The CLI will be DEPRed along with the repo but the code that is being used for retirement will be moved here. build: Drop the user-util dependency. chore: Run `make upgrade` build: Drop unused docker compose and sql files. fix: Only update downstream_customized for upstream-linked blocks (openedx#37412) We only need to track field customizations for upstream-linked (i.e., library-linked) blocks. Thd downstream_customized field is irrelevant for other blocks. It would just add a ton of noise to the OLX. Additionally, we now clear downstream_customized when severing an upstream link. Fixes: openedx#37411 fix: fix fork on multiple migrations (openedx#37422) Fixes a bug where, when running a migration using the fork strategy, it was only looking at the last migration, resulting in a slug reuse, which would cause a component update instead of a new component creation. fix: add default to `target_collection_slug` (openedx#37391) adds a default `None` value to the `target_collection_slug` parameter of the migration rest API endpoint, to prevent a `KeyError`. feat: Implementation of library v2 backup endpoints Fix upgrade job pin cryptography (openedx#37436) * fix: pin cryptography to fix the upgrade job * fix: pin pact-python<3.0.0 --------- Co-authored-by: edX requirements bot <49161187+edx-requirements-bot@users.noreply.github.com> feat: Upgrade Python dependency enterprise-integrated-channels Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` refactor: Update and migrate Video Block JS files into xmodule/assets - Move Video Block JS files from xmodule/js/src/video/ to xmodule/assets/video/public/js/ - Update JavaScript files from RequireJS to ES6 import/export - test: Enable and fix Karma Js tests for Video XBlock (openedx#37351) --------- Co-authored-by: salmannawaz <salman.nawaz@arbisoft.com> fix: Profile MFE redirection issue (URL path override) This error was occurring because the way the redirect URL was constructed caused the entire base path to be removed. This commit updates the URL construction method to correctly preserve the MFE's path. feat!: Upgrading to `django52`. chore: Run unit tests only pinned which is 5.2 Removed Django version '5.2' from the workflow. fix: add Djang<6.0 local constraint chore: python requirements upgrade refactor: include container display name in children api (openedx#37429) This helps us avoid additional API calls just to fetch display name of the container block that is being previewed. feat: Certificate sharing to linkedin (optionally) consider course level organization name (openedx#37331) By adjusting social media sharing settings(specifically linkedin) certificate parameters are autopopulated to LinkedIn API. Additional setting parameters(such as CERTIFICATE_LINKEDIN_DEFAULTS_TO_COURSE_ORGANIZATION_NAME) are introduced to override existing(platform level parameter for organization name) parameters for an operator to configure course level organization name. This will enable learners to share certificate in to LinkedIn with an option for course associated organization to be autopopulated. fix: prevent None entrance_exam_minimum_score_pct from breaking CourseOverview sync (openedx#37339) * fix: prevent None entrance_exam_minimum_score_pct from breaking CourseOverview sync When entrance exams are disabled in Studio, the field `entrance_exam_minimum_score_pct` was set to `None`. This caused silent failures when saving `CourseOverview` because the database column requires a float (NOT NULL). This patch ensures that: - CourseOverview sanitizes None values by falling back to `settings.ENTRANCE_EXAM_MIN_SCORE_PCT` (default=50). - Studio avoids writing `None` and instead applies the configured default. Impact: - Prevents IntegrityErrors and silent failures when updating course settings. - Restores proper syncing between modulestore (Mongo) and CourseOverview (MySQL). - Fixes reported issues such as display name changes not persisting and course start dates not syncing. Closes: openedx#37319 * refactor: clean up entrance_exam_minimum_score_pct handling - Consolidate logic to avoid repeated assignments - Centralize None fallback and int/float normalization - Improve readability with inline comment and consistency with Open edX style * test: update entrance exam deletion test to expect default min score - Adjusted `test_entrance_exam_created_updated_and_deleted_successfully` to check for `settings.ENTRANCE_EXAM_MIN_SCORE_PCT` instead of `None` after exam deletion - Added handling for both int and float defaults (`/100` for integer case) feat!: remove last calls to cs_comments_service (openedx#37376) This removes the last remaining code that called out to the cs_comments_service. All forums backend logic now uses the v2 API from the forum repo (https://github.com/openedx/forum). This does NOT remove MongoDB support. This also implements the endpoint to retrieve all comments for a user using the new forum backend. This is not actually called from any known frontend code, but it has not been formally deprecated as an endpoint, and therefore needs to be supported. As part of the cleanup, the ENABLE_FORUM_V2 course waffle flag has also been removed, along with all remaining switching logic that used to route between the Python API in the forum repo and service calls to the cs_comments_service Ruby service. Other endpoints affected (switching logic removed): * get course commentable counts * get/update course user stats * update comment/thread/user * delete thread (implementation moved to forum repo) * follow * retire user This is part of the following overall DEPR ticket: https://github.com/openedx/cs_comments_service/issues/437 feat: api for shifting all relative past due dates refactor: rename and refactor functions and view refactor: use path instead of re_path chore: upgrade jQuery UI to version 1.14.1 (openedx#37025) revert: chore: upgrade jQuery UI to version 1.14.1 (openedx#37025) (openedx#37449) This reverts commit e427078. feat: query_feature_names extended by custom fields, siteconfig name changed fix: pytlint issue docs: docstring updated docs: refine important notes in get_student_features_with_custom docstring fix: use parameterized logging in extract_attr refactor: use get_user_model() instead of refactor: replace setattr/delattr with direct assignment refactor: centralize get_badge_count in test class test: remove unnecessary badge_count setup in non-matching org test docs: add missing docstrings fix: over-intended fix test: similar test removed test: another similar test removed test: removed redundant test test: improve enrollment_date test reliability using direct lookup refactor!: use String field instead of Dict field to store top_level_downstream_parent_key (openedx#37448) * refactor!: use String field instead of Dict field to store top_level_downstream_parent_key Since this is a new field no production instance should have this field yet. Developers need to delete their old courses as this change will raise error in all course pages. * chore: add `top_level_parent` field in ComponentLink and ContainerLink admin * refactor: use ":" as separator * refactor: block key parsing and tests feat!: Remove Studio Maintenance & Announcements (openedx#37432) The announcements editor was never ported to frontend-app-authoring, and the announcements display was never ported to frontend-app-learner-dashboard. This announcements feature is rarely used, undocumented, non-a11y-friendly, and there were no volunteers to port it to the new frontends. It is the last remaining part of the legacy Studio "Maintenance" dashboard. So, we are removing it. BREAKING CHANGE: This removes... * Studio Maintenance dashboard legacy frontend * Studio Edit Announcements legacy frontend * The snippet of legacy learner dashboard which renders announcements * openedx/features/announcements djangoapp * The ENABLE_ANNOUNCEMENTS feature flag Not removed: * The announcements_announcement table from openedx/features/announcements . The table is most likely very small, as it is only populated by administrators. Removing it would be more labor for us and more risk of toil for operators than is worthwhile. Closes: openedx#36263 docs: ADR introducing mobile offline content support (openedx#35011) Co-authored-by: Іван Нєдєльніцев <ivan.niedielnitsev@raccoongang.com> Co-authored-by: Kyrylo Kholodenko <kyrylo.kholodenko@raccoongang.com> Co-authored-by: Glib Glugovskiy <glib.glugovskiy@raccoongang.com> feat: Upgrade Python dependency edx-enterprise feat: add an endpoint to create a customer admin user Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` chore: Move sharing_sites into video_configuration app - openedx#37456 feat: bulk modulestore migration [FC-0097] (openedx#37381) - Adds the task, python api, and rest api view for bulk migration. - Refactor the code to share code between single migration and bulk migration. fix: formated_name updated refactor: nested functions inside enrolled_students_features are now at the top of the module
6 participants
The Pointer tag OLX format of the XBlocks residing outside the edx-platform isn't supported.
Description
This issue was identified here. It was decided that we don't want to add the pointer tag support to the XBlock core parsing methods instead it should be handled by the edx-platform runtime.
Let me briefly describe what this issue is all about. The XBlocks that reside outside the edx-platform are exported in the inline OLX format while the BuiltIn XBlocks (WordCloud, Annotatable, LTI etc.) are exported in the pointer tag format.
Here are the examples of the inline VS the pointer tag OLX format:
Inline format
Pointer tag format
Here the
<lti/>tag is a pointer tag because it's definition (or you can say, configurations) isn't defined inline as above. The definition is present atlti/e73666f5807e47cbbd161d0d3aa5132b.xmlas follows:This issue was discovered during the LTI XBlock extraction when one of its test cases on edx-platform was failing. It was later identified that it's pointer tag format is parsed correctly by the edx-platform's XmlMixin
parse_xmlmethod and it doesn't get parsed correctly by the xblock.coreparse_xmlmethod.Approach
Following this comment of @bradenmacdonald
I am now loading the full definition of the Vertical block's children before it calls the
system.process_xml()inside thedefinition_from_xml()method which in turn calls thexblock_from_nodeand thenparse_xmlmethod is called . This way we have the full definition of the child block (LTI, for example) before calling theparse_xmlmethod. Followed the same approach for theItemBankBlock'sdefinition_from_xml()method.Here's the call stack captured during the course import.
Tutor requirements