Refactor library transformer to gather all modulestore-required queries during the collect phase.

common/lib/xmodule/xmodule/library_content_module.py

@@ -136,7 +136,7 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule):
     """
 
     @classmethod
-    def make_selection(cls, selected, children, max_count, mode, location, lib_tools, emit):
+    def make_selection(cls, selected, children, max_count, mode):
         """
         Dynamically selects block_ids indicating which of the possible children are displayed to the current user.
 
@@ -145,29 +145,17 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule):
             children - children of this block
             max_count - number of components to display to each student
             mode - how content is drawn from the library
-            location -
-            lib_tools - instance of LibraryToolsService
-            emit - function to emit events
 
         Returns:
-            a set of (block_type, block_id) tuples used to record
-            which random/first set of matching blocks was selected per user
-        """
-
-        def publish_event(event_name, result, **kwargs):
-            """ Helper method to publish an event for analytics purposes """
-            event_data = {
-                "location": unicode(location),
-                "result": result,
-                "previous_count": last_event_result_count,
-                "max_count": max_count,
-            }
-            event_data.update(kwargs)
-            emit("edx.librarycontentblock.content.{}".format(event_name), event_data)
+            A dict containing the following keys:
 
+            'selected' (set) of (block_type, block_id) tuples assigned to this student
+            'invalid' (set) of dropped (block_type, block_id) tuples that are no longer valid
+            'overlimit' (set) of dropped (block_type, block_id) tuples that were previously selected
+            'added' (set) of newly added (block_type, block_id) tuples
+        """
         last_event_result_count = len(selected)
         selected = set(tuple(k) for k in selected)  # set of (block_type, block_id) tuples assigned to this student
-        format_block_keys = lambda keys: lib_tools.create_block_analytics_summary(location.course_key, keys)
 
         # Determine which of our children we will show:
         valid_block_keys = set([(c.block_type, c.block_id) for c in children])  # pylint: disable=no-member
@@ -175,29 +163,12 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule):
         invalid_block_keys = (selected - valid_block_keys)
         if invalid_block_keys:
             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",
-                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) > max_count:
             overlimit_block_keys.add(selected.pop())
 
-        if overlimit_block_keys:
-            # Publish an event for analytics purposes:
-            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 = max_count - len(selected)
 
@@ -213,15 +184,24 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule):
                 raise NotImplementedError("Unsupported mode.")
             selected |= added_block_keys
 
-            if added_block_keys:
-                # Publish an event for analytics purposes:
-                publish_event(
-                    "assigned",
-                    result=format_block_keys(selected),
-                    added=format_block_keys(added_block_keys)
-                )
+        return {
+            'selected': selected,
+            'invalid': invalid_block_keys,
+            'overlimit': overlimit_block_keys,
+            'added': added_block_keys,
+        }
 
-        return selected
+    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):
         """
@@ -238,17 +218,42 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule):
             # Already done:
             return self._selected_set  # pylint: disable=access-member-before-definition
 
-        lib_tools = self.runtime.service(self, 'library_tools')
-        emit = lambda *args: self.runtime.publish(self, *args)
-
-        selected = self.make_selection(
-            self.selected, self.children, self.max_count, "random", self.location, lib_tools, emit
-        )
+        block_keys = self.make_selection(self.selected, self.children, self.max_count, "random")
+        selected = block_keys['selected']
 
         # 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
         self._selected_set = selected  # pylint: disable=attribute-defined-outside-init
+
+        # Publish events for analytics purposes:
+        lib_tools = self.runtime.service(self, 'library_tools')
+        format_block_keys = lambda keys: lib_tools.create_block_analytics_summary(self.location.course_key, keys)
+
+        if block_keys['invalid']:
+            # reason "invalid" means deleted from library or a different library is now being used.
+            self._publish_event(
+                "removed",
+                result=format_block_keys(selected),
+                removed=format_block_keys(block_keys['invalid']),
+                reason="invalid"
+            )
+
+        if block_keys['overlimit']:
+            self._publish_event(
+                "removed",
+                result=format_block_keys(selected),
+                removed=format_block_keys(block_keys['overlimit']),
+                reason="overlimit"
+            )
+
+        if block_keys['added']:
+            self._publish_event(
+                "assigned",
+                result=format_block_keys(selected),
+                added=format_block_keys(block_keys['added'])
+            )
+
         return selected
 
     def _get_selected_child_blocks(self):

lms/djangoapps/course_blocks/transformers/library_content.py

@@ -4,10 +4,8 @@ Content Library Transformer, used to filter course structure per user.
 import json
 from courseware.access import _has_access_to_course
 from courseware.models import StudentModule
-from opaque_keys.edx.locator import BlockUsageLocator
 from openedx.core.lib.block_cache.transformer import BlockStructureTransformer
 from xmodule.library_content_module import LibraryContentModule
-from xmodule.library_tools import LibraryToolsService
 from xmodule.modulestore.django import modulestore
 from eventtracking import tracker
 
@@ -53,6 +51,17 @@ class ContentLibraryTransformer(BlockStructureTransformer):
         """
         block_structure.request_xblock_fields('mode')
         block_structure.request_xblock_fields('max_count')
