Skip to content

Commit 6d3dfc4

Browse files
committed
fix: apply library_content transformer to all ItemBankMixin xblocks
(cherry picked from commit c5ba526) fix: update the write version and add fallback for old cache (cherry picked from commit 2682a9a)
1 parent 314cd1e commit 6d3dfc4

File tree

1 file changed

+79
-25
lines changed

1 file changed

+79
-25
lines changed

lms/djangoapps/course_blocks/transformers/library_content.py

Lines changed: 79 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,59 @@
11
"""
2-
Content Library Transformer.
2+
Item Bank Transformer.
3+
4+
Transformers for handling item bank blocks (library_content, itembank, etc.)
5+
that use ItemBankMixin for randomized content selection.
36
"""
47

58

69
import json
710
import logging
811

912
from eventtracking import tracker
13+
from xblock.core import XBlock
1014

1115
from common.djangoapps.track import contexts
1216
from lms.djangoapps.courseware.models import StudentModule
1317
from openedx.core.djangoapps.content.block_structure.transformer import (
1418
BlockStructureTransformer,
1519
FilteringTransformerMixin
1620
)
17-
from xmodule.library_content_block import LegacyLibraryContentBlock # lint-amnesty, pylint: disable=wrong-import-order
21+
from xmodule.item_bank_block import ItemBankMixin # lint-amnesty, pylint: disable=wrong-import-order
1822
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
1923

2024
from ..utils import get_student_module_as_dict
2125

2226
logger = logging.getLogger(__name__)
2327

28+
# Module-level cache to avoid repeated XBlock.load_class() calls
29+
_BLOCK_CLASS_CACHE = {}
30+
31+
32+
def _get_block_class(block_type: str):
33+
"""
34+
Get the XBlock class from a block type.
35+
"""
36+
if block_type not in _BLOCK_CLASS_CACHE:
37+
try:
38+
_BLOCK_CLASS_CACHE[block_type] = XBlock.load_class(block_type)
39+
except Exception: # lint-amnesty, pylint: disable=broad-except
40+
logger.exception('Failed to load block class for type %s', block_type)
41+
_BLOCK_CLASS_CACHE[block_type] = None
42+
return _BLOCK_CLASS_CACHE[block_type]
43+
2444

2545
class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransformer):
2646
"""
2747
A transformer that manipulates the block structure by removing all
28-
blocks within a library_content block to which a user should not
29-
have access.
48+
blocks within item bank blocks (library_content, itembank, etc.)
49+
to which a user should not have access.
3050
31-
Staff users are not to be exempted from library content pathways.
51+
This transformer works with any XBlock that inherits from ItemBankMixin,
52+
filtering children based on the selection logic defined by each block type.
53+
54+
Staff users are not to be exempted from item bank pathways.
3255
"""
33-
WRITE_VERSION = 1
56+
WRITE_VERSION = 2
3457
READ_VERSION = 1
3558

3659
@classmethod
@@ -61,22 +84,27 @@ def summarize_block(usage_key):
6184
"original_usage_version": str(orig_version) if orig_version else None,
6285
}
6386

64-
# For each block check if block is library_content.
65-
# If library_content add children array to content_library_children field
87+
# For each block check if block uses ItemBankMixin (e.g., library_content, itembank).
88+
# Mark these blocks and collect analytics data for their children.
6689
for block_key in block_structure.topological_traversal(
67-
filter_func=lambda block_key: block_key.block_type == 'library_content',
6890
yield_descendants_of_unyielded=True,
6991
):
70-
xblock = block_structure.get_xblock(block_key)
71-
for child_key in xblock.children:
72-
summary = summarize_block(child_key)
73-
block_structure.set_transformer_block_field(child_key, cls, 'block_analytics_summary', summary)
92+
block_class = _get_block_class(block_key.block_type)
93+
is_item_bank = block_class and issubclass(block_class, ItemBankMixin)
94+
95+
if is_item_bank:
96+
block_structure.set_transformer_block_field(block_key, cls, 'is_item_bank_block', True)
97+
xblock = block_structure.get_xblock(block_key)
98+
for child_key in xblock.children:
99+
summary = summarize_block(child_key)
100+
block_structure.set_transformer_block_field(child_key, cls, 'block_analytics_summary', summary)
74101

75102
def transform_block_filters(self, usage_info, block_structure):
76103
all_library_children = set()
77104
all_selected_children = set()
78105
for block_key in block_structure:
79-
if block_key.block_type != 'library_content':
106+
# Check if this block was marked as an ItemBankMixin block during collect
107+
if not block_structure.get_transformer_block_field(block_key, self, 'is_item_bank_block'):
80108
continue
81109
library_children = block_structure.get_children(block_key)
82110
if library_children:
@@ -98,7 +126,12 @@ def transform_block_filters(self, usage_info, block_structure):
98126

