fix: avoid item bank side effects during field data prefetch

[mraman-2U]

This pull request updates how descendant blocks are prefetched and cached in the LMS, improving support for dynamic blocks (like library content or item banks) that expose different children per learner. The main change ensures that only the relevant, learner-selected child blocks are prefetched, avoiding unnecessary loading and side effects for blocks that won't be rendered for the current user. The changes also introduce a read-only prefetch API for dynamic blocks, and add comprehensive tests to verify the correct behavior.

Dynamic block prefetch improvements:

• Added the _children_for_field_data_cache function in model_data.py to ensure that only learner-selected children are prefetched for dynamic blocks, using the appropriate API depending on whether the operation is read-only.
• Updated FieldDataCache.add_block_descendents to use _children_for_field_data_cache, preventing unnecessary loading of unselected children for dynamic blocks. [1] [2]

Dynamic block API changes:

• In item_bank_block.py, updated selected_children and _get_selected_child_blocks to support a read_only parameter, and added get_child_blocks_for_prefetch to return selected child blocks without publishing events or mutating user state. [1] [2] [3]

Testing and verification:

• Added TestFieldDataCacheDynamicChildren tests to verify that only selected children are prefetched for dynamic blocks, that the read-only prefetch API is used when appropriate, and that static blocks continue to prefetch all children.
• Added a test to ensure that read-only prefetching of child blocks does not trigger analytics events or persist user state in test_item_bank.py.

These changes make field data caching more efficient and side-effect-free for dynamic blocks, improving both performance and correctness in learner-specific content rendering.<!--

Note: Please refer to the Support Development Guidelines on the wiki page to consider backporting to active releases:
https://openedx.atlassian.net/wiki/spaces/COMM/pages/4248436737/Support+Guidelines+for+active+releases

Please give your pull request a short but descriptive title.
Use conventional commits to separate and summarize commits logically:
https://open-edx-proposals.readthedocs.io/en/latest/oep-0051-bp-conventional-commits.html

Use this template as a guide. Omit sections that don't apply.
You may link to information rather than copy it, but only if the link is publicly readable.
If the linked information must be private (because it contains secrets), clearly label the link as private.

-->

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.

[Copilot]

Pull request overview

This PR adjusts LMS field-data descendant prefetching to better support XBlocks with learner-specific (“dynamic”) children by only prefetching the learner-selected subset (and introducing a side-effect-free prefetch API for item banks), with accompanying test coverage.

Changes:

• Added _children_for_field_data_cache() and updated FieldDataCache.add_block_descendents() to prefetch only selected children for dynamic blocks (and use a read-only API when available).
• Updated ItemBank/LegacyLibraryContent selection APIs to support a read_only mode and added get_child_blocks_for_prefetch() to avoid analytics/state side effects during prefetch.
• Added new unit tests covering dynamic-child prefetch behavior and ensuring read-only prefetch does not publish analytics or persist selection.

Reviewed changes

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

File	Description
lms/djangoapps/courseware/model_data.py	Introduces _children_for_field_data_cache() and switches descendant collection to use it (including read-only mode).
lms/djangoapps/courseware/tests/test_model_data.py	Adds tests asserting dynamic blocks only prefetch selected children and that read-only prefetch uses the side-effect-free API when present.
xmodule/item_bank_block.py	Adds read_only support to selection + introduces get_child_blocks_for_prefetch() for side-effect-free child retrieval.
xmodule/tests/test_item_bank.py	Adds a regression test ensuring read-only child prefetching does not publish analytics or persist selection.

Suppressed comments (1)

xmodule/item_bank_block.py:246

• The selected_children docstring is now more misleading: it says it returns “block_ids” and that it “reads and updates” user_state, but the method actually returns (block_type, block_id) pairs and, with the new read_only flag, it may avoid publishing analytics / persisting state. Please update the docstring to match the real return type and the read_only behavior (see make_selection docstring above, which documents the tuple structure).

    def selected_children(self, read_only=False):
        """
        Returns a [] of block_ids indicating which of the possible children
        have been selected to display to the current user.

        This reads and updates the "selected" field, which has user_state scope.

---

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

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (2)

xmodule/item_bank_block.py:270

• selected_children(read_only=True) still calls make_selection(...), which can randomly choose a new set of children when self.selected is empty or stale. Because the read-only path doesn’t persist the result, the blocks prefetched can differ from the blocks later rendered/persisted in non-read-only mode, defeating the goal of “prefetch only what will be rendered” and potentially causing unnecessary loads.

        max_count = self.max_count
        if max_count < 0:
            max_count = len(self.children)

        block_keys = self.make_selection(self.selected, self.children, max_count)  # pylint: disable=no-member
        selected = block_keys['selected']

        if not read_only:
            self.publish_selected_children_events(
                block_keys,
                self.format_block_keys_for_analytics,
                self._publish_event,
            )

        if not read_only and any(block_keys[changed] for changed in ('invalid', 'overlimit', 'added')):
            # Save our selections to the user state, to ensure consistency:
            self.selected = selected  # TODO: this doesn't save from the LMS "Progress" page.

        return selected if read_only else self.selected

xmodule/tests/test_item_bank.py:246

• This test asserts that get_child_blocks_for_prefetch() returns a selected child even when item_bank.selected is empty, while also asserting selection is not persisted. That combination can allow prefetch/render mismatches (prefetch may choose a random child that differs from the later persisted/rendered selection). Consider seeding the test with an existing persisted selection and asserting prefetch returns that selection without publishing or mutating state.

    def test_prefetch_child_blocks_does_not_publish_or_persist_selection(self):
        """
        Read-only child selection should avoid analytics events and user-state writes.
        """
        self._bind_course_block(self.item_bank)

        selected_children = self.item_bank.get_child_blocks_for_prefetch()

        assert len(selected_children) == 1
        self.publisher.assert_not_called()
        assert self.item_bank.selected == []