SE-2170 Port changes not yet merged to upstream into our juniper release branch

cms/envs/common.py

@@ -74,6 +74,7 @@
     HEARTBEAT_CHECKS,
     HEARTBEAT_EXTENDED_CHECKS,
     HEARTBEAT_CELERY_TIMEOUT,
+    CELERY_CHECK_ROUTING_KEY,
 
     # Default site to use if no site exists matching request headers
     SITE_ID,

cms/envs/production.py

@@ -292,6 +292,12 @@ def get_env_setting(setting):
 if "TRACKING_IGNORE_URL_PATTERNS" in ENV_TOKENS:
     TRACKING_IGNORE_URL_PATTERNS = ENV_TOKENS.get("TRACKING_IGNORE_URL_PATTERNS")
 
+# Heartbeat
+HEARTBEAT_CHECKS = ENV_TOKENS.get('HEARTBEAT_CHECKS', HEARTBEAT_CHECKS)
+HEARTBEAT_EXTENDED_CHECKS = ENV_TOKENS.get('HEARTBEAT_EXTENDED_CHECKS', HEARTBEAT_EXTENDED_CHECKS)
+HEARTBEAT_CELERY_TIMEOUT = ENV_TOKENS.get('HEARTBEAT_CELERY_TIMEOUT', HEARTBEAT_CELERY_TIMEOUT)
+CELERY_CHECK_ROUTING_KEY = ENV_TOKENS.get('CELERY_CHECK_ROUTING_KEY', HIGH_PRIORITY_QUEUE)
+
 LOGIN_REDIRECT_WHITELIST = [reverse_lazy('home')]
 
 

common/lib/xmodule/xmodule/library_content_module.py

@@ -158,37 +158,40 @@ def make_selection(cls, selected, children, max_count, mode):
         """
         rand = random.Random()
 
-        selected = set(tuple(k) for k in selected)  # set of (block_type, block_id) tuples assigned to this student
+        selected_keys = set(tuple(k) for k in selected)  # set of (block_type, block_id) tuples assigned to this student
 
         # Determine which of our children we will show:
-        valid_block_keys = set([(c.block_type, c.block_id) for c in children])
+        valid_block_keys = set((c.block_type, c.block_id) for c in children)
 
         # Remove any selected blocks that are no longer valid:
-        invalid_block_keys = (selected - valid_block_keys)
+        invalid_block_keys = (selected_keys - valid_block_keys)
         if invalid_block_keys:
-            selected -= invalid_block_keys
+            selected_keys -= invalid_block_keys
 
         # If max_count has been decreased, we may have to drop some previously selected blocks:
         overlimit_block_keys = set()
-        if len(selected) > max_count:
-            num_to_remove = len(selected) - max_count
-            overlimit_block_keys = set(rand.sample(selected, num_to_remove))
-            selected -= overlimit_block_keys
+        if len(selected_keys) > max_count:
+            num_to_remove = len(selected_keys) - max_count
+            overlimit_block_keys = set(rand.sample(selected_keys, num_to_remove))
+            selected_keys -= overlimit_block_keys
 
         # Do we have enough blocks now?
-        num_to_add = max_count - len(selected)
+        num_to_add = max_count - len(selected_keys)
 
         added_block_keys = None
         if num_to_add > 0:
             # We need to select [more] blocks to display to this user:
-            pool = valid_block_keys - selected
+            pool = valid_block_keys - selected_keys
             if mode == "random":
                 num_to_add = min(len(pool), num_to_add)
                 added_block_keys = set(rand.sample(pool, num_to_add))
                 # We now have the correct n random children to show for this user.
             else:
                 raise NotImplementedError("Unsupported mode.")
-            selected |= added_block_keys
+            selected_keys |= added_block_keys
+
+        if any([invalid_block_keys, overlimit_block_keys, added_block_keys]):
+            selected = selected_keys
 
         return {
             'selected': selected,
@@ -268,19 +271,15 @@ def publish_selected_children_events(cls, block_keys, format_block_keys, publish
 
     def selected_children(self):
         """
-        Returns a set() of block_ids indicating which of the possible children
+        Returns a list() of block_ids indicating which of the possible children
         have been selected to display to the current user.
 
         This reads and updates the "selected" field, which has user_state scope.
 
-        Note: self.selected and the return value contain block_ids. To get
+        Note: the return value (self.selected) contains block_ids. To get
         actual BlockUsageLocators, it is necessary to use self.children,
         because the block_ids alone do not specify the block type.
         """
-        if hasattr(self, "_selected_set"):
-            # Already done:
-            return self._selected_set  # pylint: disable=access-member-before-definition
-
         block_keys = self.make_selection(self.selected, self.children, self.max_count, "random")  # pylint: disable=no-member
 
         # Publish events for analytics purposes:
@@ -292,13 +291,13 @@ def selected_children(self):
             self._publish_event,
         )
 
-        # Save our selections to the user state, to ensure consistency:
-        selected = block_keys['selected']
-        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
+        if any(block_keys[changed] for changed in ('invalid', 'overlimit', 'added')):
+            # Save our selections to the user state, to ensure consistency:
+            selected = list(block_keys['selected'])
+            random.shuffle(selected)
+            self.selected = selected  # TODO: this doesn't save from the LMS "Progress" page.
 
-        return selected
+        return self.selected
 
     def _get_selected_child_blocks(self):
         """

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

