EDUCATOR-1290 | Remove over-limit block keys randomly for library content blocks.

common/lib/xmodule/xmodule/library_content_module.py

@@ -152,6 +152,8 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule):
             'overlimit' (set) of dropped (block_type, block_id) tuples that were previously selected
             'added' (set) of newly added (block_type, block_id) tuples
         """
+        rand = random.Random()
+
         selected = 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:
@@ -164,8 +166,10 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule):
 
         # 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 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
 
         # Do we have enough blocks now?
         num_to_add = max_count - len(selected)
@@ -176,7 +180,6 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule):
             pool = valid_block_keys - selected
             if mode == "random":
                 num_to_add = min(len(pool), num_to_add)
-                rand = random.Random()
                 added_block_keys = set(rand.sample(pool, num_to_add))
                 # We now have the correct n random children to show for this user.
             else:

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

@@ -241,6 +241,42 @@ class LibraryContentModuleTestMixin(object):
         self.assertIn(LibraryContentDescriptor.mode, non_editable_metadata_fields)
         self.assertNotIn(LibraryContentDescriptor.display_name, non_editable_metadata_fields)
 
+    def test_overlimit_blocks_chosen_randomly(self):
+        """
+        Tests that blocks to remove from selected children are chosen
+        randomly when len(selected) > max_count.
+        """
+        blocks_seen = set()
+        total_tries, max_tries = 0, 100
+
+        self.lc_block.refresh_children()
+        self.lc_block = self.store.get_item(self.lc_block.location)
+        self._bind_course_module(self.lc_block)
+
+        # Eventually, we should see every child block selected
+        while len(blocks_seen) != len(self.lib_blocks):
+            self._change_count_and_refresh_children(len(self.lib_blocks))
+            # Now set the number of selections to 1
+            selected = self._change_count_and_refresh_children(1)
+            blocks_seen.update(selected)
+            total_tries += 1
+            if total_tries >= max_tries:
+                assert False, "Max tries exceeded before seeing all blocks."
+                break
+
+    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
+        self.lc_block.max_count = count
+        selected = self.lc_block.get_child_descriptors()
+        self.assertEqual(len(selected), count)
+        return selected
+
 
 @patch('xmodule.library_tools.SearchEngine.get_search_engine', Mock(return_value=None, autospec=True))
 class TestLibraryContentModuleNoSearchIndex(LibraryContentModuleTestMixin, LibraryContentTest):