Merge pull request #7420 from open-craft/fix-library-analytics
Fix two content library analytics issues (SOL-521)
This commit is contained in:
@@ -133,6 +133,19 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule):
|
||||
as children of this block, but only a subset of those children are shown to
|
||||
any particular student.
|
||||
"""
|
||||
|
||||
def _publish_event(self, event_name, result, **kwargs):
|
||||
""" Helper method to publish an event for analytics purposes """
|
||||
event_data = {
|
||||
"location": unicode(self.location),
|
||||
"result": result,
|
||||
"previous_count": getattr(self, "_last_event_result_count", len(self.selected)),
|
||||
"max_count": self.max_count,
|
||||
}
|
||||
event_data.update(kwargs)
|
||||
self.runtime.publish(self, "edx.librarycontentblock.content.{}".format(event_name), event_data)
|
||||
self._last_event_result_count = len(result) # pylint: disable=attribute-defined-outside-init
|
||||
|
||||
def selected_children(self):
|
||||
"""
|
||||
Returns a set() of block_ids indicating which of the possible children
|
||||
@@ -149,22 +162,10 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule):
|
||||
return self._selected_set # pylint: disable=access-member-before-definition
|
||||
|
||||
selected = set(tuple(k) for k in self.selected) # set of (block_type, block_id) tuples assigned to this student
|
||||
previous_count = len(selected)
|
||||
|
||||
lib_tools = self.runtime.service(self, 'library_tools')
|
||||
format_block_keys = lambda keys: lib_tools.create_block_analytics_summary(self.location.course_key, keys)
|
||||
|
||||
def publish_event(event_name, **kwargs):
|
||||
""" Publish an event for analytics purposes """
|
||||
event_data = {
|
||||
"location": unicode(self.location),
|
||||
"result": format_block_keys(selected),
|
||||
"previous_count": previous_count,
|
||||
"max_count": self.max_count,
|
||||
}
|
||||
event_data.update(kwargs)
|
||||
self.runtime.publish(self, "edx.librarycontentblock.content.{}".format(event_name), event_data)
|
||||
|
||||
# Determine which of our children we will show:
|
||||
valid_block_keys = set([(c.block_type, c.block_id) for c in self.children]) # pylint: disable=no-member
|
||||
# Remove any selected blocks that are no longer valid:
|
||||
@@ -173,14 +174,24 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule):
|
||||
selected -= invalid_block_keys
|
||||
# Publish an event for analytics purposes:
|
||||
# reason "invalid" means deleted from library or a different library is now being used.
|
||||
publish_event("removed", removed=format_block_keys(invalid_block_keys), reason="invalid")
|
||||
self._publish_event(
|
||||
"removed",
|
||||
result=format_block_keys(selected),
|
||||
removed=format_block_keys(invalid_block_keys),
|
||||
reason="invalid"
|
||||
)
|
||||
# If max_count has been decreased, we may have to drop some previously selected blocks:
|
||||
overlimit_block_keys = set()
|
||||
while len(selected) > self.max_count:
|
||||
overlimit_block_keys.add(selected.pop())
|
||||
if overlimit_block_keys:
|
||||
# Publish an event for analytics purposes:
|
||||
publish_event("removed", removed=format_block_keys(overlimit_block_keys), reason="overlimit")
|
||||
self._publish_event(
|
||||
"removed",
|
||||
result=format_block_keys(selected),
|
||||
removed=format_block_keys(overlimit_block_keys),
|
||||
reason="overlimit"
|
||||
)
|
||||
# Do we have enough blocks now?
|
||||
num_to_add = self.max_count - len(selected)
|
||||
if num_to_add > 0:
|
||||
@@ -196,7 +207,11 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule):
|
||||
selected |= added_block_keys
|
||||
if added_block_keys:
|
||||
# Publish an event for analytics purposes:
|
||||
publish_event("assigned", added=format_block_keys(added_block_keys))
|
||||
self._publish_event(
|
||||
"assigned",
|
||||
result=format_block_keys(selected),
|
||||
added=format_block_keys(added_block_keys)
|
||||
)
|
||||
# Save our selections to the user state, to ensure consistency:
|
||||
self.selected = list(selected) # TODO: this doesn't save from the LMS "Progress" page.
|
||||
# Cache the results
|
||||
|
||||
@@ -327,6 +327,8 @@ class EditInfo(object):
|
||||
# User ID which changed this XBlock last.
|
||||
self.edited_by = edit_info.get('edited_by', None)
|
||||
|
||||
# If this block has been copied from a library using copy_from_template,
|
||||
# these fields point to the original block in the library, for analytics.
|
||||
self.original_usage = edit_info.get('original_usage', None)
|
||||
self.original_usage_version = edit_info.get('original_usage_version', None)
|
||||
|
||||
|
||||
@@ -2286,9 +2286,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
|
||||
new_block_info.fields = existing_block_info.fields # Preserve any existing overrides
|
||||
if 'children' in new_block_info.defaults:
|
||||
del new_block_info.defaults['children'] # Will be set later
|
||||
# ALERT! Why was this 'block_id' stored here? Nothing else stores block_id
|
||||
# in the block data. Was this a harmless error?
|
||||
#new_block_info['block_id'] = new_block_key.id
|
||||
|
||||
new_block_info.edit_info = existing_block_info.edit_info
|
||||
new_block_info.edit_info.previous_version = new_block_info.edit_info.update_version
|
||||
new_block_info.edit_info.update_version = dest_structure['_id']
|
||||
@@ -2922,6 +2920,10 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
|
||||
raw=True,
|
||||
block_defaults=new_block.defaults
|
||||
)
|
||||
# Extend the block's new edit_info with any extra edit_info fields from the source (e.g. original_usage):
|
||||
for key, val in new_block.edit_info.to_storable().iteritems():
|
||||
if getattr(destination_block.edit_info, key) is None:
|
||||
setattr(destination_block.edit_info, key, val)
|
||||
|
||||
# introduce new edit info field for tracing where copied/published blocks came
|
||||
destination_block.edit_info.source_version = new_block.edit_info.update_version
|
||||
|
||||
@@ -12,6 +12,7 @@ from xblock.runtime import Runtime as VanillaRuntime
|
||||
|
||||
from xmodule.library_content_module import ANY_CAPA_TYPE_VALUE, LibraryContentDescriptor
|
||||
from xmodule.library_tools import LibraryToolsService
|
||||
from xmodule.modulestore import ModuleStoreEnum
|
||||
from xmodule.modulestore.tests.factories import LibraryFactory, CourseFactory
|
||||
from xmodule.modulestore.tests.utils import MixedSplitTestCase
|
||||
from xmodule.tests import get_test_system
|
||||
@@ -49,13 +50,13 @@ class LibraryContentTest(MixedSplitTestCase):
|
||||
"""
|
||||
Bind a module (part of self.course) so we can access student-specific data.
|
||||
"""
|
||||
module_system = get_test_system(course_id=self.course.location.course_key)
|
||||
module_system = get_test_system(course_id=module.location.course_key)
|
||||
module_system.descriptor_runtime = module.runtime._descriptor_system # pylint: disable=protected-access
|
||||
module_system._services['library_tools'] = self.tools # pylint: disable=protected-access
|
||||
|
||||
def get_module(descriptor):
|
||||
"""Mocks module_system get_module function"""
|
||||
sub_module_system = get_test_system(course_id=self.course.location.course_key)
|
||||
sub_module_system = get_test_system(course_id=module.location.course_key)
|
||||
sub_module_system.get_module = get_module
|
||||
sub_module_system.descriptor_runtime = descriptor._runtime # pylint: disable=protected-access
|
||||
descriptor.bind_for_student(sub_module_system, descriptor._field_data, self.user_id) # pylint: disable=protected-access
|
||||
@@ -324,6 +325,17 @@ class TestLibraryContentAnalytics(LibraryContentTest):
|
||||
self.assertEqual(event_data["previous_count"], 1)
|
||||
self.assertEqual(event_data["max_count"], 2)
|
||||
|
||||
def test_assigned_event_published(self):
|
||||
"""
|
||||
Same as test_assigned_event but uses the published branch
|
||||
"""
|
||||
self.store.publish(self.course.location, self.user_id)
|
||||
with self.store.branch_setting(ModuleStoreEnum.Branch.published_only):
|
||||
self.lc_block = self.store.get_item(self.lc_block.location)
|
||||
self._bind_course_module(self.lc_block)
|
||||
self.lc_block.xmodule_runtime.publish = self.publisher
|
||||
self.test_assigned_event()
|
||||
|
||||
def test_assigned_descendants(self):
|
||||
"""
|
||||
Test the "assigned" event emitted includes descendant block information.
|
||||
|
||||
Reference in New Issue
Block a user