Skip to content

fix: apply library_content transformer to all ItemBankMixin xblocks [ulmo] #38296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kdmccormick merged 1 commit into from
Apr 8, 2026
+84 −36
Merged

fix: apply library_content transformer to all ItemBankMixin xblocks [ulmo] #38296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 44 additions & 20 deletions lms/djangoapps/course_blocks/transformers/library_content.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,24 @@
"""
Content Library Transformer.
Item Bank Transformer.

Transformers for handling item bank blocks (library_content, itembank, etc.)
that use ItemBankMixin for randomized content selection.
"""


import json
import logging

from eventtracking import tracker
from xblock.core import XBlock

from common.djangoapps.track import contexts
from lms.djangoapps.courseware.models import StudentModule
from openedx.core.djangoapps.content.block_structure.transformer import (
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
Expand All @@ -25,10 +29,13 @@
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
Expand Down Expand Up @@ -61,10 +68,10 @@ 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).
# If so add block analytics summary for each of its children.
for block_key in block_structure.topological_traversal(
filter_func=lambda block_key: block_key.block_type == 'library_content',
filter_func=lambda block_key: issubclass(XBlock.load_class(block_key.block_type), ItemBankMixin),
yield_descendants_of_unyielded=True,
):
xblock = block_structure.get_xblock(block_key)
Expand All @@ -76,7 +83,9 @@ 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':
block_class = XBlock.load_class(block_key.block_type)

if block_class is None or not issubclass(block_class, ItemBankMixin):
continue
library_children = block_structure.get_children(block_key)
if library_children:
Expand All @@ -98,7 +107,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 = XBlock.load_class(block_key.block_type)
if block_class is None:
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
Expand Down Expand Up @@ -128,7 +142,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:
Expand Down Expand Up @@ -156,6 +170,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 = XBlock.load_class(location.block_type)
if block_class is None:
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
Expand All @@ -170,11 +190,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,
Expand All @@ -184,12 +205,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
Expand Down Expand Up @@ -217,7 +240,8 @@ 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':
block_class = XBlock.load_class(block_key.block_type)
if block_class is None or not issubclass(block_class, ItemBankMixin):
continue

library_children = block_structure.get_children(block_key)
Expand All @@ -228,7 +252,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.
Expand Down
56 changes: 40 additions & 16 deletions lms/djangoapps/course_blocks/transformers/tests/test_library_content.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@

from unittest import mock

from ddt import data, ddt

import openedx.core.djangoapps.content.block_structure.api as bs_api
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory
from openedx.core.djangoapps.content.block_structure.api import clear_course_from_cache
from openedx.core.djangoapps.content.block_structure.transformers import BlockStructureTransformers

import openedx.core.djangoapps.content.block_structure.api as bs_api
from ...api import get_course_blocks
from ..library_content import ContentLibraryOrderTransformer, ContentLibraryTransformer
from .helpers import CourseStructureTestCase
Expand All @@ -26,6 +28,7 @@ def __init__(self, state):
self.state = state


@ddt
class ContentLibraryTransformerTestCase(CourseStructureTestCase):
"""
ContentLibraryTransformer Test
Expand All @@ -37,9 +40,14 @@ def setUp(self):
Setup course structure and create user for content library transformer test.
"""
super().setUp()
self._initialize_course_hierarchy()

def _initialize_course_hierarchy(self, block_type='library_content'):
"""
Initialize course hierarchy with the given block type.
"""
# Build course.
self.course_hierarchy = self.get_course_hierarchy()
self.course_hierarchy = self.get_course_hierarchy(block_type)
self.blocks = self.build_course(self.course_hierarchy)
self.course = self.blocks['course']
# Do this manually because publish signals are not fired by default in tests.
Expand All @@ -49,14 +57,14 @@ def setUp(self):
# Enroll user in course.
CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id, is_active=True)

