Skip to content

fix: prefetch user state only for selected library_content children - #409

Open
mraman-2U wants to merge 1 commit into
release-ulmofrom
mraman-2U/assessments-not-loading
Open

fix: prefetch user state only for selected library_content children#409
mraman-2U wants to merge 1 commit into
release-ulmofrom
mraman-2U/assessments-not-loading

Conversation

@mraman-2U

Copy link
Copy Markdown
Member

This pull request improves how the FieldDataCache prefetches child blocks, especially for dynamic blocks such as library_content, ensuring that only the relevant learner-specific children are prefetched. This change prevents unnecessary loading of user state for blocks that will not be rendered for the current learner, optimizing performance and correctness. Additionally, comprehensive tests are added to verify the new behavior.

Enhancements to child block prefetching:

  • Introduced the _children_for_field_data_cache helper function in model_data.py to determine which child blocks should be prefetched, using get_child_blocks() for dynamic blocks and falling back to get_children() and get_required_block_descriptors() for static blocks.
  • Updated the descendant collection logic in add_block_descendents to use _children_for_field_data_cache, ensuring only the appropriate children are traversed and prefetched.
  • Refactored the descendant collection function from get_child_blocks to collect_descendant_blocks for clarity and to support the new child selection logic.

Testing improvements:

  • Added a new test class TestFieldDataCacheDynamicChildren in test_model_data.py to verify that dynamic blocks only prefetch learner-selected children and static blocks continue to prefetch all children. Also tested that unselected children are not prefetched.
  • Updated imports in test_model_data.py to include the new helper function and related classes.When building FieldDataCache for render, walk get_child_blocks() for dynamic blocks (library_content, itembank) instead of all modulestore children. This aligns prefetch with vertical render behavior and reduces StudentModule over-fetch on large randomized quizzes.

Description

Describe what this pull request changes, and why. Include implications for people using this change.
Design decisions and their rationales should be documented in the repo (docstring / ADR), per
OEP-19, and can be
linked here.

Useful information to include:

  • Which edX user roles will this change impact? Common user roles are "Learner", "Course Author",
    "Developer", and "Operator".
  • Include screenshots for changes to the UI (ideally, both "before" and "after" screenshots, if applicable).
  • Provide links to the description of corresponding configuration changes. Remember to correctly annotate these
    changes.

Supporting information

Link to other information about the change, such as Jira issues, GitHub issues, or Discourse discussions.
Be sure to check they are publicly readable, or if not, repeat the information here.

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.

When building FieldDataCache for render, walk get_child_blocks() for
dynamic blocks (library_content, itembank) instead of all modulestore
children. This aligns prefetch with vertical render behavior and reduces
StudentModule over-fetch on large randomized quizzes.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copilot AI review requested due to automatic review settings July 30, 2026 11:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This pull request updates LMS FieldDataCache descendant traversal so that blocks with dynamic, learner-specific children (e.g., library_content/item bank/randomize-like blocks) prefetch user state only for the children that will be rendered for the current learner, reducing unnecessary user-state prefetch and improving correctness for dynamic content selection.

Changes:

  • Added _children_for_field_data_cache() helper to choose child blocks via get_child_blocks() for dynamic blocks, falling back to get_children() + get_required_block_descriptors() for static blocks.
  • Updated FieldDataCache.add_block_descendents traversal to use the new helper (and renamed the internal traversal function to collect_descendant_blocks).
  • Added tests validating that dynamic blocks only prefetch selected children and do not prefetch unselected children.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
lms/djangoapps/courseware/model_data.py Switches descendant traversal to prefer get_child_blocks() for dynamic blocks via a new helper.
lms/djangoapps/courseware/tests/test_model_data.py Adds coverage to ensure only learner-selected dynamic children are prefetched.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +671 to +675
get_child_blocks = getattr(block, 'get_child_blocks', None)
has_dynamic_children = getattr(block, 'has_dynamic_children', None)
if callable(has_dynamic_children) and has_dynamic_children() and callable(get_child_blocks):
return list(get_child_blocks())

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants