Allow for Extracted Word Cloud Block | Fix Test Cases by farhan · Pull Request #35983 · openedx/openedx-platform

farhan · 2024-12-07T11:34:21Z

In this PR:

We have added functionality to run Word Cloud XBlock test cases for Built-In and Extracted XBlocks.
We have fixed the test cases for Extracted Word Cloud XBlock.

~~In this PR we have enabled the usage of Extracted Word Cloud XBlock resides in xblocks-contrib repository.~~

feanil

I took a pass but since I'm not as familiar with the Xblock framework, forgive me if some of the questions have obvious answers. I'm in particular confused about why the tests need to change and if that indicates any sort of breakage that we need to handle elsewhere?

feanil · 2025-01-21T17:13:07Z

lms/djangoapps/courseware/tests/test_word_cloud.py


-        assert fragment.content == self.runtime.render_template('word_cloud.html', expected_context)
+        if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK:
+            expected_context['range_num_inputs'] = range(5)


Why not check the block type in this case?

Fixed, checking for both (Builtin & Extracted) XBlocks now.

feanil · 2025-01-21T17:14:55Z

xmodule/tests/test_word_cloud.py

+        if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK:
+            def_id = runtime.id_generator.create_definition(olx_element.tag, olx_element.get('url_name'))
+            keys = ScopeIds(None, olx_element.tag, def_id, runtime.id_generator.create_usage(def_id))
+            block = WordCloudBlock.parse_xml(olx_element, runtime, keys)


Why do keys need to be provided for one instantiation but not the other?

Update:
I have added keys for both builtin and extracted implementation to avoid If condition.

Why It's not needed in case of builtin xblock implementation?
In BuiltIn xblock implementation WordCloud block has XmlMixin and in its parse_xml method implementation, there is a check at start that if keys are None then it creates the keys.
I have copied the same lines of code and added them in test case.
In Extracted XBlock implementation parse_xml method of xblocks/core.py calls which doesn't have this check and expecting keys values in params.

Further discussion:
#35983 (comment)

feanil · 2025-01-21T17:27:50Z

xmodule/tests/test_word_cloud.py

-        # This will export the olx to a separate file.
-        block.add_xml_to_node(node)
+        if settings.USE_EXTRACTED_WORD_CLOUD_BLOCK:
+            filepath = 'word_cloud/block_id.xml'


Why not use the same method as below to export the file?

Because add_xml_to_node is defined in XmlMixin class.
BuiltIn XBlocks are using this mixin

In Extracted XBlocks we are preferring to use Xblock.core functionalities and using export_to_xml method which doesn't have export_to_file funcationlity like add_xml_to_node method of XmlMixin has

farhan · 2025-01-22T06:48:52Z

I took a pass but since I'm not as familiar with the Xblock framework, forgive me if some of the questions have obvious answers. I'm in particular confused about why the tests need to change and if that indicates any sort of breakage that we need to handle elsewhere?

@feanil Thanks for the pass.
Let me review my implementation, rethink on them, and address your rightful queries.

Let's keep this PR in draft until I make this PR and Extracted Word Cloud PR openedx/xblocks-contrib#4 fully ready.

xmodule/tests/test_word_cloud.py

farhan · 2025-01-28T13:24:22Z

@feanil You can consider this PR to be ready for next pass

open-craft-grove · 2025-01-30T13:37:00Z

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2025-08-04T13:20:43Z

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

farhan · 2025-08-04T14:49:21Z

@kdmccormick You can ignore last comment if didn't read yet. PR is available for the next pass.

Problem Cause: So it was just xblock.plugin.PLUGIN_CACHE that was creating problem, reseting it on tests setUpClass.

open-craft-grove · 2025-08-04T15:34:00Z

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2025-08-04T16:20:16Z

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2025-08-04T17:31:03Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

ttqureshi · 2025-08-05T09:54:42Z

From adding the docs against the if conditions to having the CI checks run for both the Built-in and the Extracted word cloud blocks, we've made a good progress in this PR.

But before we plan to merge this PR, I want to bring one point into consideration that is the pointer tag issue.

I think we can't merge this PR and any of the test PRs until we resolve this issue. Because the extracted blocks don't support the parsing of the pointer tags format properly which can lead to the erroneous parsing of the previously exported Builtin Blocks if they are imported in the pointer tag OLX format (see this comment). For that, I am currently working on this issue.

@feanil @kdmccormick please share your thoughts on it.

cc to: @openedx/axim-aximprovements

farhan · 2025-08-06T07:55:41Z

I think we can't merge this PR and any of the test PRs until we resolve this issue.

@kdmccormick @ttqureshi
I think in this case we should merge this PR keeping USE_EXTRACTED_WORD_CLOUD_BLOCK flag disabled for now, we can enable it default once the pointer tag issue implementation (discussed in last commit) is done.

kdmccormick · 2025-08-06T13:38:54Z

Great work on the tests. Good point @ttqureshi , I will turn my attention back to the pointer tag issue. I agree with @farhan , we can merge this PR so tests run on the extracted block, but keep the feature flag disabled until pointer tags are fixed.

open-craft-grove · 2025-08-06T18:13:29Z

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2025-08-06T19:49:19Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

kdmccormick

Great work!

edx-pipeline-bot · 2025-08-08T16:39:27Z

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

edx-pipeline-bot · 2025-08-08T16:58:50Z

2U Release Notice: This PR has been deployed to the edX production environment.

edx-pipeline-bot · 2025-08-08T17:16:43Z

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

edx-pipeline-bot · 2025-08-08T17:32:33Z

2U Release Notice: This PR has been deployed to the edX production environment.

