Andal.LND/edx-platform
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix the order randomization behaviour of Randomized Content Block

Browse Source 
The Randomized Content Block XBlock only randomizes the selection of
the children blocks and has unpredictable randomization of
the order of the selected child blocks due to the usage of sets, which
are unordered, for storing the selected blocks. This becomes apparent
when all the available child blocks in a library are chosen for a
Randomized Content Block, to randomize just the order of the child
blocks and not just the selection of the blocks. The order of the
selected blocks ends up being similar for multiple learners.

This change modifies the XBlock to store the selected child blocks in
a list, instead of a set, after randomly shuffling them.

A new block structure transformer, ContentLibraryOrderTransformer has
been added to transform the order of the selected children XBlocks to
match the order of the selections made in the
ContentLibraryTransformer. This ensures that same correct randomized order
of blocks is also returned by the course blocks API.
This commit is contained in:
Guruprasad Lakshmi Narayanan
2020-08-06 12:58:19 +05:30
committed by Kyle McCormick
parent 8795bb8c82
commit 149ebfec58
6 changed files with 278 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
lms/djangoapps/course_blocks/api.py
View File

@@ -40,6 +40,7 @@ def get_course_block_access_transformers(user):
"""
course_block_access_transformers = [
library_content.ContentLibraryTransformer(),
library_content.ContentLibraryOrderTransformer(),
start_date.StartDateTransformer(),
ContentTypeGateTransformer(),
user_partitions.UserPartitionTransformer(),

68
lms/djangoapps/course_blocks/transformers/library_content.py
View File

@@ -4,6 +4,7 @@ Content Library Transformer.
import json
import logging
import six
from eventtracking import tracker
@@ -19,6 +20,8 @@ from xmodule.modulestore.django import modulestore
from ..utils import get_student_module_as_dict
logger = logging.getLogger(__name__)
class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransformer):
"""
@@ -101,7 +104,7 @@ class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransfo
# Save back any changes
if any(block_keys[changed] for changed in ('invalid', 'overlimit', 'added')):
state_dict['selected'] = list(selected)
state_dict['selected'] = selected
StudentModule.save_state(
student=usage_info.user,
course_id=usage_info.course_key,
@@ -177,3 +180,66 @@ class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransfo
format_block_keys,
publish_event,
)
class ContentLibraryOrderTransformer(BlockStructureTransformer):
"""
A transformer that manipulates the block structure by modifying the order of the
selected blocks within a library_content module 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.
Staff users are *not* exempted from library content pathways.
"""
WRITE_VERSION = 1
READ_VERSION = 1
@classmethod
def name(cls):
"""
Unique identifier for the transformer's class;
same identifier used in setup.py
"""
return "library_content_randomize"
@classmethod
def collect(cls, block_structure):
"""
Collects any information that's necessary to execute this
transformer's transform method.
"""
# There is nothing to collect
pass # pylint:disable=unnecessary-pass
def transform(self, usage_info, block_structure):
"""
Transforms the order of the children of the randomized content block
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':
continue
library_children = block_structure.get_children(block_key)
if library_children:
state_dict = get_student_module_as_dict(usage_info.user, usage_info.course_key, block_key)
current_children_blocks = set(block.block_id for block in library_children)
current_selected_blocks = set(item[1] for item in state_dict['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
# 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.
if current_children_blocks != current_selected_blocks:
logger.info(
u'Mismatch between the children of %s in the stored state and the actual children for user %s. '
'Continuing without order transformation.',
str(block_key),
usage_info.user.username
)
else:
ordering_data = {block[1]: position for position, block in enumerate(state_dict['selected'])}
library_children.sort(key=lambda block, data=ordering_data: data[block.block_id])

183
lms/djangoapps/course_blocks/transformers/tests/test_library_content.py
View File

@@ -4,13 +4,14 @@ Tests for ContentLibraryTransformer.
from six.moves import range
import mock
from openedx.core.djangoapps.content.block_structure.api import clear_course_from_cache
from openedx.core.djangoapps.content.block_structure.transformers import BlockStructureTransformers
from student.tests.factories import CourseEnrollmentFactory
from ...api import get_course_blocks
from ..library_content import ContentLibraryTransformer
from ..library_content import ContentLibraryTransformer, ContentLibraryOrderTransformer
from .helpers import CourseStructureTestCase
@@ -167,3 +168,183 @@ class ContentLibraryTransformerTestCase(CourseStructureTestCase):
),
u"Expected 'selected' equality failed in iteration {}.".format(i)
)
class ContentLibraryOrderTransformerTestCase(CourseStructureTestCase):
"""
ContentLibraryOrderTransformer Test
"""
TRANSFORMER_CLASS_TO_TEST = ContentLibraryOrderTransformer
def setUp(self):
"""
Setup course structure and create user for content library order transformer test.
"""
super(ContentLibraryOrderTransformerTestCase, self).setUp()
self.course_hierarchy = self.get_course_hierarchy()
self.blocks = self.build_course(self.course_hierarchy)
self.course = self.blocks['course']
clear_course_from_cache(self.course.id)
# Enroll user in course.
CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id, is_active=True)
def get_course_hierarchy(self):
"""
Get a course hierarchy to test with.
"""
return [{
'org': 'ContentLibraryTransformer',
'course': 'CL101F',
'run': 'test_run',
'#type': 'course',
'#ref': 'course',
'#children': [
{
'#type': 'chapter',
'#ref': 'chapter1',
'#children': [
{
'#type': 'sequential',
'#ref': 'lesson1',
'#children': [
{
'#type': 'vertical',
'#ref': 'vertical1',
'#children': [
{
'metadata': {'category': 'library_content'},
'#type': 'library_content',
'#ref': 'library_content1',
'#children': [
{
'metadata': {'display_name': "CL Vertical 2"},
'#type': 'vertical',
'#ref': 'vertical2',
'#children': [
{
'metadata': {'display_name': "HTML1"},
'#type': 'html',
'#ref': 'html1',
}
]
},
{
'metadata': {'display_name': "CL Vertical 3"},
'#type': 'vertical',
'#ref': 'vertical3',
'#children': [
{
'metadata': {'display_name': "HTML2"},
'#type': 'html',
'#ref': 'html2',
}
]
},
{
'metadata': {'display_name': "CL Vertical 4"},
'#type': 'vertical',
'#ref': 'vertical4',
'#children': [
{
'metadata': {'display_name': "HTML3"},
'#type': 'html',
'#ref': 'html3',
}
]
}
]
}
],
}
],
}
],
}
]
}]
@mock.patch('lms.djangoapps.course_blocks.transformers.library_content.get_student_module_as_dict')
def test_content_library_randomize(self, mocked):
"""
Test whether the order of the children blocks matches the order of the selected blocks when
course has content library section
"""
mocked.return_value = {
'selected': [
['vertical', 'vertical_vertical3'],
['vertical', 'vertical_vertical2'],
['vertical', 'vertical_vertical4'],
]
}
for i in range(5):
trans_block_structure = get_course_blocks(
self.user,
self.course.location,
self.transformers,
)
children = []
for block_key in trans_block_structure.topological_traversal():
if block_key.block_type == 'library_content':
children = trans_block_structure.get_children(block_key)
break
expected_children = ['vertical_vertical3', 'vertical_vertical2', 'vertical_vertical4']
self.assertEqual(
expected_children,
[child.block_id for child in children],
u"Expected 'selected' equality failed in iteration {}.".format(i)
)
@mock.patch('lms.djangoapps.course_blocks.transformers.library_content.get_student_module_as_dict')
def test_content_library_randomize_selected_blocks_mismatch(self, mocked):
"""
Test and verify that the ContentLibraryOrderTransformer's order transformation doesn't
happen when the current children blocks no longer match the selections made in the ContentLibraryTransformer
and stored in the database.
There are two types of block structure transformers - filtering and non-filtering. The filtering transformers
can only filter blocks from the block structure but cannot perform any other transformations. The
non-filtering transformers can transform the blocks in the block structure.
The ContentLibraryTransformer is a filtering transformer that selects the children blocks of the randomized
content blocks, saves the selection in the database and filters the blocks that are not selected.
The ContentLibraryOrderTransformer is a non-filtering transformer which transforms the order of the children
blocks of the randomized content block based on the order of the stored selection made by the
ContentLibraryTransformer.
The 'transform()' methods of all the filtering transformers are combined into a single transformation function
and run in a single block structure traversal. The non-filtering block structure transformers are run
after this.
When some filtering transformers like those for content visibility, gating etc. run after the
ContentLibraryTransformer and remove some/all of the selected children blocks before the
ContentLibraryTransformer runs, there will be a mismatch between the stored selected children blocks and
the current children blocks. When this happens, the ContentLibraryOrderTransformer shouldn't
transform the order.
"""
mocked.return_value = {
'selected': [
['vertical', 'vertical_vertical3'],
]
}
expected_children_without_hiding_or_gating = ['vertical_vertical3', ]
for _ in range(5):
trans_block_structure = get_course_blocks(
self.user,
self.course.location,
self.transformers,
)
children = []
for block_key in trans_block_structure.topological_traversal():
if block_key.block_type == 'library_content':
children = trans_block_structure.get_children(block_key)
break
self.assertNotEqual(
expected_children_without_hiding_or_gating,
[child.block_id for child in children],
)