@@ -272,9 +272,8 @@ def _change_count_and_refresh_children(self, count):
         Helper method that changes the max_count of self.lc_block, refreshes
         children, and asserts that the number of selected children equals the count provided.
         """
-        # Clear the cache (only needed because we skip saving/re-loading the block) pylint: disable=protected-access
-        if hasattr(self.lc_block._xmodule, '_selected_set'):
-            del self.lc_block._xmodule._selected_set
+        # Construct the XModule for the descriptor, if not present already.
+        self.lc_block._xmodule  # pylint: disable=pointless-statement,protected-access
         self.lc_block.max_count = count
         selected = self.lc_block.get_child_descriptors()
         self.assertEqual(len(selected), count)
@@ -287,7 +286,7 @@ class TestLibraryContentModuleNoSearchIndex(LibraryContentModuleTestMixin, Libra
     Tests for library container when no search index is available.
     Tests fallback low-level CAPA problem introspection
     """
-    pass
+    pass  # pylint: disable=unnecessary-pass
 
 
 search_index_mock = Mock(spec=SearchEngine)  # pylint: disable=invalid-name
@@ -365,8 +364,8 @@ def _assert_event_was_published(self, event_type):
         Check that a LibraryContentModule analytics event was published by self.lc_block.
         """
         self.assertTrue(self.publisher.called)
-        self.assertTrue(len(self.publisher.call_args[0]), 3)
-        _, event_name, event_data = self.publisher.call_args[0]
+        self.assertTrue(len(self.publisher.call_args[0]), 3)  # pylint: disable=unsubscriptable-object
+        _, event_name, event_data = self.publisher.call_args[0]  # pylint: disable=unsubscriptable-object
         self.assertEqual(event_name, "edx.librarycontentblock.content.{}".format(event_type))
         self.assertEqual(event_data["location"], six.text_type(self.lc_block.location))
         return event_data
@@ -397,8 +396,6 @@ def test_assigned_event(self):
 
         # Now increase max_count so that one more child will be added:
         self.lc_block.max_count = 2
-        # Clear the cache (only needed because we skip saving/re-loading the block) pylint: disable=protected-access
-        del self.lc_block._xmodule._selected_set
         children = self.lc_block.get_child_descriptors()
         self.assertEqual(len(children), 2)
         child, new_child = children if children[0].location == child.location else reversed(children)
@@ -478,8 +475,6 @@ def test_removed_overlimit(self):
         self.lc_block.get_child_descriptors()  # This line is needed in the test environment or the change has no effect
         self.publisher.reset_mock()  # Clear the "assigned" event that was just published.
         self.lc_block.max_count = 0
-        # Clear the cache (only needed because we skip saving/re-loading the block) pylint: disable=protected-access
-        del self.lc_block._xmodule._selected_set
 
         # Check that the event says that one block was removed, leaving no blocks left:
         children = self.lc_block.get_child_descriptors()
@@ -497,8 +492,6 @@ def test_removed_invalid(self):
         # Start by assigning two blocks to the student:
         self.lc_block.get_child_descriptors()  # This line is needed in the test environment or the change has no effect
         self.lc_block.max_count = 2
-        # Clear the cache (only needed because we skip saving/re-loading the block) pylint: disable=protected-access
-        del self.lc_block._xmodule._selected_set
         initial_blocks_assigned = self.lc_block.get_child_descriptors()
         self.assertEqual(len(initial_blocks_assigned), 2)
         self.publisher.reset_mock()  # Clear the "assigned" event that was just published.
@@ -512,8 +505,6 @@ def test_removed_invalid(self):
         self.library.children = [keep_block_lib_usage_key]
         self.store.update_item(self.library, self.user_id)
         self.lc_block.refresh_children()
-        # Clear the cache (only needed because we skip saving/re-loading the block) pylint: disable=protected-access
-        del self.lc_block._xmodule._selected_set
 
         # Check that the event says that one block was removed, leaving one block left:
         children = self.lc_block.get_child_descriptors()

lms/djangoapps/course_blocks/api.py

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

lms/djangoapps/course_blocks/transformers/library_content.py

@@ -4,6 +4,8 @@
 
 
 import json
+import logging
+import random
 
 import six
 from eventtracking import tracker
@@ -19,6 +21,8 @@
 
 from ..utils import get_student_module_as_dict
 
+logger = logging.getLogger(__name__)
+
 
 class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransformer):
     """
@@ -40,17 +44,8 @@ def name(cls):
         return "library_content"
 
     @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
+    def set_block_analytics_summary(cls, block_structure, store):
+        """Set the block analytics summary information in the children fields."""
         def summarize_block(usage_key):
             """ Basic information about the given block """
             orig_key, orig_version = store.get_block_original_usage(usage_key)
@@ -71,6 +66,20 @@ def summarize_block(usage_key):
                 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()
@@ -102,6 +111,7 @@ def transform_block_filters(self, usage_info, block_structure):
                 # 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,
@@ -112,7 +122,7 @@ def transform_block_filters(self, usage_info, block_structure):
                     )
 
                 # publish events for analytics
-                self._publish_events(
+                self.publish_events(
                     block_structure,
                     block_key,
                     previous_count,
@@ -137,7 +147,8 @@ def check_child_removal(block_key):
 
         return [block_structure.create_removal_filter(check_child_removal)]
 
-    def _publish_events(self, block_structure, location, previous_count, max_count, block_keys, user_id):
+    @staticmethod
+    def publish_events(block_structure, location, previous_count, max_count, block_keys, user_id):
         """
         Helper method to publish events for analytics purposes
         """
@@ -177,3 +188,68 @@ def publish_event(event_name, result, **kwargs):
             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'])
+
+                # 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])