Test PR | Enable Extracted Word Cloud #35983

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

It's the test PR for this xblocks-contrib repo PR

Steps done in this PR:

Enabled Extracted Block in lms/envs/common.py settings
Enable fail-fast: false in .github/workflows/unit-tests.yml
Replace pip installed version of xblocks-contrib with following git branched installation.

git+https://github.com/openedx/xblocks-contrib.git@farhan/word-cloud-xblock

PR sandbox

Username: openedx
PW: openedx

Settings

TUTOR_GROVE_COMMON_SETTINGS: |
  USE_EXTRACTED_WORD_CLOUD_BLOCK: false

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

            'element_id': self.block.location.html_id(),
            'num_inputs': 5,  # default value
            'submitted': False,  # default value,
        }

-        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.

farhan · 2025-01-24T14:29:02Z

xmodule/tests/test_word_cloud.py

+
+        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)


As per discussion in pair programming session (@kdmccormick , @ttqureshi ) we are creating scope ids here which shouldn't happen and is a potential bug

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-02-05T16:06:48Z

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-02-06T04:13:50Z

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

farhan · 2025-02-06T06:37:54Z

@kdmccormick Thanks for doing the testing

I only see half the words I added, but maybe that's existing behavior.

Yes, its existing behaviour. Often word cloud doesn't show all the words. You can refresh the page to get a refreshed experience.

But the LMS view shows nothing for me:

It was an issue. I have updated the word cloud selector in this commit and redeploy the sandbox.
So everything should work good now

Last thing looking into is #36199

cc: @ttqureshi @irtazaakram @feanil @salman2013

salman2013 · 2025-02-06T11:07:41Z

On local edx-platform Studio works fine.

When i preview it then it do not show anything, Please tell me if i do anything wrong or there is something need to configure, i am wondering as the video xblock is showing in preview mode.

@farhan

open-craft-grove · 2025-02-10T12:34:12Z

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

open-craft-grove · 2025-02-10T16:53:20Z

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-02-10T17:56:24Z

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-02-11T03:03:27Z

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

open-craft-grove · 2025-02-11T08:33:11Z

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

open-craft-grove · 2025-02-11T09:10:33Z

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-02-11T14:40:14Z

@kdmccormick
I think the settings configured in the PR description has effected the sandbox but it's not reflecting the changes accordingly.

For testing, I have turned the setting USE_EXTRACTED_WORD_CLOUD_BLOCK to false as it's already hard coded to true in this PR.

But this disabling is not reflecting on the sandbox environment with multiple formats applied.

To easily test the django settings flag we have added a data variable here

I don't think so that it's something that is blocking the Word Cloud Extraction PR to be release.

cc: @openedx/axim-aximprovements , @ttqureshi

open-craft-grove · 2025-02-18T14:34:11Z

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

open-craft-grove · 2025-02-19T09:55:42Z

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-02-19T11:00:55Z

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

farhan · 2025-02-19T15:11:46Z

@kdmccormick @irtazaakram @salman2013
This PR is in merge state now.
Should we merge it or close it. Sooner or later we need it when we will make the extracted word cloud as a default one.

kdmccormick · 2025-02-19T15:30:03Z

Let's keep this unmerged. I approved the xblocks-contrib PR without testing it much myself in order to unblock development of other blocks, so I'd like to take more time to test this before we make it the default.

open-craft-grove · 2025-02-19T15:35:35Z

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

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 kdmccormick, irtazaakram, salman2013 and feanil 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

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 removed the create-sandbox open-craft-grove should create a sandbox environment from this PR label Feb 5, 2025

farhan closed this Feb 5, 2025

farhan reopened this Feb 5, 2025

farhan added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Feb 5, 2025

irtazaakram mentioned this pull request Feb 12, 2025

Adds Annotatable XBlock extracting from edx-platform repo openedx/xblocks-contrib#5

Open

8 tasks

farhan closed this Feb 12, 2025

farhan reopened this Feb 12, 2025

