From a12c003215c66ea9f574d8e1d80149eafaed725a Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Fri, 17 Feb 2023 12:49:33 +0100 Subject: [PATCH] feat: allow using all components of LibraryContentBlock Setting max_count to a negative value resulted in raising an unhandled ValueError. Currently, it selects all children of the LibraryContentBlock. --- .../course_blocks/transformers/library_content.py | 2 ++ xmodule/library_content_block.py | 14 +++++++++++--- xmodule/tests/test_library_content.py | 14 ++++++++++++++ 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/course_blocks/transformers/library_content.py b/lms/djangoapps/course_blocks/transformers/library_content.py index 11321196d2e9..616cf68f4b62 100644 --- a/lms/djangoapps/course_blocks/transformers/library_content.py +++ b/lms/djangoapps/course_blocks/transformers/library_content.py @@ -85,6 +85,8 @@ def transform_block_filters(self, usage_info, block_structure): selected = [] mode = block_structure.get_xblock_field(block_key, 'mode') max_count = block_structure.get_xblock_field(block_key, 'max_count') + if max_count < 0: + max_count = len(library_children) # Retrieve "selected" json from LMS MySQL database. state_dict = get_student_module_as_dict(usage_info.user, usage_info.course_key, block_key) diff --git a/xmodule/library_content_block.py b/xmodule/library_content_block.py index ee8dbfeb76eb..e3e38c857017 100644 --- a/xmodule/library_content_block.py +++ b/xmodule/library_content_block.py @@ -160,7 +160,7 @@ def completion_mode(cls): # pylint: disable=no-self-argument ) max_count = Integer( display_name=_("Count"), - help=_("Enter the number of components to display to each student."), + help=_("Enter the number of components to display to each student. Set it to -1 to display all components."), default=1, scope=Scope.settings, ) @@ -337,7 +337,11 @@ def selected_children(self): actual BlockUsageLocators, it is necessary to use self.children, because the block_ids alone do not specify the block type. """ - block_keys = self.make_selection(self.selected, self.children, self.max_count, "random") # pylint: disable=no-member + max_count = self.max_count + if max_count < 0: + max_count = len(self.children) + + block_keys = self.make_selection(self.selected, self.children, max_count, "random") # pylint: disable=no-member # Publish events for analytics purposes: lib_tools = self.runtime.service(self, 'library_tools') @@ -433,9 +437,13 @@ def author_view(self, context): if is_root: # User has clicked the "View" link. Show a preview of all possible children: if self.children: # pylint: disable=no-member + max_count = self.max_count + if max_count < 0: + max_count = len(self.children) + fragment.add_content(self.runtime.service(self, 'mako').render_template( "library-block-author-preview-header.html", { - 'max_count': self.max_count, + 'max_count': max_count, 'display_name': self.display_name or self.url_name, })) context['can_edit_visibility'] = False diff --git a/xmodule/tests/test_library_content.py b/xmodule/tests/test_library_content.py index 30bbfdb11708..09f176ce63f8 100644 --- a/xmodule/tests/test_library_content.py +++ b/xmodule/tests/test_library_content.py @@ -280,18 +280,21 @@ def test_validation_of_matching_blocks(self): assert result.summary assert StudioValidationMessage.WARNING == result.summary.type assert 'only 4 matching problems' in result.summary.text + assert len(self.lc_block.selected_children()) == 4 # Add some capa problems so we can check problem type validation messages self.lc_block.max_count = 1 self._create_capa_problems() self.lc_block.refresh_children() assert self.lc_block.validate() + assert len(self.lc_block.selected_children()) == 1 # Existing problem type should pass validation self.lc_block.max_count = 1 self.lc_block.capa_type = 'multiplechoiceresponse' self.lc_block.refresh_children() assert self.lc_block.validate() + assert len(self.lc_block.selected_children()) == 1 # ... unless requested more blocks than exists in library self.lc_block.max_count = 10 @@ -303,6 +306,7 @@ def test_validation_of_matching_blocks(self): assert result.summary assert StudioValidationMessage.WARNING == result.summary.type assert 'only 1 matching problem' in result.summary.text + assert len(self.lc_block.selected_children()) == 1 # Missing problem type should always fail validation self.lc_block.max_count = 1 @@ -314,6 +318,14 @@ def test_validation_of_matching_blocks(self): assert result.summary assert StudioValidationMessage.WARNING == result.summary.type assert 'no matching problem types' in result.summary.text + assert len(self.lc_block.selected_children()) == 0 + + # -1 selects all blocks from the library. + self.lc_block.max_count = -1 + self.lc_block.capa_type = ANY_CAPA_TYPE_VALUE + self.lc_block.refresh_children() + assert self.lc_block.validate() + assert len(self.lc_block.selected_children()) == len(self.lc_block.children) def test_capa_type_filtering(self): """ @@ -388,6 +400,8 @@ def _change_count_and_refresh_children(self, count): (True, 8), # User resets selected children without reset button on content block (False, 8), + # User resets selected children with reset button on content block when all library blocks should be selected. + (True, -1), ) @ddt.unpack def test_reset_selected_children_capa_blocks(self, allow_resetting_children, max_count):