From 0557c263cbb37580745415f6992ba73f3ad1245f Mon Sep 17 00:00:00 2001 From: ruzniaievdm Date: Wed, 6 Mar 2024 12:15:34 +0200 Subject: [PATCH 1/2] fix: wrap into try/except block getting icon for xblock --- .../xblock_storage_handlers/view_handlers.py | 11 +---------- xmodule/split_test_block.py | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index 3989df7753ae..523dcee7a7ce 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -88,11 +88,6 @@ CREATE_IF_NOT_FOUND = ["course_info"] -# List of categories to check for presence in the children of the XBlock. -# This list is used to determine if all of the specified categories are absent -# in the categories of the children XBlock instances otherwise icon class variable will be set to `None`. -CATEGORIES_WITH_ABSENT_ICON = ["split_test"] - # Useful constants for defining predicates NEVER = lambda x: False ALWAYS = lambda x: True @@ -1068,10 +1063,6 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements ) else: user_partitions = get_user_partition_info(xblock, course=course) - all_excluded_categories_absent = all( - category not in [child.category for child in xblock.get_children()] - for category in CATEGORIES_WITH_ABSENT_ICON - ) xblock_info.update( { "edited_on": get_default_time_display(xblock.subtree_edited_on) @@ -1099,7 +1090,7 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements "group_access": xblock.group_access, "user_partitions": user_partitions, "show_correctness": xblock.show_correctness, - "xblock_type": get_icon(xblock) if is_xblock_unit and all_excluded_categories_absent else None, + "xblock_type": get_icon(xblock), } ) diff --git a/xmodule/split_test_block.py b/xmodule/split_test_block.py index 88b51db19b3b..d9d0de2dd7fa 100644 --- a/xmodule/split_test_block.py +++ b/xmodule/split_test_block.py @@ -15,6 +15,7 @@ from web_fragments.fragment import Fragment from webob import Response from xblock.core import XBlock +from xblock.exceptions import NoSuchServiceError from xblock.fields import Integer, ReferenceValueDict, Scope, String from xmodule.mako_block import MakoTemplateBlockBase from xmodule.modulestore.inheritance import UserPartitionList @@ -172,10 +173,14 @@ def child_block(self): def child(self): """ Return the user bound child block for the partition or None. + + We are using try/except block here because we ran into issues with + CachingDescriptorSystem has no attribute when we get an icon for the split_test xblock. """ - if self.child_block is not None: + try: return self.runtime.get_block_for_descriptor(self.child_block) - else: + except AttributeError: + log.warning("Error while getting block for descriptor") return None def get_child_block_by_location(self, location): @@ -212,8 +217,16 @@ def get_content_titles(self): def get_child_blocks(self): """ For grading--return just the chosen child. + + We are using try/except block here because we ran into issues with + User service being undefined when we get an icon for the split_test xblock. """ - group_id = self.get_group_id() + try: + group_id = self.get_group_id() + except NoSuchServiceError: + log.warning("Error while getting user service in runtime") + return [] + if group_id is None: return [] From c42d7dd379f708e341cfa5e5497c5cb3fa68f2ef Mon Sep 17 00:00:00 2001 From: ruzniaievdm Date: Wed, 6 Mar 2024 14:02:51 +0200 Subject: [PATCH 2/2] fix: revision after review --- xmodule/split_test_block.py | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/xmodule/split_test_block.py b/xmodule/split_test_block.py index d9d0de2dd7fa..05ca3a5db454 100644 --- a/xmodule/split_test_block.py +++ b/xmodule/split_test_block.py @@ -174,13 +174,19 @@ def child(self): """ Return the user bound child block for the partition or None. - We are using try/except block here because we ran into issues with - CachingDescriptorSystem has no attribute when we get an icon for the split_test xblock. + Handles the AttributeError exception that may occur when attempting to retrieve + an icon for the split_test xblock within the CMS. """ try: - return self.runtime.get_block_for_descriptor(self.child_block) + if self.child_block is not None: + return self.runtime.get_block_for_descriptor(self.child_block) + else: + return None except AttributeError: - log.warning("Error while getting block for descriptor") + log.warning( + "Error while getting block instance for descriptor with location: [%s]", + self.location + ) return None def get_child_block_by_location(self, location): @@ -218,13 +224,22 @@ def get_child_blocks(self): """ For grading--return just the chosen child. - We are using try/except block here because we ran into issues with - User service being undefined when we get an icon for the split_test xblock. + Handles the NoSuchServiceError and ValueError exception that may occur when attempting to retrieve + an icon for the split_test xblock within the CMS. """ try: group_id = self.get_group_id() except NoSuchServiceError: - log.warning("Error while getting user service in runtime") + log.warning( + "Error while getting user service in runtime with location: [%s]", + self.location + ) + return [] + except ValueError: + log.warning( + "Error while getting group ID for partition with location: [%s]", + self.location + ) return [] if group_id is None: @@ -232,6 +247,7 @@ def get_child_blocks(self): # group_id_to_child comes from json, so it has to have string keys str_group_id = str(group_id) + child_block = None if str_group_id in self.group_id_to_child: child_location = self.group_id_to_child[str_group_id] child_block = self.get_child_block_by_location(child_location)