def get_course_hierarchy(self):
def get_course_hierarchy(self, block_type='library_content'):
"""
Get a course hierarchy to test with.
"""
return [{
'org': 'ContentLibraryTransformer',
'course': 'CL101F',
'run': 'test_run',
'run': f'test_run_{block_type}',
'#type': 'course',
'#ref': 'course',
'#children': [
Expand All @@ -73,8 +81,8 @@ def get_course_hierarchy(self):
'#ref': 'vertical1',
'#children': [
{
'#type': 'library_content',
'#ref': 'library_content1',
'#type': block_type,
'#ref': f'{block_type}1',
'#children': [
{
'metadata': {'display_name': "CL Vertical 2"},
Expand Down Expand Up @@ -111,13 +119,18 @@ def get_course_hierarchy(self):
]
}]

def test_content_library(self):
@data('library_content', 'itembank')
def test_content_library(self, block_type):
"""
Test when course has content library section.
First test user can't see any content library section,
and after that mock response from MySQL db.
Check user can see mocked sections in content library.
"""
# Re-initialize if testing with a different block type
if block_type != 'library_content':
self._initialize_course_hierarchy(block_type)

raw_block_structure = get_course_blocks(
self.user,
self.course.location,
Expand All @@ -136,7 +149,7 @@ def test_content_library(self):
# Should dynamically assign a block to student
trans_keys = set(trans_block_structure.get_block_keys())
block_key_set = self.get_block_key_set(
self.blocks, 'course', 'chapter1', 'lesson1', 'vertical1', 'library_content1'
self.blocks, 'course', 'chapter1', 'lesson1', 'vertical1', f'{block_type}1'
)
for key in block_key_set:
assert key in trans_keys
Expand All @@ -160,11 +173,12 @@ def test_content_library(self):
assert set(trans_block_structure.get_block_keys()) == self.get_block_key_set(self.blocks, 'course',
'chapter1', 'lesson1',
'vertical1',
'library_content1',
f'{block_type}1',
selected_vertical,
selected_child), f"Expected 'selected' equality failed in iteration {i}." # pylint: disable=line-too-long


@ddt
class ContentLibraryOrderTransformerTestCase(CourseStructureTestCase):
"""
ContentLibraryOrderTransformer Test
Expand All @@ -176,7 +190,13 @@ def setUp(self):
Setup course structure and create user for content library order transformer test.
"""
super().setUp()
self.course_hierarchy = self.get_course_hierarchy()
self._initialize_course_hierarchy()

def _initialize_course_hierarchy(self, block_type='library_content'):
"""
Initialize course hierarchy with the given block type.
"""
self.course_hierarchy = self.get_course_hierarchy(block_type)
self.blocks = self.build_course(self.course_hierarchy)
self.course = self.blocks['course']
bs_api.update_course_in_cache(self.course.id)
Expand All @@ -185,14 +205,14 @@ def setUp(self):
# Enroll user in course.
CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id, is_active=True)

def get_course_hierarchy(self):
def get_course_hierarchy(self, block_type='library_content'):
"""
Get a course hierarchy to test with.
"""
return [{
'org': 'ContentLibraryTransformer',
'course': 'CL101F',
'run': 'test_run',
'run': f'test_run_{block_type}',
'#type': 'course',
'#ref': 'course',
'#children': [
Expand All @@ -209,8 +229,8 @@ def get_course_hierarchy(self):
'#ref': 'vertical1',
'#children': [
{
'#type': 'library_content',
'#ref': 'library_content1',
'#type': block_type,
'#ref': f'{block_type}1',
'#children': [
{
'metadata': {'display_name': "CL Vertical 2"},
Expand Down Expand Up @@ -260,11 +280,15 @@ def get_course_hierarchy(self):
}]

@mock.patch('lms.djangoapps.course_blocks.transformers.library_content.get_student_module_as_dict')
def test_content_library_randomize(self, mocked):
@data('library_content', 'itembank')
def test_content_library_randomize(self, block_type, mocked):
"""
Test whether the order of the children blocks matches the order of the selected blocks when
course has content library section
"""
# Re-initialize if testing with a different block type
if block_type != 'library_content':
self._initialize_course_hierarchy(block_type)
mocked.return_value = {
'selected': [
['vertical', 'vertical_vertical3'],
Expand All @@ -280,7 +304,7 @@ def test_content_library_randomize(self, mocked):
)
children = []
for block_key in trans_block_structure.topological_traversal():
if block_key.block_type == 'library_content':
if block_key.block_type == block_type:
children = trans_block_structure.get_children(block_key)
break

Expand Down
Loading