99127
# Update selected
100128
previous_count = len(selected)
101-
block_keys = LegacyLibraryContentBlock.make_selection(selected, library_children, max_count)
129+
# Get the cached block class to call make_selection
130+
block_class = _get_block_class(block_key.block_type)
131+
if not block_class:
132+
logger.error('Failed to load block class for %s', block_key)
133+
continue
134+
block_keys = block_class.make_selection(selected, library_children, max_count)
102135
selected = block_keys['selected']
103136

104137
# Save back any changes
@@ -128,7 +161,7 @@ def check_child_removal(block_key):
128161
"""
129162
Return True if selected block should be removed.
130163
131-
Block is removed if it is part of library_content, but has
164+
Block is removed if it is a child of an item bank block, but has
132165
not been selected for current user.
133166
"""
134167
if block_key not in all_library_children:
@@ -156,6 +189,12 @@ def format_block_keys(keys):
156189
json_result.append(info)
157190
return json_result
158191

192+
# Get the cached block class to call publish_selected_children_events
193+
block_class = _get_block_class(location.block_type)
194+
if not block_class:
195+
logger.error('Failed to load block class for publishing events: %s', location)
196+
return
197+
159198
def publish_event(event_name, result, **kwargs):
160199
"""
161200
Helper function to publish an event for analytics purposes
@@ -170,11 +209,12 @@ def publish_event(event_name, result, **kwargs):
170209
context = contexts.course_context_from_course_id(location.course_key)
171210
if user_id:
172211
context['user_id'] = user_id
173-
full_event_name = f"edx.librarycontentblock.content.{event_name}"
212+
event_prefix = block_class.get_selected_event_prefix()
213+
full_event_name = f"{event_prefix}.{event_name}"
174214
with tracker.get_tracker().context(full_event_name, context):
175215
tracker.emit(full_event_name, event_data)
176216

177-
LegacyLibraryContentBlock.publish_selected_children_events(
217+
block_class.publish_selected_children_events(
178218
block_keys,
179219
format_block_keys,
180220
publish_event,
@@ -184,12 +224,14 @@ def publish_event(event_name, result, **kwargs):
184224
class ContentLibraryOrderTransformer(BlockStructureTransformer):
185225
"""
186226
A transformer that manipulates the block structure by modifying the order of the
187-
selected blocks within a library_content block to match the order of the selections
188-
made by the ContentLibraryTransformer or the corresponding XBlock. So this transformer
189-
requires the selections for the randomized content block to be already
190-
made either by the ContentLibraryTransformer or the XBlock.
227+
selected blocks within item bank blocks (library_content, itembank, etc.)
228+
to match the order of the selections made by the ContentLibraryTransformer or the
229+
corresponding XBlock. This transformer requires the selections for the item bank block
230+
to be already made either by the ContentLibraryTransformer or the XBlock.
231+
232+
This transformer works with any XBlock that inherits from ItemBankMixin.
191233
192-
Staff users are *not* exempted from library content pathways.
234+
Staff users are *not* exempted from item bank pathways.
193235
"""
194236
WRITE_VERSION = 1
195237
READ_VERSION = 1
@@ -217,7 +259,19 @@ def transform(self, usage_info, block_structure):
217259
to match the order of the selections made and stored in the XBlock 'selected' field.
218260
"""
219261
for block_key in block_structure:
220-
if block_key.block_type != 'library_content':
262+
# Try to read from cache (collected by ContentLibraryTransformer)
263+
is_item_bank = block_structure.get_transformer_block_field(
264+
block_key,
265+
ContentLibraryTransformer,
266+
'is_item_bank_block'
267+
)
268+
269+
# Fallback for old cache that doesn't have the 'is_item_bank_block' field
270+
if is_item_bank is None:
271+
block_class = _get_block_class(block_key.block_type)
272+
is_item_bank = block_class and issubclass(block_class, ItemBankMixin)
273+
274+
if not is_item_bank:
221275
continue
222276

223277
library_children = block_structure.get_children(block_key)
@@ -228,7 +282,7 @@ def transform(self, usage_info, block_structure):
228282
current_selected_blocks = {item[1] for item in state_dict.get('selected', [])}
229283

230284
# As the selections should have already been made by the ContentLibraryTransformer,
231-
# the current children of the library_content block should be the same as the stored
285+
# the current children of the item bank block should be the same as the stored
232286
# selections. If they aren't, some other transformer that ran before this transformer
233287
# has modified those blocks (for example, content gating may have affected this). So do not
234288
# transform the order in that case.

0 commit comments

Comments
 (0)