feat: [FC-0044] XBlock's children API as DRF #34055

ruzniaievdm · 2024-01-15T13:11:46Z

Settings

TUTOR_GROVE_WAFFLE_FLAGS:
  - name: contentstore.new_studio_mfe.use_new_unit_page
    everyone: true

TUTOR_GROVE_MFE_LMS_COMMON_SETTINGS:
  MFE_CONFIG:
    ENABLE_UNIT_PAGE: true

Description

This PR addresses a need in the development of the Course Unit Page feature within the frontend-app-course-authoring repository. The core objective is the implement of a DRF endpoint designed to render and deliver children data for container xblock.

This PR also includes:

adding unit type to create_xblock_info handler. It needs to display type of unit icons for the unit course page. Each unit must have one of the following types: video, vertical, problem, lock.
resolving problem with 500 error if user adds a Content Experiment (also needs to discuss)

Testing instructions

Run master devstack.
Start platform make dev.up.lms+cms and make checkout on this branch.
Go to http://localhost:18010/api-docs.
Find the required API endpoint /api/contentstore/v1/container/vertical/{usage_key_string}/children.
Open needed unit page in cms http://localhost:18010/container/{usage_key_string} and make sure nothing has changed.
Copy usage_key_string from previous step and paste it to required API endpoint to get data for unit.

How to reproduce an error

The problem arose at the stage of defining icons for children xblock's of the unit.
Just when we tried to get get_icon() for xblock which have split_test children.

Go to Advanced settings page and add "split_test" to Advanced module list
Go to Group configurations page and create group in Experiment group configurations section
Go to Course Outline, create new unit, go to unit page
In the Advanced select "Content Experiment"

File "/edx/app/edxapp/edx-platform/xmodule/vertical_block.py", line 254, in get_icon_class
    child_classes = {child.get_icon_class() for child in self.get_children()}
  File "/edx/app/edxapp/edx-platform/xmodule/vertical_block.py", line 254, in <setcomp>
    child_classes = {child.get_icon_class() for child in self.get_children()}
  File "/edx/app/edxapp/edx-platform/xmodule/split_test_block.py", line 398, in get_icon_class
    return self.child.get_icon_class() if self.child else 'other'
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/utils/functional.py", line 48, in __get__
    res = instance.__dict__[self.name] = self.func(instance)
  File "/edx/app/edxapp/edx-platform/xmodule/split_test_block.py", line 176, in child
    if self.child_block is not None:
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/utils/functional.py", line 48, in __get__
    res = instance.__dict__[self.name] = self.func(instance)
  File "/edx/app/edxapp/edx-platform/xmodule/split_test_block.py", line 166, in child_block
    child_blocks = self.get_child_blocks()
  File "/edx/app/edxapp/edx-platform/xmodule/split_test_block.py", line 216, in get_child_blocks
    group_id = self.get_group_id()
  File "/edx/app/edxapp/edx-platform/xmodule/split_test_block.py", line 243, in get_group_id
    user_service = self.runtime.service(self, 'user')
  File "/edx/app/edxapp/edx-platform/xmodule/modulestore/split_mongo/caching_descriptor_system.py", line 245, in service
    return super().service(block, service_name)
  File "/edx/app/edxapp/edx-platform/xmodule/x_module.py", line 1502, in service
    service = super().service(block=block, service_name=service_name)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/xblock/runtime.py", line 1125, in service
    raise NoSuchServiceError(f"Service {service_name!r} is not available.")
xblock.exceptions.NoSuchServiceError: Service 'user' is not available.

openedx-webhooks · 2024-01-15T13:11:52Z

Thanks for the pull request, @ruzniaievdm! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

mphilbrick211 · 2024-01-16T15:12:06Z

Hi @ruzniaievdm! Just flagging that there's a couple failing checks. Would you mind taking a look? Thanks!

open-craft-grove · 2024-02-14T18:34:38Z

Sandbox deployment successful.

Sandbox LMS is available at https://pr-34055-ff10e3.sandboxes.opencraft.hosting
Sandbox Studio is available at https://studio.pr-34055-ff10e3.sandboxes.opencraft.hosting

Instance Tutor config : https://axim-sandbox-build-logs-4626178240.nyc3.digitaloceanspaces.com/pr-34055-ff10e3-51564656-6173868163-tutor-config.yml
Instance Grove config : https://axim-sandbox-build-logs-4626178240.nyc3.digitaloceanspaces.com/pr-34055-ff10e3-51564656-6173868163-grove-config.yml
Instance Python requirements : https://axim-sandbox-build-logs-4626178240.nyc3.digitaloceanspaces.com/pr-34055-ff10e3-51564656-6173868163-tutor-requirements.txt

