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

Revert "Fix the unpredictable order randomization issue with randomized content blocks"

Browse Source
This commit is contained in:
David Ormsbee
2019-07-25 13:42:20 -04:00
committed by GitHub
parent e1b9869b26
commit b1f3d2cef2
6 changed files with 50 additions and 260 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -4,8 +4,6 @@ Content Library Transformer.
from __future__ import absolute_import
import json
import logging
import random
import six
from eventtracking import tracker
@@ -21,8 +19,6 @@ from xmodule.modulestore.django import modulestore
from ..utils import get_student_module_as_dict
logger = logging.getLogger(__name__)
class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransformer):
"""
@@ -44,15 +40,24 @@ class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransfo
return "library_content"
@classmethod
def set_block_analytics_summary(cls, block_structure, store):
"""Set the block analytics summary information in the children fields."""
def collect(cls, block_structure):
"""
Collects any information that's necessary to execute this
transformer's transform method.
"""
block_structure.request_xblock_fields('mode')
block_structure.request_xblock_fields('max_count')
block_structure.request_xblock_fields('category')
store = modulestore()
# needed for analytics purposes
def summarize_block(usage_key):
""" Basic information about the given block """
orig_key, orig_version = store.get_block_original_usage(usage_key)
return {
"usage_key": unicode(usage_key),
"original_usage_key": unicode(orig_key) if orig_key else None,
"original_usage_version": unicode(orig_version) if orig_version else None,
"usage_key": six.text_type(usage_key),
"original_usage_key": six.text_type(orig_key) if orig_key else None,
"original_usage_version": six.text_type(orig_version) if orig_version else None,
}
# For each block check if block is library_content.
@@ -66,20 +71,6 @@ class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransfo
summary = summarize_block(child_key)
block_structure.set_transformer_block_field(child_key, cls, 'block_analytics_summary', summary)
@classmethod
def collect(cls, block_structure):
"""
Collects any information that's necessary to execute this
transformer's transform method.
"""
block_structure.request_xblock_fields('mode')
block_structure.request_xblock_fields('max_count')
block_structure.request_xblock_fields('category')
store = modulestore()
# needed for analytics purposes
cls.set_block_analytics_summary(block_structure, store)
def transform_block_filters(self, usage_info, block_structure):
all_library_children = set()
all_selected_children = set()
@@ -111,7 +102,6 @@ class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransfo
# Save back any changes
if any(block_keys[changed] for changed in ('invalid', 'overlimit', 'added')):
state_dict['selected'] = list(selected)
random.shuffle(state_dict['selected'])
StudentModule.save_state(
student=usage_info.user,
course_id=usage_info.course_key,
@@ -122,7 +112,7 @@ class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransfo
)
# publish events for analytics
self.publish_events(
self._publish_events(
block_structure,
block_key,
previous_count,
@@ -147,8 +137,7 @@ class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransfo
return [block_structure.create_removal_filter(check_child_removal)]
@staticmethod
def publish_events(block_structure, location, previous_count, max_count, block_keys, user_id):
def _publish_events(self, block_structure, location, previous_count, max_count, block_keys, user_id):
"""
Helper method to publish events for analytics purposes
"""
@@ -188,82 +177,3 @@ 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.
"""
block_structure.request_xblock_fields('mode')
block_structure.request_xblock_fields('max_count')
ContentLibraryTransformer.set_block_analytics_summary(block_structure, modulestore())
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'])
# Ensure that the current children blocks are the same as the stored selections
# before ordering the blocks.
if current_children_blocks != current_selected_blocks:
logger.info(
u'Mismatch between the children of %s in the stored state and the actual children',
str(block_key)
)
previous_count = len(current_selected_blocks.intersection(current_children_blocks))
mode = block_structure.get_xblock_field(block_key, 'mode')
max_count = block_structure.get_xblock_field(block_key, 'max_count')
block_keys = LibraryContentModule.make_selection(
state_dict['selected'], library_children, max_count, mode
)
state_dict['selected'] = block_keys['selected']
random.shuffle(state_dict['selected'])
StudentModule.save_state(
student=usage_info.user,
course_id=usage_info.course_key,
module_state_key=block_key,
defaults={
'state': json.dumps(state_dict)
},
)
ContentLibraryTransformer.publish_events(
block_structure, block_key, previous_count, max_count, block_keys, usage_info.user.id
)
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])

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

@@ -5,14 +5,13 @@ Tests for ContentLibraryTransformer.
from __future__ import absolute_import
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, ContentLibraryOrderTransformer
from ..library_content import ContentLibraryTransformer
from .helpers import CourseStructureTestCase
@@ -169,130 +168,3 @@ 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)
)