edx-pipeline-bot · 2025-08-08T17:32:33Z

2U Release Notice: This PR has been deployed to the edX production environment.

edx-pipeline-bot · 2025-08-08T17:55:16Z

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

edx-pipeline-bot · 2025-08-08T18:14:10Z

2U Release Notice: This PR has been deployed to the edX production environment.

edx-pipeline-bot · 2025-08-08T18:14:10Z

2U Release Notice: This PR has been deployed to the edX production environment.

…35983) Run tests for both the built-in and extracted WordCloud block. The tests are mostly compatible with both versions of the block, except for a few places where the XBlock framework and the built-in XModule system differ which we've had to handle using conditionals. This moves us closer to enabling the extracted WordCloud block by default and eventually removing the built-in block. Part of: openedx#34840

This was referenced Dec 7, 2024

Adds word cloud xblock extracting from edx-platform repo openedx/xblocks-contrib#4

Merged

Test PR to check failed test cases for Extracted Built In Blocks #35951

Closed

farhan force-pushed the farhan/test-extracted-word-cloud branch from 8cf696a to 988cb0c Compare January 13, 2025 14:28

farhan closed this Jan 15, 2025

farhan reopened this Jan 15, 2025

farhan force-pushed the farhan/test-extracted-word-cloud branch 4 times, most recently from ad5a8b7 to 02848a2 Compare January 16, 2025 10:40

farhan marked this pull request as ready for review January 16, 2025 11:09

farhan requested review from feanil, irtazaakram, kdmccormick and salman2013 January 16, 2025 11:09

farhan closed this Jan 17, 2025

farhan reopened this Jan 17, 2025

feanil reviewed Jan 21, 2025

View reviewed changes

kdmccormick marked this pull request as draft January 21, 2025 17:54

farhan force-pushed the farhan/test-extracted-word-cloud branch from 3f0447a to add935e Compare January 22, 2025 12:19

farhan commented Jan 24, 2025

View reviewed changes

xmodule/tests/test_word_cloud.py Outdated Show resolved Hide resolved

farhan force-pushed the farhan/test-extracted-word-cloud branch from 5768e3b to d23a0a6 Compare January 28, 2025 07:38

farhan requested a review from feanil January 28, 2025 13:23

kdmccormick added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Jan 30, 2025

farhan marked this pull request as ready for review February 4, 2025 09:07

farhan added create-sandbox open-craft-grove should create a sandbox environment from this PR and removed create-sandbox open-craft-grove should create a sandbox environment from this PR labels Feb 4, 2025

farhan closed this Feb 4, 2025

farhan added 2 commits August 4, 2025 19:38

chore: update tests fix

ba3df94

chore: fix pylint error

bfea346

chore: fix quality check

c2c9b9e

feanil changed the title ~~Enable Extracted Word Cloud | Fix Test Cases~~ Allow for Extracted Word Cloud Block | Fix Test Cases Aug 6, 2025

chore: kept USE_EXTRACTED_WORD_CLOUD_BLOCK flag False for now

af88397

kdmccormick self-requested a review August 8, 2025 14:12

kdmccormick approved these changes Aug 8, 2025

View reviewed changes

kdmccormick merged commit 4a9fc77 into master Aug 8, 2025
49 checks passed

kdmccormick deleted the farhan/test-extracted-word-cloud branch August 8, 2025 15:22

github-project-automation bot moved this from 🏗 In progress to ✅ Done in Aximprovements Team Aug 8, 2025

ormsbee mentioned this pull request Nov 7, 2025

fix: bump learning-core to 0.30.0 #37614

Merged

Comments

Conversation

farhan commented Dec 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

feanil left a comment

Choose a reason for hiding this comment

Uh oh!

feanil Jan 21, 2025

Choose a reason for hiding this comment

Uh oh!

farhan Jan 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

feanil Jan 21, 2025

Choose a reason for hiding this comment

Uh oh!

farhan Jan 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

farhan Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

feanil Jan 21, 2025

Choose a reason for hiding this comment

Uh oh!

farhan Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

farhan commented Jan 22, 2025

Uh oh!

Uh oh!

farhan commented Jan 28, 2025

Uh oh!

open-craft-grove commented Jan 30, 2025

Uh oh!

open-craft-grove commented Aug 4, 2025

Uh oh!

farhan commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

open-craft-grove commented Aug 4, 2025

Uh oh!

open-craft-grove commented Aug 4, 2025

Uh oh!

open-craft-grove commented Aug 4, 2025

Uh oh!

ttqureshi commented Aug 5, 2025

Uh oh!

farhan commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kdmccormick commented Aug 6, 2025

Uh oh!

open-craft-grove commented Aug 6, 2025

Uh oh!

open-craft-grove commented Aug 6, 2025

Uh oh!

kdmccormick left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

edx-pipeline-bot commented Aug 8, 2025

Uh oh!

edx-pipeline-bot commented Aug 8, 2025

Uh oh!

edx-pipeline-bot commented Aug 8, 2025

Uh oh!

edx-pipeline-bot commented Aug 8, 2025

Uh oh!

edx-pipeline-bot commented Aug 8, 2025

Uh oh!

edx-pipeline-bot commented Aug 8, 2025

Uh oh!

edx-pipeline-bot commented Aug 8, 2025

Uh oh!

edx-pipeline-bot commented Aug 8, 2025

Uh oh!

farhan commented Dec 7, 2024 •

edited

Loading

farhan Jan 22, 2025 •

edited

Loading

farhan Jan 22, 2025 •

edited

Loading

farhan commented Aug 4, 2025 •

edited

Loading

farhan commented Aug 6, 2025 •

edited

Loading