open-craft-grove · 2024-02-14T18:51:24Z

Sandbox deployment successful.

Sandbox LMS is available at https://pr-34055-ff10e3.sandboxes.opencraft.hosting
Sandbox Studio is available at https://studio.pr-34055-ff10e3.sandboxes.opencraft.hosting

Instance Tutor config : https://axim-sandbox-build-logs-4626178240.nyc3.digitaloceanspaces.com/pr-34055-ff10e3-51564656-6174127840-tutor-config.yml
Instance Grove config : https://axim-sandbox-build-logs-4626178240.nyc3.digitaloceanspaces.com/pr-34055-ff10e3-51564656-6174127840-grove-config.yml
Instance Python requirements : https://axim-sandbox-build-logs-4626178240.nyc3.digitaloceanspaces.com/pr-34055-ff10e3-51564656-6174127840-tutor-requirements.txt

arbrandes

I have tested this in the sandbox and locally as described, and cannot find any problems.

I think a second opinion is warranted on how to solve the split_test bug. (Plus, I'm technically not a CC here, so can't give final approval.)

arbrandes · 2024-02-23T16:02:05Z

cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py

+# List of categories to check for presence in the children of the XBlock.
+# This list is used to determine if all of the specified categories are absent
+# in the categories of the children XBlock instances otherwise icon class variable will be set to `None`.
+CATEGORIES_WITH_ABSENT_ICON = ["split_test"]


This seems difficult to maintain, but I can't think of a cheaper workaround. The question is if there's a more architecturally sound way of doing it, and if we want to hold off on merging this until we find one.

@kdmccormick or @feanil, thoughts?

@arbrandes Agreed, we should not hard code block types if at all possible. There are places that do it already, but I would rather avoid adding more.

The comment is somewhat confusing to me. What is the meaning of an item being in this list? Is it that is type should be ignored when determining the icon?

* fix: wrap into try/except block getting icon for xblock * fix: revision after review

open-craft-grove · 2024-03-06T15:06:46Z

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

ruzniaievdm · 2024-03-06T15:10:53Z

@arbrandes I apologize for confusing you, indeed, in the previous version namely up to and including the second commit, you did not see the error, there was a workaround to this problem. And if you had made these changes, you would have seen the problem.

"xblock_type": get_icon(xblock) if is_xblock_unit and all_excluded_categories_absent else None → "xblock_type": get_icon(xblock)

@arbrandes @kdmccormick After reviewing your comments we decided to take a different approach to solving this problem. Please review again.

arbrandes

I have no objections, this time around.

arbrandes · 2024-03-08T13:17:29Z

xmodule/split_test_block.py

+            log.warning(
+                "Error while getting block instance for descriptor with location: [%s]",
+                self.location
+            )


Ok, this feels much more manageable. 👍🏼

openedx-webhooks · 2024-03-12T15:07:35Z

@ruzniaievdm 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

edx-pipeline-bot · 2024-03-12T16:32:06Z

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

edx-pipeline-bot · 2024-03-12T17:06:04Z

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

edx-pipeline-bot · 2024-03-12T17:24:01Z

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

edx-pipeline-bot · 2024-03-12T18:26:27Z

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

edx-pipeline-bot · 2024-03-12T18:26:28Z

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

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jan 15, 2024

ruzniaievdm changed the title ~~feat: XBlock's children API as DRF~~ draft: feat: XBlock's children API as DRF Jan 15, 2024

ruzniaievdm changed the title ~~draft: feat: XBlock's children API as DRF~~ feat: XBlock's children API as DRF Jan 15, 2024

ruzniaievdm changed the title ~~feat: XBlock's children API as DRF~~ DRAFT: feat: XBlock's children API as DRF Jan 15, 2024

ruzniaievdm force-pushed the ruzniaievdm/feat/unit-children branch 4 times, most recently from 36c9991 to 417187a Compare January 24, 2024 21:08

ruzniaievdm force-pushed the ruzniaievdm/feat/unit-children branch from 417187a to 3543267 Compare February 7, 2024 16:29

ruzniaievdm changed the title ~~DRAFT: feat: XBlock's children API as DRF~~ feat: [FC-0044] XBlock's children API as DRF Feb 7, 2024

ihor-romaniuk requested a review from arbrandes February 12, 2024 08:54

