Publish mixed exercises QTI only when Perseus can't read a legacy input answer

contentcuration/contentcuration/tests/test_restore_channel.py

@@ -16,6 +16,7 @@
 from le_utils.constants import content_kinds
 from le_utils.constants import exercises
 from le_utils.constants import format_presets
+from lxml import etree
 from mixer.backend.django import mixer
 from mock import MagicMock
 from mock import patch
@@ -219,6 +220,13 @@ def test_generate_assessment_item(self):
 PERSEUS_ID = "fedcba0987654321fedcba0987654321"
 
 
+def _canonical_xml(data):
+    # Restore re-serializes items to rewrite media paths, so compare as XML.
+    if isinstance(data, str):
+        data = data.encode("utf-8")
+    return etree.tostring(etree.fromstring(data), method="c14n")
+
+
 class QTIRestoreTestCase(StudioTestCase):
     EDITOR_ITEM_XML = _item_xml(
         "{identifier}",
@@ -370,7 +378,9 @@ def _stored_package(self, preset=format_presets.QTI_ZIP):
     def _package_contents(self, package):
         with zipfile.ZipFile(package) as zf:
             return {
-                name: zf.read(name)
+                name: _canonical_xml(zf.read(name))
+                if name.endswith(".xml")
+                else zf.read(name)
                 for name in zf.namelist()
                 # Its manifest identifier is the node's content_id.
                 if name != "imsmanifest.xml"
@@ -412,7 +422,13 @@ def _assert_restores_source_rows(self, restored, assessment_ids):
                 else:
                     # The package names this item only by its editor slug.
                     self.assertRegex(item.assessment_id, "^[0-9a-f]{32}$")
-                if source_item.type in (exercises.QTI, exercises.PERSEUS_QUESTION):
+                if source_item.type == exercises.QTI:
+                    self.assertEqual(item.type, source_item.type)
+                    self.assertEqual(
+                        _canonical_xml(item.raw_data),
+                        _canonical_xml(source_item.raw_data),
+                    )
+                elif source_item.type == exercises.PERSEUS_QUESTION:
                     self.assertEqual(item.type, source_item.type)
                     self.assertEqual(item.raw_data, source_item.raw_data)
                 else:

contentcuration/contentcuration/tests/utils/qti/test_perseus_derive.py

@@ -145,22 +145,6 @@ def test_text_input_derivation():
     assert answers == [{"answer": "42", "correct": True, "order": 0}]
 
 
-@pytest.mark.parametrize(
-    "value,expected",
-    [
-        ("+5", "5"),
-        ("1E3", "1e3"),
-        (" +5 ", "5"),
-        ("+0", "0"),
-    ],
-)
-def test_text_input_normalises_xsd_double_answers(value, expected):
-    result = derive_perseus_item(_Item(_text_item("single", [value])))
-    assert json.loads(result.answers) == [
-        {"answer": expected, "correct": True, "order": 0}
-    ]
-
-
 def test_text_input_multiple_correct_values():
     item = _Item(_text_item("multiple", ["1", "2"]))
     result = derive_perseus_item(item)

contentcuration/contentcuration/tests/utils/test_exercise_creation.py

@@ -298,6 +298,17 @@ def test_play_once_gif_stays_play_once(self):
             self.assertNotIn("loop", img.info)
 
 
+def _exercise_data(items):
+    return {
+        "mastery_model": exercises.M_OF_N,
+        "randomize": True,
+        "n": len(items),
+        "m": 1,
+        "all_assessment_items": [item.assessment_id for item in items],
+        "assessment_mapping": {item.assessment_id: item.type for item in items},
+    }
+
+
 class TestPerseusExerciseCreation(StudioTestCase):
     """
     Tests for the create_perseus_exercise function which handles exercise file generation.
@@ -367,6 +378,18 @@ def _create_perseus_zip(self, exercise_data):
         )
         return generator.create_exercise_archive()
 
+    def _perseus_input_values(self, answers):
+        """Values the Perseus numeric-input renders for an input question."""
+        item = self._create_assessment_item(
+            exercises.INPUT_QUESTION, "What is the answer?", answers
+        )
+        self._create_perseus_zip(_exercise_data([item]))
+        exercise_file = self.exercise_node.files.get(preset_id=format_presets.EXERCISE)
+        zip_file, _ = self._validate_perseus_zip(exercise_file)
+        item_json = json.loads(zip_file.read(f"{item.assessment_id}.json"))
+        widget = item_json["question"]["widgets"]["numeric-input 1"]
+        return [answer["value"] for answer in widget["options"]["answers"]]
+
     def _validate_perseus_zip(self, exercise_file):
         """Helper to validate the structure of the Perseus zip file"""
         # Use Django's storage backend to read the file
@@ -838,6 +861,95 @@ def test_exercise_with_graphie(self):
                 processed_perseus_json,
             )
 
+    def _assert_only_perseus_item_packaged(self, perseus_item, answerless):
+        with self.assertLogs(level="WARNING") as logs:
+            self._create_perseus_zip(_exercise_data([perseus_item, answerless]))
+        self.assertTrue(any("no correct answer" in message for message in logs.output))
+        exercise_file = self.exercise_node.files.get(preset_id=format_presets.EXERCISE)
+        zip_file, exercise_data = self._validate_perseus_zip(exercise_file)
+        self.assertNotIn(f"{answerless.assessment_id}.json", zip_file.namelist())
+        self.assertEqual(
+            exercise_data["all_assessment_items"], [perseus_item.assessment_id]
+        )
+        self.assertEqual(
+            exercise_data["assessment_mapping"],
+            {perseus_item.assessment_id: exercises.PERSEUS_QUESTION},
+        )
+        return zip_file
+
+    def test_answerless_input_question_is_left_out(self):
+        perseus_item, _ = self._create_perseus_item()
+        for answers in (
+            [{"answer": "", "correct": True, "order": 1}],
+            [{"answer": False, "correct": True, "order": 1}],
+            [{"answer": None, "order": 1}],
+            [{"answer": "5", "correct": False, "order": 1}],
+            [],
+        ):
+            with self.subTest(answers=answers):
+                answerless = self._create_assessment_item(
+                    exercises.INPUT_QUESTION, "What is 2+3?", answers
+                )
+                self._assert_only_perseus_item_packaged(perseus_item, answerless)
+                answerless.delete()
+
+    def test_answerless_input_question_images_are_not_packaged(self):
+        perseus_item, _ = self._create_perseus_item()
+        image_file = fileobj_exercise_image()
+        image_url = exercises.CONTENT_STORAGE_FORMAT.format(image_file.filename())
+        answerless = self._create_assessment_item(
+            exercises.INPUT_QUESTION,
+            f"How many sides? ![shape]({image_url})",
+            [{"answer": "", "correct": True, "order": 1}],
+        )
+        image_file.assessment_item = answerless
+        image_file.save()
+        zip_file = self._assert_only_perseus_item_packaged(perseus_item, answerless)
+        self.assertNotIn(f"images/{image_file.filename()}", zip_file.namelist())
+
+    def test_unit_keeps_answerless_input_question(self):
+        self.exercise_node.kind_id = content_kinds.TOPIC
+        self.exercise_node.save()
+        perseus_item, _ = self._create_perseus_item()
+        answerless = self._create_assessment_item(
+            exercises.INPUT_QUESTION,
+            "What is 2+3?",
+            [{"answer": "", "correct": True, "order": 1}],
+        )
+        self._create_perseus_zip(_exercise_data([perseus_item, answerless]))
+        exercise_file = self.exercise_node.files.get(preset_id=format_presets.EXERCISE)
+        zip_file, exercise_data = self._validate_perseus_zip(exercise_file)
+        self.assertIn(f"{answerless.assessment_id}.json", zip_file.namelist())
+        self.assertEqual(
+            exercise_data["all_assessment_items"],
+            [perseus_item.assessment_id, answerless.assessment_id],
+        )
+
+    def test_numeric_input_answers_are_read(self):
+        for answer, numbers in (
+            (0, [0]),
+            (4.5, [4.5]),
+            ("+-3", []),
+        ):
+            with self.subTest(answer=answer):
+                self.assertEqual(
+                    self._perseus_input_values(
+                        [{"answer": answer, "correct": True, "order": 1}]
+                    ),
+                    numbers,
+                )
+                self.exercise_node.assessment_items.all().delete()
+
+    def test_incorrect_input_answers_are_dropped(self):
+        values = self._perseus_input_values(
+            [
+                {"answer": "4", "correct": True, "order": 1},
+                {"answer": 5, "correct": False, "order": 2},
+                {"answer": "6", "correct": False, "order": 3},
+            ]
+        )
+        self.assertEqual(values, [4])
+
     def test_formula_processing(self):
         """Test that formulas are properly processed in exercises"""
         # Create a question with LaTeX formulas
@@ -873,41 +985,12 @@ def test_formula_processing(self):
         )
         self.assertIn("$\\frac{x}{2} = 3$", item_json["question"]["content"])
 
-    def _numeric_answer_values(self, answers):
-        item = self._create_assessment_item(
-            exercises.INPUT_QUESTION,
-            "What is the answer?",
+    def test_input_question_drops_non_finite_answers(self):
+        answers = ["6", "1e400", "-1e400", "1" * 401, "1" * 401 + "/" + "1" * 401]
+        values = self._perseus_input_values(
             [
                 {"answer": answer, "correct": True, "order": order}
                 for order, answer in enumerate(answers, start=1)
-            ],
-        )
-        self._create_perseus_zip(
-            {
-                "mastery_model": exercises.M_OF_N,
-                "randomize": True,
-                "n": 1,
-                "m": 1,
-                "all_assessment_items": [item.assessment_id],
-                "assessment_mapping": {item.assessment_id: exercises.INPUT_QUESTION},
-            }
-        )
-        exercise_file = self.exercise_node.files.get(preset_id=format_presets.EXERCISE)
-        zip_file, _ = self._validate_perseus_zip(exercise_file)
-        item_json = json.loads(
-            zip_file.read(f"{item.assessment_id}.json").decode("utf-8")
-        )
-        options = item_json["question"]["widgets"]["numeric-input 1"]["options"]
-        return [answer["value"] for answer in options["answers"]]
-
-    def test_input_question_drops_non_finite_answers(self):
-        values = self._numeric_answer_values(
-            [
-                "6",
-                "1e400",
-                "-1e400",
-                "1" * 401,
-                "1" * 401 + "/" + "1" * 401,
             ]
         )
         self.assertEqual(values, [6.0])
@@ -2383,7 +2466,7 @@ def test_legacy_item_that_is_not_well_formed_xml_is_skipped(self):
         image_file.save()
 
         with self.assertLogs(level="ERROR") as logs:
-            self._create_qti_zip(self._exercise_data([item]))
+            self._create_qti_zip(_exercise_data([item]))
         self.assertTrue(
             any("is not well-formed XML" in message for message in logs.output)
         )
@@ -2408,7 +2491,7 @@ def _publish_legacy_sized_image(self, content, ext, width, height, ref_ext=None)
         image_file.assessment_item = item
         image_file.save()
 
-        self._create_qti_zip(self._exercise_data([item]))
+        self._create_qti_zip(_exercise_data([item]))
 
         exercise_file = self.exercise_node.files.get(preset_id=format_presets.QTI_ZIP)
         zip_file = self._validate_qti_zip_structure(exercise_file)
@@ -2830,24 +2913,14 @@ def test_native_qti_item_valid_only_in_its_declared_encoding_is_packaged(self):
             'encoding="UTF-8"', 'encoding="ISO-8859-1"'
         ).replace("<qti-prompt>", "<qti-prompt>￿")
         item = self._create_native_qti_item(raw_data)
-        self._create_qti_zip(self._exercise_data([item]))
+        self._create_qti_zip(_exercise_data([item]))
         exercise_file = self.exercise_node.files.get(preset_id=format_presets.QTI_ZIP)
         zip_file = self._validate_qti_zip_structure(exercise_file)
         self.assertIn("items/item_1.xml", zip_file.namelist())
 
-    def _exercise_data(self, items):
-        return {
-            "mastery_model": exercises.M_OF_N,
-            "randomize": True,
-            "n": len(items),
-            "m": 1,
-            "all_assessment_items": [item.assessment_id for item in items],
-            "assessment_mapping": {item.assessment_id: item.type for item in items},
-        }
-
     def _assert_only_item_packaged(self, items, identifier):
         with self.assertLogs(level="WARNING") as logs:
-            self._create_qti_zip(self._exercise_data(items))
+            self._create_qti_zip(_exercise_data(items))
         self.assertTrue(any("no correct answer" in message for message in logs.output))
         exercise_file = self.exercise_node.files.get(preset_id=format_presets.QTI_ZIP)
         zip_file = self._validate_qti_zip_structure(exercise_file)
@@ -2899,7 +2972,7 @@ def test_legacy_input_question_zero_answer_is_packaged(self):
             "What is 5 minus 5?",
             [{"answer": 0, "correct": True, "order": 1}],
         )
-        self._create_qti_zip(self._exercise_data([item]))
+        self._create_qti_zip(_exercise_data([item]))
         exercise_file = self.exercise_node.files.get(preset_id=format_presets.QTI_ZIP)
         zip_file = self._validate_qti_zip_structure(exercise_file)
         item_xml = zip_file.read(
@@ -2914,15 +2987,15 @@ def test_native_answerless_float_entry_is_skipped(self):
 
     def test_native_custom_processed_float_entry_is_packaged(self):
         item = self._create_native_qti_item(TOLERANCE_ITEM)
-        self._create_qti_zip(self._exercise_data([item]))
+        self._create_qti_zip(_exercise_data([item]))
         exercise_file = self.exercise_node.files.get(preset_id=format_presets.QTI_ZIP)
         zip_file = self._validate_qti_zip_structure(exercise_file)
         self.assertIn("items/item_text.xml", zip_file.namelist())
 
     def _assert_missing_media_is_logged_and_omitted(self, raw_data):
         item = self._create_native_qti_item(raw_data)  # no File row linked
         with self.assertLogs(level="ERROR") as logs:
-            self._create_qti_zip(self._exercise_data([item]))
+            self._create_qti_zip(_exercise_data([item]))
         self.assertTrue(
             any("no matching File record linked" in message for message in logs.output)
         )
@@ -2955,7 +3028,7 @@ def _create_native_qti_image_item(self, image_file, imgs):
 
     def _publish_native_qti_images(self, image_file, imgs):
         item = self._create_native_qti_image_item(image_file, imgs)
-        self._create_qti_zip(self._exercise_data([item]))
+        self._create_qti_zip(_exercise_data([item]))
         exercise_file = self.exercise_node.files.get(preset_id=format_presets.QTI_ZIP)
         zip_file = self._validate_qti_zip_structure(exercise_file)
         item_doc = parse_qti_xml(zip_file.read("items/native_item_1.xml"))
@@ -3179,7 +3252,7 @@ def test_native_qti_perseus_derivation_resizes_sized_images(self):
             image_file, self.TWO_SIZE_IMGS.format(filename=filename)
         )
 
-        self._create_perseus_zip(self._exercise_data([item]))
+        self._create_perseus_zip(_exercise_data([item]))
 
         exercise_file = self.exercise_node.files.get(preset_id=format_presets.EXERCISE)
         with storage.open(exercise_file.file_on_disk.name, "rb") as f:
@@ -3198,18 +3271,26 @@ def test_native_qti_perseus_derivation_resizes_sized_images(self):
         )
 
     def test_native_qti_perseus_derivation_signed_numeric_answer(self):
-        raw_data = _text_item("single", ["+5"])
-        item = self._create_native_qti_item(raw_data)
+        for value, expected in [("+5", 5), ("1E3", 1000), (" +5 ", 5), ("+0", 0)]:
+            with self.subTest(value=value):
+                self.exercise_node.assessment_items.all().delete()
+                self.exercise_node.files.all().delete()
+                raw_data = _text_item("single", [value])
+                item = self._create_native_qti_item(raw_data)
 
-        self._create_perseus_zip(self._exercise_data([item]))
+                self._create_perseus_zip(_exercise_data([item]))
 
-        exercise_file = self.exercise_node.files.get(preset_id=format_presets.EXERCISE)
-        with storage.open(exercise_file.file_on_disk.name, "rb") as f:
-            zip_file = zipfile.ZipFile(BytesIO(f.read()))
-        qti_id = parse_qti_xml(raw_data.encode("utf-8")).getroot().get("identifier")
-        item_json = json.loads(zip_file.read(f"{qti_id}.json").decode("utf-8"))
+                exercise_file = self.exercise_node.files.get(
+                    preset_id=format_presets.EXERCISE
+                )
+                with storage.open(exercise_file.file_on_disk.name, "rb") as f:
+                    zip_file = zipfile.ZipFile(BytesIO(f.read()))
+                qti_id = (
+                    parse_qti_xml(raw_data.encode("utf-8")).getroot().get("identifier")
+                )
+                item_json = json.loads(zip_file.read(f"{qti_id}.json").decode("utf-8"))
 
-        answers = item_json["question"]["widgets"]["numeric-input 1"]["options"][
-            "answers"
-        ]
-        self.assertEqual([answer["value"] for answer in answers], [5])
+                answers = item_json["question"]["widgets"]["numeric-input 1"][
+                    "options"
+                ]["answers"]
+                self.assertEqual([answer["value"] for answer in answers], [expected])