Skip to content

Commit bfd94b5

Browse files
rtibblesbotclaude
andcommitted
fix: surface IndexError from conversion and pin the response shape
serialize_object() swallows IndexError alongside ValueError and TypeError into a 404, so an IndexError raised during conversion would report a corrupt row as a missing one - the failure mode consolidate()'s re-raise exists to prevent. Widen the caught tuple to match. Assert the node language the values tuple carries for the conversion does not leak into the response, on both the converted and passed-through branches; without the latter a pop placed after the passthrough check would ship an internal join key to the client. Correct the comment on the answerless item body: an empty <div /> inside qti-item-body is XSD-valid, so the empty <p /> is there to give the body a paragraph to render and edit, not to satisfy the schema. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 2ad5d7b commit bfd94b5

4 files changed

Lines changed: 14 additions & 9 deletions

File tree

contentcuration/contentcuration/tests/utils/qti/test_convert.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,8 @@ def test_choice_types_with_no_answers_omit_the_interaction(self):
156156
self.assertTrue(validate_qti_item(result.xml.encode("utf-8")).is_valid)
157157

158158
def test_choice_type_with_no_answers_and_no_question(self):
159-
# The model's own defaults, and qti-item-body cannot be empty - so a
160-
# question with nothing typed into it yet carries an empty paragraph.
159+
# The model's own defaults - a question with nothing typed into it yet
160+
# still carries an empty paragraph to render and edit.
161161
item = _make_item(
162162
type=exercises.MULTIPLE_SELECTION,
163163
question="",

contentcuration/contentcuration/tests/viewsets/test_assessmentitem.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1265,6 +1265,8 @@ def test_converted_item_has_no_legacy_field_content(self):
12651265
self.assertEqual(item["question"], "")
12661266
self.assertEqual(item["answers"], "[]")
12671267
self.assertEqual(item["hints"], "[]")
1268+
# The node language is only in the values tuple to feed the conversion.
1269+
self.assertNotIn("contentnode__language__lang_code", item)
12681270

12691271
def test_converted_items_are_tagged_with_their_own_node_language(self):
12701272
# A contentnode__in read spans several nodes, so each item has to pick
@@ -1321,6 +1323,8 @@ def test_perseus_question_returned_unchanged(self):
13211323

13221324
self.assertEqual(item["type"], exercises.PERSEUS_QUESTION)
13231325
self.assertEqual(item["raw_data"], raw_data)
1326+
# A passed-through row must shed the node language too.
1327+
self.assertNotIn("contentnode__language__lang_code", item)
13241328

13251329
def test_native_qti_item_returned_unchanged(self):
13261330
assessment_id = self._create_item(

contentcuration/contentcuration/utils/assessment/qti/convert.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -325,8 +325,9 @@ def convert_legacy_assessment_item_to_qti(
325325
if interaction is None:
326326
# Emit the question text alone, ungraded. Div because rendered markdown
327327
# can start with a top level <math>, which qti-item-body does not accept
328-
# directly; P() because the container cannot be empty and a newly added
329-
# question has no text yet.
328+
# directly; the empty P stands in for the text a newly added question
329+
# does not have yet, so the body is a paragraph to render and edit
330+
# rather than a bare empty div.
330331
item_body = ItemBody(
331332
children=[
332333
Div(children=_create_html_content_from_text(item.question) or [P()])

contentcuration/contentcuration/viewsets/assessmentitem.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -360,11 +360,11 @@ def consolidate(self, items, queryset):
360360
try:
361361
# A new dict, so the language does not leak into the response.
362362
result = convert_legacy_question_to_qti(dict(item, language=language))
363-
except (ValueError, TypeError) as e:
364-
# serialize_object() turns ValueError/TypeError into a 404
365-
# (base.py), reporting a corrupt row as a missing one; re-raise
366-
# as a type it does not catch (pydantic and json errors both
367-
# subclass ValueError).
363+
except (IndexError, ValueError, TypeError) as e:
364+
# serialize_object() turns IndexError/ValueError/TypeError into
365+
# a 404 (base.py), reporting a corrupt row as a missing one;
366+
# re-raise as a type it does not catch (pydantic and json errors
367+
# both subclass ValueError).
368368
raise LegacyConversionError(
369369
f"Could not convert assessment item {item['assessment_id']} to QTI"
370370
) from e

0 commit comments

Comments
 (0)