From 076acc899d11a7079412189ea5af0d4872687683 Mon Sep 17 00:00:00 2001 From: Maria Fernanda Magallanes Zubillaga Date: Wed, 31 Dec 2025 14:54:16 -0500 Subject: [PATCH 1/2] fix: apply library_content transformer to all ItemBankMixin xblocks --- .../transformers/library_content.py | 93 ++++++++++++++----- 1 file changed, 69 insertions(+), 24 deletions(-) diff --git a/lms/djangoapps/course_blocks/transformers/library_content.py b/lms/djangoapps/course_blocks/transformers/library_content.py index 10ef8c2138b6..2521d1340fc0 100644 --- a/lms/djangoapps/course_blocks/transformers/library_content.py +++ b/lms/djangoapps/course_blocks/transformers/library_content.py @@ -1,5 +1,8 @@ """ -Content Library Transformer. +Item Bank Transformer. + +Transformers for handling item bank blocks (library_content, itembank, etc.) +that use ItemBankMixin for randomized content selection. """ @@ -7,6 +10,7 @@ import logging from eventtracking import tracker +from xblock.core import XBlock from common.djangoapps.track import contexts from lms.djangoapps.courseware.models import StudentModule @@ -14,21 +18,40 @@ BlockStructureTransformer, FilteringTransformerMixin ) -from xmodule.library_content_block import LegacyLibraryContentBlock # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.item_bank_block import ItemBankMixin # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from ..utils import get_student_module_as_dict logger = logging.getLogger(__name__) +# Module-level cache to avoid repeated XBlock.load_class() calls +_BLOCK_CLASS_CACHE = {} + + +def _get_block_class(block_type: str): + """ + Get the XBlock class from a block type. + """ + if block_type not in _BLOCK_CLASS_CACHE: + try: + _BLOCK_CLASS_CACHE[block_type] = XBlock.load_class(block_type) + except Exception: # lint-amnesty, pylint: disable=broad-except + logger.exception('Failed to load block class for type %s', block_type) + _BLOCK_CLASS_CACHE[block_type] = None + return _BLOCK_CLASS_CACHE[block_type] + class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransformer): """ A transformer that manipulates the block structure by removing all - blocks within a library_content block to which a user should not - have access. + blocks within item bank blocks (library_content, itembank, etc.) + to which a user should not have access. + + This transformer works with any XBlock that inherits from ItemBankMixin, + filtering children based on the selection logic defined by each block type. - Staff users are not to be exempted from library content pathways. + Staff users are not to be exempted from item bank pathways. """ WRITE_VERSION = 1 READ_VERSION = 1 @@ -61,22 +84,27 @@ def summarize_block(usage_key): "original_usage_version": str(orig_version) if orig_version else None, } - # For each block check if block is library_content. - # If library_content add children array to content_library_children field + # For each block check if block uses ItemBankMixin (e.g., library_content, itembank). + # Mark these blocks and collect analytics data for their children. for block_key in block_structure.topological_traversal( - filter_func=lambda block_key: block_key.block_type == 'library_content', yield_descendants_of_unyielded=True, ): - xblock = block_structure.get_xblock(block_key) - for child_key in xblock.children: - summary = summarize_block(child_key) - block_structure.set_transformer_block_field(child_key, cls, 'block_analytics_summary', summary) + block_class = _get_block_class(block_key.block_type) + is_item_bank = block_class and issubclass(block_class, ItemBankMixin) + + if is_item_bank: + block_structure.set_transformer_block_field(block_key, cls, 'is_item_bank_block', True) + xblock = block_structure.get_xblock(block_key) + for child_key in xblock.children: + summary = summarize_block(child_key) + block_structure.set_transformer_block_field(child_key, cls, 'block_analytics_summary', summary) def transform_block_filters(self, usage_info, block_structure): all_library_children = set() all_selected_children = set() for block_key in block_structure: - if block_key.block_type != 'library_content': + # Check if this block was marked as an ItemBankMixin block during collect + if not block_structure.get_transformer_block_field(block_key, self, 'is_item_bank_block'): continue library_children = block_structure.get_children(block_key) if library_children: @@ -98,7 +126,12 @@ def transform_block_filters(self, usage_info, block_structure): # Update selected previous_count = len(selected) - block_keys = LegacyLibraryContentBlock.make_selection(selected, library_children, max_count) + # Get the cached block class to call make_selection + block_class = _get_block_class(block_key.block_type) + if not block_class: + logger.error('Failed to load block class for %s', block_key) + continue + block_keys = block_class.make_selection(selected, library_children, max_count) selected = block_keys['selected'] # Save back any changes @@ -128,7 +161,7 @@ def check_child_removal(block_key): """ Return True if selected block should be removed. - Block is removed if it is part of library_content, but has + Block is removed if it is a child of an item bank block, but has not been selected for current user. """ if block_key not in all_library_children: @@ -156,6 +189,12 @@ def format_block_keys(keys): json_result.append(info) return json_result + # Get the cached block class to call publish_selected_children_events + block_class = _get_block_class(location.block_type) + if not block_class: + logger.error('Failed to load block class for publishing events: %s', location) + return + def publish_event(event_name, result, **kwargs): """ Helper function to publish an event for analytics purposes @@ -170,11 +209,12 @@ def publish_event(event_name, result, **kwargs): context = contexts.course_context_from_course_id(location.course_key) if user_id: context['user_id'] = user_id - full_event_name = f"edx.librarycontentblock.content.{event_name}" + event_prefix = block_class.get_selected_event_prefix() + full_event_name = f"{event_prefix}.{event_name}" with tracker.get_tracker().context(full_event_name, context): tracker.emit(full_event_name, event_data) - LegacyLibraryContentBlock.publish_selected_children_events( + block_class.publish_selected_children_events( block_keys, format_block_keys, publish_event, @@ -184,12 +224,14 @@ def publish_event(event_name, result, **kwargs): class ContentLibraryOrderTransformer(BlockStructureTransformer): """ A transformer that manipulates the block structure by modifying the order of the - selected blocks within a library_content block to match the order of the selections - made by the ContentLibraryTransformer or the corresponding XBlock. So this transformer - requires the selections for the randomized content block to be already - made either by the ContentLibraryTransformer or the XBlock. + selected blocks within item bank blocks (library_content, itembank, etc.) + to match the order of the selections made by the ContentLibraryTransformer or the + corresponding XBlock. This transformer requires the selections for the item bank block + to be already made either by the ContentLibraryTransformer or the XBlock. + + This transformer works with any XBlock that inherits from ItemBankMixin. - Staff users are *not* exempted from library content pathways. + Staff users are *not* exempted from item bank pathways. """ WRITE_VERSION = 1 READ_VERSION = 1 @@ -217,7 +259,10 @@ def transform(self, usage_info, block_structure): to match the order of the selections made and stored in the XBlock 'selected' field. """ for block_key in block_structure: - if block_key.block_type != 'library_content': + # Check if this block was marked as an ItemBankMixin block by ContentLibraryTransformer + if not block_structure.get_transformer_block_field( + block_key, ContentLibraryTransformer, 'is_item_bank_block' + ): continue library_children = block_structure.get_children(block_key) @@ -228,7 +273,7 @@ def transform(self, usage_info, block_structure): current_selected_blocks = {item[1] for item in state_dict.get('selected', [])} # As the selections should have already been made by the ContentLibraryTransformer, - # the current children of the library_content block should be the same as the stored + # the current children of the item bank block should be the same as the stored # selections. If they aren't, some other transformer that ran before this transformer # has modified those blocks (for example, content gating may have affected this). So do not # transform the order in that case. From 8090f8a4b0fdd3f6c4fca30b8d901fc2621a526e Mon Sep 17 00:00:00 2001 From: Maria Fernanda Magallanes Zubillaga Date: Fri, 2 Jan 2026 12:10:19 -0500 Subject: [PATCH 2/2] fix: update the write version and add fallback for old cache --- .../transformers/library_content.py | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/lms/djangoapps/course_blocks/transformers/library_content.py b/lms/djangoapps/course_blocks/transformers/library_content.py index 2521d1340fc0..925d49ecb692 100644 --- a/lms/djangoapps/course_blocks/transformers/library_content.py +++ b/lms/djangoapps/course_blocks/transformers/library_content.py @@ -53,7 +53,7 @@ class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransfo Staff users are not to be exempted from item bank pathways. """ - WRITE_VERSION = 1 + WRITE_VERSION = 2 READ_VERSION = 1 @classmethod @@ -104,7 +104,13 @@ def transform_block_filters(self, usage_info, block_structure): all_selected_children = set() for block_key in block_structure: # Check if this block was marked as an ItemBankMixin block during collect - if not block_structure.get_transformer_block_field(block_key, self, 'is_item_bank_block'): + is_item_bank = block_structure.get_transformer_block_field(block_key, self, 'is_item_bank_block') + # Fallback for old cache that doesn't have the 'is_item_bank_block' field + if is_item_bank is None: + block_class = _get_block_class(block_key.block_type) + is_item_bank = block_class and issubclass(block_class, ItemBankMixin) + + if not is_item_bank: continue library_children = block_structure.get_children(block_key) if library_children: @@ -259,10 +265,19 @@ def transform(self, usage_info, block_structure): to match the order of the selections made and stored in the XBlock 'selected' field. """ for block_key in block_structure: - # Check if this block was marked as an ItemBankMixin block by ContentLibraryTransformer - if not block_structure.get_transformer_block_field( - block_key, ContentLibraryTransformer, 'is_item_bank_block' - ): + # Try to read from cache (collected by ContentLibraryTransformer) + is_item_bank = block_structure.get_transformer_block_field( + block_key, + ContentLibraryTransformer, + 'is_item_bank_block' + ) + + # Fallback for old cache that doesn't have the 'is_item_bank_block' field + if is_item_bank is None: + block_class = _get_block_class(block_key.block_type) + is_item_bank = block_class and issubclass(block_class, ItemBankMixin) + + if not is_item_bank: continue library_children = block_structure.get_children(block_key)