-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Render Word cloud block editor [FC-0076] #36199
base: master
Are you sure you want to change the base?
fix: Render Word cloud block editor [FC-0076] #36199
Conversation
- The xmodule-type to render is MetadataOnlyEditingDescriptor - The xmodule type `MetadataOnlyEditingDescriptor` renders a `<div>` with the block metadata in the `data-metadata` attribute. But is necessary to call `XBlockEditorView.xblockReady()` to run the scripts to build the editor using the metadata. - To call XBlockEditorView.xblockReady() we need a specific require.config
Thanks for the pull request, @ChrisChV! 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.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Thank you for your work @ChrisChV!
- I tested this using the instructions from the PR
- I read through the code
- I checked for accessibility issues
- Includes documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ChrisChV , this is working fine for Word Clouds 👍
But I can't paste a Conditional into a Library at all -- see comment for traceback.
xmodule/conditional_block.py
Outdated
return { | ||
'result': 'success', | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add Conditional blocks to courses with no issue. 👍
But I'm unable to test pasting a Conditional block into a library due to this error during re-indexing:
cms-1 | 2025-02-04 00:27:28,772 INFO 1640 [openedx.core.djangoapps.content.search.tasks] [user 6] [ip 172.24.0.1] tasks.py:55 - Updating content index document for library block with id: lb:OpenCraft:FAL-4029:conditional:f021eaec-5964-4ce7-8f1c-89f1f2f2ab04
cms-1 | 2025-02-04 00:27:28,806 ERROR 1640 [celery_utils.logged_task] [user 6] [ip 172.24.0.1] logged_task.py:48 - [2f7df8e9-8d8d-4e3b-9989-8ffd2d575f77] failed due to Traceback (most recent call last):
cms-1 | File "/openedx/edx-platform/xmodule/xml_block.py", line 399, in parse_xml_new_runtime
cms-1 | return super().parse_xml_new_runtime(node, runtime, keys)
cms-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
cms-1 | AttributeError: 'super' object has no attribute 'parse_xml_new_runtime'
cms-1 |
cms-1 | During handling of the above exception, another exception occurred:
cms-1 |
cms-1 | Traceback (most recent call last):
cms-1 | File "/openedx/venv/lib/python3.11/site-packages/celery/app/trace.py", line 453, in trace_task
cms-1 | R = retval = fun(*args, **kwargs)
cms-1 | ^^^^^^^^^^^^^^^^^^^^
cms-1 | File "/openedx/venv/lib/python3.11/site-packages/celery/app/autoretry.py", line 38, in run
cms-1 | return task._orig_run(*args, **kwargs)
cms-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
cms-1 | File "/openedx/venv/lib/python3.11/site-packages/edx_django_utils/monitoring/internal/code_owner/utils.py", line 195, in new_function
cms-1 | return wrapped_function(*args, **kwargs)
cms-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
cms-1 | File "/openedx/edx-platform/openedx/core/djangoapps/content/search/tasks.py", line 57, in upsert_library_block_index_doc
cms-1 | api.upsert_library_block_index_doc(usage_key)
cms-1 | File "/openedx/edx-platform/openedx/core/djangoapps/content/search/api.py", line 639, in upsert_library_block_index_doc
cms-1 | searchable_doc_for_library_block(library_block_metadata)
cms-1 | File "/openedx/edx-platform/openedx/core/djangoapps/content/search/documents.py", line 381, in searchable_doc_for_library_block
cms-1 | block = xblock_api.load_block(xblock_metadata.usage_key, user=None)
cms-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
cms-1 | File "/openedx/edx-platform/openedx/core/djangoapps/xblock/api.py", line 117, in load_block
cms-1 | return runtime.get_block(usage_key, version=version)
cms-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
cms-1 | File "/openedx/edx-platform/openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py", line 217, in get_block
cms-1 | block = block_class.parse_xml_new_runtime(xml_node, runtime=self, keys=keys)
cms-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
cms-1 | File "/openedx/edx-platform/xmodule/xml_block.py", line 401, in parse_xml_new_runtime
cms-1 | return super().parse_xml(node, runtime, keys)
cms-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
cms-1 | File "/mnt/xblock/xblock/core.py", line 763, in parse_xml
cms-1 | block.runtime.add_node_as_child(block, child)
cms-1 | File "/openedx/edx-platform/openedx/core/djangoapps/xblock/runtime/runtime.py", line 267, in add_node_as_child
cms-1 | raise NotImplementedError("XML Serialization is only supported with LearningCoreXBlockRuntime")
cms-1 | NotImplementedError: XML Serialization is only supported with LearningCoreXBlockRuntime
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pomegranited I can't reproduce this error. Copying creates the block without any problems. Maybe there is an additional context that I'm ignoring?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok weird.. I'm still seeing this issue? I don't get an error in the frontend at all, but the pasted Conditional block never appears.
I tried:
- checking all my mounted dependencies to make sure i'm running the platform required versions (esp XBlock)
- reindexing studio -- edit these errors don't show up in the shell to, e.g. "Error indexing library component xblock.v1:conditional:33a7bf8b-bf02-4783-9e76-941097b53c36: XML Serialization is only supported with LearningCoreXBlockRuntime"
- checking Django Admin -- the conditional blocks show up in the learning package's component list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ChrisChV Could you share the course you used? Maybe there's something wrong with the configuration I chose... grasping at straws here :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it seems strange. I have created a new course and it works fine. Here I attach the course: course.q8xomjt4.tar.gz
@rpenido Did you have any problems with it, suddenly we have some configurations that we forgot to mention?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ChrisChV I was able to reproduce this issue, if you try to copy and paste a conditional block that has children, it fails with above error that @pomegranited pasted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @navinkarkera @pomegranited! I've seen the issue. We're not going to support multi-level blocks for now (ref), so I've removed this new code related to the conditional in 0be1cfe. Can you please review it again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to reproduce this issue, if you try to copy and paste a conditional block that has children, it fails with above error that @pomegranited pasted.
Ohhh.. that makes sense. Thank you for confirming that I'm not crazy, @navinkarkera !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ChrisChV Got it, but now save button for conditional block doesn't work as you removed studio_submit
handler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@navinkarkera I created openedx/frontend-app-authoring#1652 to avoid copy/edit multilevel blocks, it's ready for review
@XBlock.json_handler | ||
def studio_submit(self, submissions, suffix=''): # pylint: disable=unused-argument | ||
""" | ||
Change the settings for this XBlock given by the Studio user | ||
""" | ||
if 'display_name' in submissions: | ||
self.display_name = submissions['display_name'] | ||
if 'instructions' in submissions: | ||
self.instructions = submissions['instructions'] | ||
if 'num_inputs' in submissions: | ||
self.num_inputs = submissions['num_inputs'] | ||
if 'num_top_words' in submissions: | ||
self.num_top_words = submissions['num_top_words'] | ||
if 'display_student_percents' in submissions: | ||
self.display_student_percents = submissions['display_student_percents'] == 'True' | ||
|
||
return { | ||
'result': 'success', | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@irtazaakram @farhan This PR makes a fix to the built-in Word Cloud block. If it is relevant, could you please ensure the fix is applied to the extracted Word Cloud as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(context for OpenCraft: We are in the process of extracting WordCloud from edx-platform)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ChrisChV This looks good except for the error while pasting conditional blocks with children. Also clicking on Add
button in conditional block Source components
field opens up a new tab with the same fields.
xmodule/conditional_block.py
Outdated
return { | ||
'result': 'success', | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ChrisChV I was able to reproduce this issue, if you try to copy and paste a conditional block that has children, it fails with above error that @pomegranited pasted.
Description
xmodule-type
to render isMetadataOnlyEditingDescriptor
andSequenceDescriptor
MetadataOnlyEditingDescriptor
andSequenceDescriptor
renders a<div>
with the block metadata in thedata-metadata
attribute. But is necessary to callXBlockEditorView.xblockReady()
to run the scripts to build the editor using the metadata.XBlockEditorView.xblockReady()
we need aspecific require.config
Supporting information
Issue from testing: Word Cloud
Internal ticket: https://tasks.opencraft.com/browse/FAL-4029
Testing instructions
Deadline
No rush
Other information