farhan force-pushed the farhan/test-extracted-word-cloud branch from 8d2faea to 933fdc6 Compare February 18, 2025 13:52

farhan force-pushed the farhan/test-extracted-word-cloud branch from 933fdc6 to 0cefe16 Compare February 19, 2025 09:20

chore: Fix test cases for extracted word cloud block

22ee2c8

farhan force-pushed the farhan/test-extracted-word-cloud branch from 0cefe16 to 22ee2c8 Compare February 19, 2025 09:47

farhan closed this Feb 19, 2025

farhan reopened this Feb 19, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test PR | Enable Extracted Word Cloud #35983

Test PR | Enable Extracted Word Cloud #35983

farhan commented Dec 7, 2024 •

edited

Loading

feanil left a comment

feanil Jan 21, 2025

farhan Jan 22, 2025 •

edited

Loading

feanil Jan 21, 2025

farhan Jan 22, 2025 •

edited

Loading

farhan Jan 27, 2025

feanil Jan 21, 2025

farhan Jan 22, 2025

farhan commented Jan 22, 2025

farhan Jan 24, 2025 •

edited

Loading

farhan commented Jan 28, 2025

open-craft-grove commented Jan 30, 2025

open-craft-grove commented Feb 5, 2025

open-craft-grove commented Feb 6, 2025

farhan commented Feb 6, 2025 •

edited

Loading

salman2013 commented Feb 6, 2025 •

edited

Loading

open-craft-grove commented Feb 10, 2025

open-craft-grove commented Feb 10, 2025

open-craft-grove commented Feb 10, 2025

open-craft-grove commented Feb 11, 2025

open-craft-grove commented Feb 11, 2025

open-craft-grove commented Feb 11, 2025

farhan commented Feb 11, 2025 •

edited

Loading

open-craft-grove commented Feb 18, 2025

open-craft-grove commented Feb 19, 2025

open-craft-grove commented Feb 19, 2025

farhan commented Feb 19, 2025

kdmccormick commented Feb 19, 2025

open-craft-grove commented Feb 19, 2025

Test PR | Enable Extracted Word Cloud #35983

Are you sure you want to change the base?

Test PR | Enable Extracted Word Cloud #35983

Conversation

farhan commented Dec 7, 2024 • edited Loading

PR sandbox

feanil left a comment

Choose a reason for hiding this comment

feanil Jan 21, 2025

Choose a reason for hiding this comment

farhan Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

feanil Jan 21, 2025

Choose a reason for hiding this comment

farhan Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

farhan Jan 27, 2025

Choose a reason for hiding this comment

feanil Jan 21, 2025

Choose a reason for hiding this comment

farhan Jan 22, 2025

Choose a reason for hiding this comment

farhan commented Jan 22, 2025

farhan Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

farhan commented Jan 28, 2025

open-craft-grove commented Jan 30, 2025

open-craft-grove commented Feb 5, 2025

open-craft-grove commented Feb 6, 2025

farhan commented Feb 6, 2025 • edited Loading

salman2013 commented Feb 6, 2025 • edited Loading

open-craft-grove commented Feb 10, 2025

open-craft-grove commented Feb 10, 2025

open-craft-grove commented Feb 10, 2025

open-craft-grove commented Feb 11, 2025

open-craft-grove commented Feb 11, 2025

open-craft-grove commented Feb 11, 2025

farhan commented Feb 11, 2025 • edited Loading

open-craft-grove commented Feb 18, 2025

open-craft-grove commented Feb 19, 2025

open-craft-grove commented Feb 19, 2025

farhan commented Feb 19, 2025

kdmccormick commented Feb 19, 2025

open-craft-grove commented Feb 19, 2025

farhan commented Dec 7, 2024 •

edited

Loading

farhan Jan 22, 2025 •

edited

Loading

farhan Jan 22, 2025 •

edited

Loading

farhan Jan 24, 2025 •

edited

Loading

farhan commented Feb 6, 2025 •

edited

Loading

salman2013 commented Feb 6, 2025 •

edited

Loading

farhan commented Feb 11, 2025 •

edited

Loading