Leave answerless legacy input questions out of Perseus archives

contentcuration/contentcuration/tests/test_exportchannel.py

@@ -1,3 +1,4 @@
+import io
 import json
 import os
 import random
@@ -53,6 +54,7 @@
 from contentcuration.utils.publish import convert_channel_thumbnail
 from contentcuration.utils.publish import create_content_database
 from contentcuration.utils.publish import create_draft_channel_version
+from contentcuration.utils.publish import create_kolibri_assessment_metadata
 from contentcuration.utils.publish import create_slideshow_manifest
 from contentcuration.utils.publish import fill_published_fields
 from contentcuration.utils.publish import map_prerequisites
@@ -455,6 +457,85 @@ def setUp(self):
             randomize=False,
         )
 
+        # Perseus packaging with an input question that has no accepted answer
+        perseus_answerless_input_exercise = create_node(
+            {
+                "kind_id": "exercise",
+                "title": "Perseus Answerless Input Exercise",
+                "extra_fields": qti_extra_fields,
+            }
+        )
+        perseus_answerless_input_exercise.complete = True
+        perseus_answerless_input_exercise.parent = current_exercise.parent
+        perseus_answerless_input_exercise.save()
+        cc.AssessmentItem.objects.create(
+            contentnode=perseus_answerless_input_exercise,
+            assessment_id=uuid.uuid4().hex,
+            type=exercises.PERSEUS_QUESTION,
+            question="",
+            answers="[]",
+            hints="[]",
+            raw_data="{}",
+            order=1,
+            randomize=False,
+        )
+        cc.AssessmentItem.objects.create(
+            contentnode=perseus_answerless_input_exercise,
+            assessment_id=uuid.uuid4().hex,
+            type=exercises.INPUT_QUESTION,
+            question="What is 2+3?",
+            answers=json.dumps([{"answer": "", "correct": True, "order": 1}]),
+            hints="[]",
+            raw_data="{}",
+            order=2,
+            randomize=False,
+        )
+
+        # Dual-published: derivable native QTI item plus an answerless input question
+        native_qti_answerless_input_exercise = create_node(
+            {
+                "kind_id": "exercise",
+                "title": "Native QTI Answerless Input Exercise",
+                "extra_fields": qti_extra_fields,
+            }
+        )
+        native_qti_answerless_input_exercise.complete = True
+        native_qti_answerless_input_exercise.parent = current_exercise.parent
+        native_qti_answerless_input_exercise.save()
+        cc.AssessmentItem.objects.create(
+            contentnode=native_qti_answerless_input_exercise,
+            assessment_id=uuid.uuid4().hex,
+            type=exercises.QTI,
+            question="",
+            answers="[]",
+            hints="[]",
+            raw_data=VALID_CHOICE_ITEM,
+            order=1,
+            randomize=False,
+        )
+        cc.AssessmentItem.objects.create(
+            contentnode=native_qti_answerless_input_exercise,
+            assessment_id=uuid.uuid4().hex,
+            type=exercises.INPUT_QUESTION,
+            question="What is 2+3?",
+            answers=json.dumps([{"answer": "", "correct": True, "order": 1}]),
+            hints="[]",
+            raw_data="{}",
+            order=2,
+            randomize=False,
+        )
+        cc.AssessmentItem.objects.create(
+            contentnode=native_qti_answerless_input_exercise,
+            assessment_id=uuid.uuid4().hex,
+            type=exercises.INPUT_QUESTION,
+            question="What is 2+3?",
+            answers=json.dumps([{"answer": "5", "correct": True, "order": 1}]),
+            hints="[]",
+            raw_data="{}",
+            order=3,
+            randomize=False,
+        )
+
         # A node mixing a native QTI item with a raw perseus_question item ->
         # must route to a single QTI package (Perseus embedded as custom
         # interactions), not a separate Perseus archive.
@@ -1104,18 +1185,24 @@ def item_contents(title):
         self.assertTrue(commented)
         self.assertEqual(commented, item_contents("Native QTI Exercise"))
 
-    def test_native_qti_perseus_ids_match_assessment_metadata(self):
-        """The derived Perseus item JSON filenames must equal the ids recorded
-        in the published node's ``AssessmentMetaData.assessment_item_ids`` (the
-        QTI manifest ``K``-ids), so older Kolibri resolves the derived items."""
-        node = cc.ContentNode.objects.get(title="Native QTI Exercise")
+    def _read_perseus_archive(self, node):
         exercise_file = node.files.get(preset_id=format_presets.EXERCISE)
-        with exercise_file.file_on_disk.open("rb") as file_handle:
+        with exercise_file.file_on_disk.open("rb") as file_handle, zipfile.ZipFile(
+            file_handle
+        ) as archive:
             item_stems = {
                 name[: -len(".json")]
-                for name in zipfile.ZipFile(file_handle).namelist()
+                for name in archive.namelist()
                 if name.endswith(".json") and name != "exercise.json"
             }
