Fix two content library analytics issues (SOL-521)

common/lib/xmodule/xmodule/library_content_module.py

@@ -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

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -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)
 

common/lib/xmodule/xmodule/modulestore/split_mongo/split.py

@@ -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

common/lib/xmodule/xmodule/tests/test_library_content.py

@@ -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.