Skip to content

feat: unify ModuleSystem and DescriptorSystem [BD-13] - #31472

Merged
jristau1984 merged 13 commits into
openedx:masterfrom
open-craft:kaustav/unify_modulesystem_descriptorsystem
Apr 24, 2023
Merged

feat: unify ModuleSystem and DescriptorSystem [BD-13]#31472
jristau1984 merged 13 commits into
openedx:masterfrom
open-craft:kaustav/unify_modulesystem_descriptorsystem

Conversation

@kaustavb12

Copy link
Copy Markdown
Contributor

Description

Work in progress PR to unify ModuleSystem and DescriptorSystem

Supporting information

OpenCraft internal ticket BB-5967

Testing instructions

Please provide detailed step-by-step instructions for testing this change.

Deadline

"None" if there's no rush, or provide a specific date or event (and reason) if there is one.

Other information

Include anything else that will help reviewers and consumers understand the change.

  • Does this change depend on other changes elsewhere?
  • Any special concerns or limitations? For example: deprecations, migrations, security, or accessibility.
  • If your database migration can't be rolled back easily.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Dec 22, 2022
@openedx-webhooks

openedx-webhooks commented Dec 22, 2022

Copy link
Copy Markdown

Thanks for the pull request, @kaustavb12!

When this pull request is ready, tag your edX technical lead.

@kaustavb12
kaustavb12 force-pushed the kaustav/unify_modulesystem_descriptorsystem branch 2 times, most recently from 4917eeb to 84beb2b Compare January 5, 2023 06:08
@Agrendalath Agrendalath changed the title feat: unify ModuleSystem and DescriptorSystem feat: unify ModuleSystem and DescriptorSystem [BD-13] Jan 9, 2023
@openedx-webhooks openedx-webhooks added blended PR is managed through 2U's blended developmnt program and removed open-source-contribution PR author is not from Axim or 2U labels Jan 9, 2023
@kaustavb12
kaustavb12 force-pushed the kaustav/unify_modulesystem_descriptorsystem branch 10 times, most recently from 56519af to ef00433 Compare February 10, 2023 06:56
Comment thread xmodule/x_module.py Outdated
Comment thread xmodule/x_module.py Outdated
Comment thread xmodule/x_module.py Outdated
Comment thread xmodule/x_module.py Outdated
Comment thread xmodule/x_module.py Outdated
Comment thread lms/djangoapps/courseware/block_render.py Outdated
Comment thread lms/djangoapps/courseware/block_render.py Outdated
Comment thread lms/djangoapps/lms_xblock/runtime.py Outdated
Comment thread lms/djangoapps/courseware/block_render.py Outdated
Comment thread xmodule/services.py Outdated
@kaustavb12
kaustavb12 force-pushed the kaustav/unify_modulesystem_descriptorsystem branch 4 times, most recently from f90bb9d to 7b16070 Compare February 15, 2023 18:03
@Agrendalath
Agrendalath force-pushed the kaustav/unify_modulesystem_descriptorsystem branch from 7b16070 to 6b10606 Compare March 22, 2023 18:02
@Agrendalath
Agrendalath force-pushed the kaustav/unify_modulesystem_descriptorsystem branch 3 times, most recently from 06d148b to d6eec2f Compare April 1, 2023 01:26

@Agrendalath Agrendalath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

  • I tested this: did the regression testing of XBlocks, instructor tasks, etc.
  • I read through the code
  • I checked for accessibility issues: n/a
  • Includes documentation: n/a
  • I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository: n/a

@Agrendalath
Agrendalath force-pushed the kaustav/unify_modulesystem_descriptorsystem branch from c4d9e66 to 6b5f9ee Compare April 21, 2023 09:53
@Agrendalath
Agrendalath force-pushed the kaustav/unify_modulesystem_descriptorsystem branch from 6b5f9ee to c74f382 Compare April 21, 2023 09:54
@Agrendalath Agrendalath self-assigned this Apr 24, 2023
@jristau1984
jristau1984 merged commit 472a3b5 into openedx:master Apr 24, 2023
@openedx-webhooks

Copy link
Copy Markdown

@kaustavb12 🎉 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

Copy link
Copy Markdown
Contributor

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

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

@Agrendalath
Agrendalath deleted the kaustav/unify_modulesystem_descriptorsystem branch April 25, 2023 14:06
Agrendalath added a commit to open-craft/openedx-platform that referenced this pull request May 2, 2023
After changes from openedx#31472, the user service of a "leaf" XBlock gets overridden
with the one created for its parent (SequenceBlock). Therefore, the
`requires_per_student_anonymous_id` is ignored in these XBlocks. The
subsequent renders of an XBlock (e.g., when requesting the solution) use
the student-specific IDs.

This removes choosing the proper ID (course-specific or student-specific) from
the runtime initialization. Instead, both IDs are passed to the user service.

There are only two XBlocks that relied on the
`requires_per_student_anonymous_id` - `ProblemBlock` and `HtmlBlock`. They
now request the "deprecated" (student-specific) user ID directly from the user
service.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blended PR is managed through 2U's blended developmnt program

Projects

Archived in project
Status: Done

Development

Successfully merging this pull request may close these issues.

7 participants