+            return item_stems, json.loads(archive.read("exercise.json"))
+
+    def test_native_qti_perseus_ids_match_assessment_metadata(self):
+        """The derived Perseus item JSON filenames must equal the ids recorded
+        in the published node's ``AssessmentMetaData.assessment_item_ids`` (the
+        QTI manifest ``K``-ids), so older Kolibri resolves the derived items."""
+        node = cc.ContentNode.objects.get(title="Native QTI Exercise")
+        item_stems, _ = self._read_perseus_archive(node)
 
         published_node = kolibri_models.ContentNode.objects.get(
             title="Native QTI Exercise"
@@ -1131,15 +1218,7 @@ def test_mixed_native_legacy_perseus_ids_match_assessment_metadata(self):
         """Older Kolibri looks up legacy items by the QTI manifest id too."""
         title = "Native QTI + Legacy Mixed Exercise"
         node = cc.ContentNode.objects.get(title=title)
-        exercise_file = node.files.get(preset_id=format_presets.EXERCISE)
-        with exercise_file.file_on_disk.open("rb") as file_handle:
-            archive = zipfile.ZipFile(file_handle)
-            item_stems = {
-                name[: -len(".json")]
-                for name in archive.namelist()
-                if name.endswith(".json") and name != "exercise.json"
-            }
-            exercise_data = json.loads(archive.read("exercise.json"))
+        item_stems, exercise_data = self._read_perseus_archive(node)
 
         assessment_item_ids = (
             kolibri_models.ContentNode.objects.get(title=title)
@@ -1206,6 +1285,75 @@ def test_perseus_question_item_routes_to_perseus_packaging(self):
         self.assertTrue(node.files.filter(preset_id=format_presets.EXERCISE).exists())
         self.assertFalse(node.files.filter(preset_id=format_presets.QTI_ZIP).exists())
 
+    def test_answerless_input_left_out_of_perseus_archive_and_metadata(self):
+        title = "Perseus Answerless Input Exercise"
+        node = cc.ContentNode.objects.get(title=title)
+        perseus_id = node.assessment_items.get(
+            type=exercises.PERSEUS_QUESTION
+        ).assessment_id
+        self.assertFalse(node.files.filter(preset_id=format_presets.QTI_ZIP).exists())
+        item_stems, exercise_data = self._read_perseus_archive(node)
+        self.assertEqual(item_stems, {perseus_id})
+        self.assertEqual(exercise_data["all_assessment_items"], [perseus_id])
+
+        published_node = kolibri_models.ContentNode.objects.get(title=title)
+        self.assertEqual(
+            published_node.assessmentmetadata.first().assessment_item_ids,
+            [perseus_id],
+        )
+
+    def test_unchanged_republish_metadata_leaves_out_answerless_input(self):
+        # An unchanged node keeps its archive, which may predate the skip.
+        title = "Perseus Answerless Input Exercise"
+        node = cc.ContentNode.objects.get(title=title)
+        all_ids = list(
+            node.assessment_items.order_by("order").values_list(
+                "assessment_id", flat=True
+            )
+        )
+        perseus_id = node.assessment_items.get(
+            type=exercises.PERSEUS_QUESTION
+        ).assessment_id
+        stale_archive = io.BytesIO()
+        with zipfile.ZipFile(stale_archive, "w") as archive:
+            archive.writestr(
+                "exercise.json", json.dumps({"all_assessment_items": all_ids})
+            )
+            for assessment_id in all_ids:
+                archive.writestr(f"{assessment_id}.json", "{}")
+        node.files.filter(preset_id=format_presets.EXERCISE).delete()
+        stale_file = create_studio_file(
+            stale_archive.getvalue(), preset=format_presets.EXERCISE, ext="perseus"
+        )["db_file"]
+        stale_file.contentnode = node
+        stale_file.save()
+        published_node = kolibri_models.ContentNode.objects.get(title=title)
+        published_node.assessmentmetadata.all().delete()
+
+        create_kolibri_assessment_metadata(node, published_node)
+
+        self.assertEqual(
+            published_node.assessmentmetadata.get().assessment_item_ids,
+            [perseus_id],
+        )
+
+    def test_answerless_input_left_out_of_dual_published_archives(self):
+        title = "Native QTI Answerless Input Exercise"
+        node = cc.ContentNode.objects.get(title=title)
+        answerless_id = node.assessment_items.get(
+            type=exercises.INPUT_QUESTION, order=2
+        ).assessment_id
+        item_stems, exercise_data = self._read_perseus_archive(node)
+        self.assertEqual(len(item_stems), 2)
+        self.assertNotIn(hex_to_qti_id(answerless_id), item_stems)
+        self.assertEqual(set(exercise_data["all_assessment_items"]), item_stems)
+
+        published_node = kolibri_models.ContentNode.objects.get(title=title)
+        self.assertEqual(
+            set(published_node.assessmentmetadata.first().assessment_item_ids),
+            item_stems,
+        )
+
     def test_mixed_perseus_and_native_qti_routes_to_qti(self):
         node = cc.ContentNode.objects.get(title="Perseus + Native QTI Mixed Exercise")
         self.assertTrue(node.files.filter(preset_id=format_presets.QTI_ZIP).exists())

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: