Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: wrap into try/except block getting icon for xblock #2509

Merged
merged 2 commits into from
Mar 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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),
}
)

Expand Down
37 changes: 33 additions & 4 deletions xmodule/split_test_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -172,10 +173,20 @@ def child_block(self):
def child(self):
"""
Return the user bound child block for the partition or None.

Handles the AttributeError exception that may occur when attempting to retrieve
an icon for the split_test xblock within the CMS.
"""
if self.child_block is not None:
return self.runtime.get_block_for_descriptor(self.child_block)
else:
try:
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 instance for descriptor with location: [%s]",
self.location
)
return None

def get_child_block_by_location(self, location):
Expand Down Expand Up @@ -212,13 +223,31 @@ def get_content_titles(self):
def get_child_blocks(self):
"""
For grading--return just the chosen child.

Handles the NoSuchServiceError and ValueError exception that may occur when attempting to retrieve
an icon for the split_test xblock within the CMS.
"""
group_id = self.get_group_id()
try:
group_id = self.get_group_id()
except NoSuchServiceError:
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:
return []

# 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)
Expand Down
Loading