monteri requested a review from KristinAoki February 14, 2024 13:06

arbrandes added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Feb 14, 2024

arbrandes self-assigned this Feb 14, 2024

mphilbrick211 added the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Feb 21, 2024

arbrandes reviewed Feb 23, 2024

View reviewed changes

ihor-romaniuk mentioned this pull request Feb 28, 2024

feat: [FC-0044] Unit page - display xblock components openedx/frontend-app-authoring#857

Merged

ruzniaievdm mentioned this pull request Mar 5, 2024

fix: wrap into try/except block getting icon for xblock raccoongang/edx-platform#2509

Merged

ruzniaievdm added 2 commits March 6, 2024 16:19

feat: XBlock's children API as DRF

de569b7

fix: 500 error appears if user adds a Content Experiment

d8a42c9

ruzniaievdm force-pushed the ruzniaievdm/feat/unit-children branch from 3543267 to 6f45f44 Compare March 6, 2024 14:23

fix: wrap into try/except block getting icon for xblock (#2509)

8979134

* fix: wrap into try/except block getting icon for xblock * fix: revision after review

ruzniaievdm force-pushed the ruzniaievdm/feat/unit-children branch from 6f45f44 to 8979134 Compare March 6, 2024 14:25

ruzniaievdm requested a review from arbrandes March 6, 2024 15:11

ruzniaievdm requested a review from kdmccormick March 6, 2024 15:11

arbrandes approved these changes Mar 8, 2024

View reviewed changes

bradenmacdonald mentioned this pull request Mar 11, 2024

fix: [FC-0044] Course unit - Replaced the LMS endpoints openedx/frontend-app-authoring#882

Merged

KristinAoki merged commit cd5c4c9 into openedx:master Mar 12, 2024
47 checks passed

ChrisChV mentioned this pull request Mar 12, 2024

[FC-0036] Tags Sidebar openedx/frontend-app-authoring#852

Merged

arbrandes mentioned this pull request Mar 19, 2024

New Unit Page (Studio MFE conversion) openedx/platform-roadmap#321

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: [FC-0044] XBlock's children API as DRF #34055

feat: [FC-0044] XBlock's children API as DRF #34055

ruzniaievdm commented Jan 15, 2024 •

edited by arbrandes

Loading

openedx-webhooks commented Jan 15, 2024

mphilbrick211 commented Jan 16, 2024

open-craft-grove commented Feb 14, 2024

open-craft-grove commented Feb 14, 2024

arbrandes left a comment

arbrandes Feb 23, 2024

kdmccormick Feb 23, 2024 •

edited

Loading

open-craft-grove commented Mar 6, 2024

ruzniaievdm commented Mar 6, 2024

arbrandes left a comment

arbrandes Mar 8, 2024

openedx-webhooks commented Mar 12, 2024

edx-pipeline-bot commented Mar 12, 2024

edx-pipeline-bot commented Mar 12, 2024

edx-pipeline-bot commented Mar 12, 2024

edx-pipeline-bot commented Mar 12, 2024

edx-pipeline-bot commented Mar 12, 2024

feat: [FC-0044] XBlock's children API as DRF #34055

feat: [FC-0044] XBlock's children API as DRF #34055

Conversation

ruzniaievdm commented Jan 15, 2024 • edited by arbrandes Loading

Description

Testing instructions

How to reproduce an error

openedx-webhooks commented Jan 15, 2024

mphilbrick211 commented Jan 16, 2024

open-craft-grove commented Feb 14, 2024

open-craft-grove commented Feb 14, 2024

arbrandes left a comment

Choose a reason for hiding this comment

arbrandes Feb 23, 2024

Choose a reason for hiding this comment

kdmccormick Feb 23, 2024 • edited Loading

Choose a reason for hiding this comment

open-craft-grove commented Mar 6, 2024

ruzniaievdm commented Mar 6, 2024

arbrandes left a comment

Choose a reason for hiding this comment

arbrandes Mar 8, 2024

Choose a reason for hiding this comment

openedx-webhooks commented Mar 12, 2024

edx-pipeline-bot commented Mar 12, 2024

edx-pipeline-bot commented Mar 12, 2024

edx-pipeline-bot commented Mar 12, 2024

edx-pipeline-bot commented Mar 12, 2024

edx-pipeline-bot commented Mar 12, 2024

ruzniaievdm commented Jan 15, 2024 •

edited by arbrandes

Loading

kdmccormick Feb 23, 2024 •

edited

Loading