Skip to content

Commit d12b8d9

Browse files
fix: fall back gracefully on malformed persisted selection
Address review feedback: a saved `selected` value that doesn't have the expected (block_type, block_id) shape would raise during prefetch and take the render down. Fall back to the full-pool behavior instead, consistent with every other case here where the saved state can't be used.
1 parent 04e7ed0 commit d12b8d9

2 files changed

Lines changed: 30 additions & 1 deletion

File tree

lms/djangoapps/courseware/model_data.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
from collections import defaultdict, namedtuple
2929

3030
from django.db import DatabaseError, IntegrityError, transaction
31+
from opaque_keys import InvalidKeyError
3132
from opaque_keys.edx.asides import AsideUsageKeyV1, AsideUsageKeyV2
3233
from opaque_keys.edx.block_types import BlockTypeKeyV1
3334
from opaque_keys.edx.keys import LearningContextKey
@@ -832,7 +833,14 @@ def _persisted_selection_usage_keys(self, block):
832833
return None
833834

834835
course_key = block.location.course_key
835-
return [course_key.make_usage_key(block_type, block_id) for block_type, block_id in selected]
836+
try:
837+
return [course_key.make_usage_key(block_type, block_id) for block_type, block_id in selected]
838+
except (TypeError, ValueError, InvalidKeyError):
839+
log.warning(
840+
"Malformed 'selected' state for %s / %s while narrowing FieldDataCache prefetch",
841+
self.user.id, block.location,
842+
)
843+
return None
836844

837845
@classmethod
838846
def cache_for_block_descendents(cls, course_id, user, block, depth=None,

lms/djangoapps/courseware/tests/test_model_data.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,3 +543,24 @@ def test_ignores_empty_selection(self):
543543

544544
parent.get_children.assert_called_once()
545545
parent.get_child.assert_not_called()
546+
547+
def test_falls_back_on_malformed_selection(self):
548+
"""
549+
A saved `selected` value that isn't a list of (block_type, block_id)
550+
pairs shouldn't blow up the render -- fall back to the full pool, same
551+
as any other case where we can't make sense of what's saved.
552+
"""
553+
StudentModule.objects.create(
554+
student=self.user,
555+
course_id=COURSE_KEY,
556+
module_state_key=self.parent_location,
557+
module_type='library_content',
558+
state=json.dumps({'selected': ['not-a-pair', 'also-not-a-pair']}),
559+
)
560+
parent = self._make_parent_block()
561+
562+
cache = FieldDataCache([], COURSE_KEY, self.user)
563+
cache.add_block_descendents(parent)
564+
565+
parent.get_children.assert_called_once()
566+
parent.get_child.assert_not_called()

0 commit comments

Comments
 (0)