+        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,
+            }
 
         # For each block check if block is library_content.
         # If library_content add children array to content_library_children field
@@ -61,6 +70,9 @@ class ContentLibraryTransformer(BlockStructureTransformer):
             block_structure.set_transformer_block_data(block_key, cls, 'content_library_children', [])
             if getattr(xblock, 'category', None) == 'library_content':
                 block_structure.set_transformer_block_data(block_key, cls, 'content_library_children', xblock.children)
+                for child_key in xblock.children:
+                    summary = summarize_block(child_key)
+                    block_structure.set_transformer_block_data(child_key, cls, 'block_analytics_summary', summary)
 
     def transform(self, user_info, block_structure):
         """
@@ -70,22 +82,56 @@ class ContentLibraryTransformer(BlockStructureTransformer):
             user_info(object)
             block_structure (BlockStructureCollectedData)
         """
-        store = modulestore()
-        lib_tools = LibraryToolsService(store)
 
-        def build_key(block_type, block_id):
+        def publish_events(location, previous_count, max_count, block_keys):
             """
-            Helper method to build a BlockUsageLocator for user_info.course_key
+            Helper method to publish events for analytics purposes
             """
-            return BlockUsageLocator(user_info.course_key, block_type, block_id)
 
-        def update_selection(selected, children, max_count, mode, location):
-            """
-            Helper method to update library content selection
-            """
-            return LibraryContentModule.make_selection(
-                selected, children, max_count, mode, location, lib_tools, tracker.emit
-            )
+            def format_block_keys(keys):
+                """ Helper method to format block keys """
+                json = []
+                for key in keys:
+                    info = block_structure.get_transformer_block_data(block_key, self, 'block_analytics_summary')
+                    json.append(info)
+                return json
+
+            def publish_event(event_name, result, **kwargs):
+                """ Helper method to publish an event for analytics purposes """
+                event_data = {
+                    "location": unicode(location),
+                    "previous_count": previous_count,
+                    "result": result,
+                    "max_count": max_count
+                }
+                event_data.update(kwargs)
+                tracker.emit("edx.librarycontentblock.content.{}".format(event_name), event_data)
+
+
+            if block_keys['invalid']:
+                # reason "invalid" means deleted from library or a different library is now being used.
+                publish_event(
+                    "removed",
+                    result=format_block_keys(block_keys['selected']),
+                    removed=format_block_keys(block_keys['invalid']),
+                    reason="invalid"
+                )
+
+            if block_keys['overlimit']:
+                publish_event(
+                    "removed",
+                    result=format_block_keys(block_keys['selected']),
+                    removed=format_block_keys(block_keys['overlimit']),
+                    reason="overlimit"
+                )
+
+            if block_keys['added']:
+                publish_event(
+                    "assigned",
+                    result=format_block_keys(block_keys['selected']),
+                    added=format_block_keys(block_keys['added'])
+                )
+
 
         def check_child_removal(block_key):
             """
@@ -117,13 +163,18 @@ class ContentLibraryTransformer(BlockStructureTransformer):
                     # Check all selected entries for this user on selected library.
                     # Add all selected to selected list.
                     for state in state_dict['selected']:
-                        usage_key = build_key(state[0], state[1])
+                        usage_key = user_info.course_key.make_usage_key(state[0], state[1])
                         if usage_key in library_children:
                             selected.append((state[0], state[1]))
 
                 # update selected
-                selected = update_selection(selected, children, max_count, mode, block_key)
-                selected_children.extend([build_key(s[0], s[1]) for s in selected])
+                previous_count = len(selected)
+                block_keys = LibraryContentModule.make_selection(selected, children, max_count, mode)
+                selected = block_keys['selected']
+
+                # publish events for analytics
+                publish_events(block_key, previous_count, max_count, block_keys)
+                selected_children.extend([user_info.course_key.make_usage_key(s[0], s[1]) for s in selected])
 
         # Check and remove all non-selected children from course structure.
         block_structure.remove_block_if(