Skip to content

Commit b082f49

Browse files
committed
fix: apply library_content transformer to all ItemBankMixin xblocks
(cherry picked from commit 076acc8) fix: update the write version and add fallback for old cache (cherry picked from commit 8090f8a)
1 parent 314cd1e commit b082f49

File tree

1 file changed

+85
-25
lines changed

1 file changed

+85
-25
lines changed

lms/djangoapps/course_blocks/transformers/library_content.py

Lines changed: 85 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,33 @@ 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+
is_item_bank = block_structure.get_transformer_block_field(block_key, self, 'is_item_bank_block')
108+
# Fallback for old cache that doesn't have the 'is_item_bank_block' field
109+
if is_item_bank is None:
110+
block_class = _get_block_class(block_key.block_type)
111+
is_item_bank = block_class and issubclass(block_class, ItemBankMixin)
112+
113+
if not is_item_bank:
80114
continue
81115
library_children = block_structure.get_children(block_key)
82116
if library_children:
@@ -98,7 +132,12 @@ def transform_block_filters(self, usage_info, block_structure):
98132

99133
# Update selected
100134
previous_count = len(selected)
101-
block_keys = LegacyLibraryContentBlock.make_selection(selected, library_children, max_count)
135+
# Get the cached block class to call make_selection
136+
block_class = _get_block_class(block_key.block_type)
137+
if not block_class:
138+
logger.error('Failed to load block class for %s', block_key)
139+
continue
140+
block_keys = block_class.make_selection(selected, library_children, max_count)
102141
selected = block_keys['selected']
103142

104143
# Save back any changes
@@ -128,7 +167,7 @@ def check_child_removal(block_key):
128167
"""
129168
Return True if selected block should be removed.
130169
131-
Block is removed if it is part of library_content, but has
170+
Block is removed if it is a child of an item bank block, but has
132171
not been selected for current user.
133172
"""
134173
if block_key not in all_library_children:
@@ -156,6 +195,12 @@ def format_block_keys(keys):
156195
json_result.append(info)
157196
return json_result
158197

198+
# Get the cached block class to call publish_selected_children_events
199+
block_class = _get_block_class(location.block_type)
200+
if not block_class:
201+
logger.error('Failed to load block class for publishing events: %s', location)
202+
return
203+
159204
def publish_event(event_name, result, **kwargs):
160205
"""
161206
Helper function to publish an event for analytics purposes
@@ -170,11 +215,12 @@ def publish_event(event_name, result, **kwargs):
170215
context = contexts.course_context_from_course_id(location.course_key)
171216
if user_id:
172217
context['user_id'] = user_id
173-
full_event_name = f"edx.librarycontentblock.content.{event_name}"
218+
event_prefix = block_class.get_selected_event_prefix()
219+
full_event_name = f"{event_prefix}.{event_name}"
174220
with tracker.get_tracker().context(full_event_name, context):
175221
tracker.emit(full_event_name, event_data)
176222

177-
LegacyLibraryContentBlock.publish_selected_children_events(
223+
block_class.publish_selected_children_events(
178224
block_keys,
179225
format_block_keys,
180226
publish_event,
@@ -184,12 +230,14 @@ def publish_event(event_name, result, **kwargs):
184230
class ContentLibraryOrderTransformer(BlockStructureTransformer):
185231
"""
186232
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.
233+
selected blocks within item bank blocks (library_content, itembank, etc.)
234+
to match the order of the selections made by the ContentLibraryTransformer or the
235+
corresponding XBlock. This transformer requires the selections for the item bank block
236+
to be already made either by the ContentLibraryTransformer or the XBlock.
237+
238+
This transformer works with any XBlock that inherits from ItemBankMixin.
191239
192-
Staff users are *not* exempted from library content pathways.
240+
Staff users are *not* exempted from item bank pathways.
193241
"""
194242
WRITE_VERSION = 1
195243
READ_VERSION = 1
@@ -217,7 +265,19 @@ def transform(self, usage_info, block_structure):
217265
to match the order of the selections made and stored in the XBlock 'selected' field.
218266
"""
219267
for block_key in block_structure:
220-
if block_key.block_type != 'library_content':
268+
# Try to read from cache (collected by ContentLibraryTransformer)
269+
is_item_bank = block_structure.get_transformer_block_field(
270+
block_key,
271+
ContentLibraryTransformer,
272+
'is_item_bank_block'
273+
)
274+
275+
# Fallback for old cache that doesn't have the 'is_item_bank_block' field
276+
if is_item_bank is None:
277+
block_class = _get_block_class(block_key.block_type)
278+
is_item_bank = block_class and issubclass(block_class, ItemBankMixin)
279+
280+
if not is_item_bank:
221281
continue
222282

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

230290
# 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
291+
# the current children of the item bank block should be the same as the stored
232292
# selections. If they aren't, some other transformer that ran before this transformer
233293
# has modified those blocks (for example, content gating may have affected this). So do not
234294
# transform the order in that case.

0 commit comments

Comments
 (0)