diff --git a/contentcuration/contentcuration/tests/test_exportchannel.py b/contentcuration/contentcuration/tests/test_exportchannel.py index a296d08967..b7ab99beda 100644 --- a/contentcuration/contentcuration/tests/test_exportchannel.py +++ b/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 @@ -85,6 +87,25 @@ # Perseus input questions are numeric-only, so this publishes QTI only. STRING_ENTRY_QTI_ITEM = _text_item("single", ["cat"], base_type="string") +# Legacy input answers Perseus can't render as a finite number, or QTI converts +# to a string entry. +UNREADABLE_PERSEUS_INPUT_ANSWERS = ["Sphere", "1e400", "+50%"] + +# Publishing strips a zero-size image. +ZERO_SIZE_IMAGE = ( + f"![]({exercises.CONTENT_STORAGE_FORMAT.format('a' * 32 + '.png')} =0x0)" +) + +# Legacy input answers Perseus can render, and the numbers it renders. +# Publishing drops the image-only answer. +READABLE_PERSEUS_INPUT_ANSWERS = [ + (["+3"], [3]), + ([" +3"], [3]), + (["1E3"], [1000]), + ([f"4{ZERO_SIZE_IMAGE}"], [4]), + ([ZERO_SIZE_IMAGE, "4"], [4]), +] + # Larger than the signed 32-bit maximum (2_147_483_647); ~3 GB. LARGE_FILE_SIZE = 3 * 1024 ** 3 @@ -455,6 +476,98 @@ 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, + ) + 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": ZERO_SIZE_IMAGE, "correct": True, "order": 1}] + ), + hints="[]", + raw_data="{}", + order=4, + 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. @@ -525,6 +638,55 @@ def setUp(self): randomize=False, ) + # Mixed nodes with legacy input questions -> QTI only if Perseus can't + # read an answer, so each unreadable answer needs a node of its own + mixed_input_answers = { + f"Native QTI + Input {answer!r} Mixed Exercise": [[answer]] + for answer in UNREADABLE_PERSEUS_INPUT_ANSWERS + } + mixed_input_answers["Native QTI + Readable Inputs Mixed Exercise"] = [ + answers for answers, _ in READABLE_PERSEUS_INPUT_ANSWERS + ] + for title, input_answers in mixed_input_answers.items(): + mixed_exercise = create_node( + { + "kind_id": "exercise", + "title": title, + "extra_fields": qti_extra_fields, + } + ) + mixed_exercise.complete = True + mixed_exercise.parent = current_exercise.parent + mixed_exercise.save() + cc.AssessmentItem.objects.create( + contentnode=mixed_exercise, + assessment_id=uuid.uuid4().hex, + type=exercises.QTI, + question="", + answers="[]", + hints="[]", + raw_data=VALID_CHOICE_ITEM, + order=1, + randomize=False, + ) + for item_order, answers in enumerate(input_answers, start=2): + cc.AssessmentItem.objects.create( + contentnode=mixed_exercise, + assessment_id=uuid.uuid4().hex, + type=exercises.INPUT_QUESTION, + question="What is the answer?", + answers=json.dumps( + [ + {"answer": answer, "correct": True, "order": order} + for order, answer in enumerate(answers, start=1) + ] + ), + hints="[]", + raw_data="{}", + order=item_order, + randomize=False, + ) + first_topic = self.content_channel.main_tree.get_descendants().first() # Add a publishable topic to ensure it does not inherit but that its children do @@ -1104,18 +1266,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 +1299,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) @@ -1154,6 +1314,40 @@ def test_mixed_native_legacy_perseus_ids_match_assessment_metadata(self): set(exercise_data["assessment_mapping"]), set(assessment_item_ids) ) + def test_dual_published_numeric_inputs_have_answers(self): + dual_published = cc.ContentNode.objects.filter( + files__preset_id=format_presets.QTI_ZIP + ).filter(files__preset_id=format_presets.EXERCISE) + mixed_answers = {} + for node in dual_published: + exercise_file = node.files.get(preset_id=format_presets.EXERCISE) + with exercise_file.file_on_disk.open("rb") as file_handle, zipfile.ZipFile( + file_handle + ) as archive: + for name in archive.namelist(): + if not name.endswith(".json") or name == "exercise.json": + continue + widgets = json.loads(archive.read(name))["question"]["widgets"] + for widget in widgets.values(): + if widget["type"] != "numeric-input": + continue + answers = [ + answer["value"] for answer in widget["options"]["answers"] + ] + with self.subTest(node=node.title, item=name): + self.assertTrue(answers) + mixed_answers.setdefault(node.title, []).append(answers) + + expected = { + "Native QTI + Legacy Mixed Exercise": [[4]], + "Native QTI + Readable Inputs Mixed Exercise": [ + values for _, values in READABLE_PERSEUS_INPUT_ANSWERS + ], + } + for title, answers in expected.items(): + with self.subTest(node=title): + self.assertCountEqual(mixed_answers.get(title), answers) + def test_native_qti_item_declares_the_node_language(self): """The editor has no language of its own to write, so publishing supplies it. @@ -1196,6 +1390,19 @@ def test_native_qti_string_entry_publishes_qti_only(self): self.assertTrue(node.files.filter(preset_id=format_presets.QTI_ZIP).exists()) self.assertFalse(node.files.filter(preset_id=format_presets.EXERCISE).exists()) + def test_mixed_native_unreadable_input_publishes_qti_only(self): + for answer in UNREADABLE_PERSEUS_INPUT_ANSWERS: + with self.subTest(answer=answer): + node = cc.ContentNode.objects.get( + title=f"Native QTI + Input {answer!r} Mixed Exercise" + ) + self.assertTrue( + node.files.filter(preset_id=format_presets.QTI_ZIP).exists() + ) + self.assertFalse( + node.files.filter(preset_id=format_presets.EXERCISE).exists() + ) + def test_legacy_items_without_perseus_question_route_to_qti_packaging(self): node = cc.ContentNode.objects.get(title="Legacy No Perseus Exercise") self.assertTrue(node.files.filter(preset_id=format_presets.QTI_ZIP).exists()) @@ -1206,6 +1413,76 @@ 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_ids = node.assessment_items.filter( + type=exercises.INPUT_QUESTION, order__in=[2, 4] + ).values_list("assessment_id", flat=True) + item_stems, exercise_data = self._read_perseus_archive(node) + self.assertEqual(len(item_stems), 2) + for answerless_id in answerless_ids: + 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()) diff --git a/contentcuration/contentcuration/tests/test_restore_channel.py b/contentcuration/contentcuration/tests/test_restore_channel.py index faa1130689..6e2010d170 100644 --- a/contentcuration/contentcuration/tests/test_restore_channel.py +++ b/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: diff --git a/contentcuration/contentcuration/tests/utils/qti/test_perseus_derive.py b/contentcuration/contentcuration/tests/utils/qti/test_perseus_derive.py index f91d8dc250..bad9f9b516 100644 --- a/contentcuration/contentcuration/tests/utils/qti/test_perseus_derive.py +++ b/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) diff --git a/contentcuration/contentcuration/tests/utils/test_exercise_creation.py b/contentcuration/contentcuration/tests/utils/test_exercise_creation.py index 0da2360e56..cad87fc531 100644 --- a/contentcuration/contentcuration/tests/utils/test_exercise_creation.py +++ b/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("", "￿") 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,7 +2987,7 @@ 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()) @@ -2922,7 +2995,7 @@ def test_native_custom_processed_float_entry_is_packaged(self): 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]) diff --git a/contentcuration/contentcuration/utils/assessment/base.py b/contentcuration/contentcuration/utils/assessment/base.py index 6ff22f5517..d7cd037b16 100644 --- a/contentcuration/contentcuration/utils/assessment/base.py +++ b/contentcuration/contentcuration/utils/assessment/base.py @@ -47,6 +47,38 @@ image_pattern = rf"!\[([^\]]*)]\(\${exercises.CONTENT_STORAGE_PLACEHOLDER}/([^\s)]+)(?:\s=([0-9\.]+)x([0-9\.]+))*[^)]*\)" +def _image_size(img_match): + return tuple(float(size) if size else None for size in img_match.group(3, 4)) + + +def _is_checksum_filename(filename): + checksum, ext = os.path.splitext(filename) + try: + int(checksum, 16) + except ValueError: + return False + return bool(ext) + + +def _strip_unpackageable_images(content): + def _strip(img_match): + packageable = _is_checksum_filename(img_match.group(2)) and ( + 0 not in _image_size(img_match) + ) + return img_match.group(0) if packageable else "" + + return re.sub(image_pattern, _strip, content) + + +def packageable_answers(assessment_item): + """The item's answers less the image references ``process_image_strings`` drops.""" + answers = json.loads(assessment_item.answers) + for answer in answers: + if isinstance(answer.get("answer"), str): + answer["answer"] = _strip_unpackageable_images(answer["answer"]) + return answers + + def _exif_orientation(img): try: return img.getexif().get(ExifTags.Base.Orientation) @@ -501,8 +533,10 @@ def _process_answers(self, assessment_item): processed_answers = [] for answer in answer_data: - if answer["answer"]: - if isinstance(answer["answer"], str): + # A JSON 0 is a valid input answer; each format drops the blank + # ones itself. + if answer.get("answer") or assessment_item.type == exercises.INPUT_QUESTION: + if isinstance(answer.get("answer"), str): (answer["answer"], answer_images,) = self._process_content( answer["answer"], ) diff --git a/contentcuration/contentcuration/utils/assessment/perseus.py b/contentcuration/contentcuration/utils/assessment/perseus.py index adf0c4fb25..451b228901 100644 --- a/contentcuration/contentcuration/utils/assessment/perseus.py +++ b/contentcuration/contentcuration/utils/assessment/perseus.py @@ -1,23 +1,54 @@ import copy import json -import math +import logging import re import zipfile from django.template.loader import render_to_string +from django.utils.functional import cached_property +from le_utils.constants import content_kinds from le_utils.constants import exercises from le_utils.constants import file_formats from le_utils.constants import format_presets from contentcuration.utils.assessment.base import ExerciseArchiveGenerator +from contentcuration.utils.assessment.base import packageable_answers +from contentcuration.utils.assessment.qti.convert import accepted_answers +from contentcuration.utils.assessment.qti.convert import format_number from contentcuration.utils.assessment.qti.convert import hex_to_qti_id +from contentcuration.utils.assessment.qti.convert import is_answerless_input from contentcuration.utils.assessment.qti.perseus_derive import derive_perseus_item -from contentcuration.utils.parser import extract_value +logger = logging.getLogger(__name__) + _DOUBLE_DOLLAR_RE = re.compile(r"\$\$(.+?)\$\$", flags=re.DOTALL) +def _perseus_input_value(answer): + """The number a Perseus numeric-input renders for a legacy input answer, or None.""" + number = format_number(str(answer).strip()) + return float(number) if number else None + + +def is_perseus_readable_input_item(assessment_item): + """Whether Perseus renders every accepted answer of a legacy input question.""" + return all( + _perseus_input_value(answer) is not None + for answer in accepted_answers(packageable_answers(assessment_item)) + ) + + +def answerless_input_ids(ccnode): + """Input questions an exercise's Perseus archive leaves out, judged from + stored answers, as exercise.json is written before any item is processed.""" + return { + item.assessment_id + for item in ccnode.assessment_items.filter(type=exercises.INPUT_QUESTION) + if is_answerless_input(item.type, packageable_answers(item)) + } + + class PerseusExerciseGenerator(ExerciseArchiveGenerator): """ Exercise zip generator for Perseus format exercises. @@ -56,6 +87,19 @@ def _derived_items(self): } return self._derived_cache + @cached_property + def _answerless_input_ids(self): + """Units keep them: their pre/post test lists every id.""" + if self.ccnode.kind_id != content_kinds.EXERCISE: + return set() + answerless_ids = answerless_input_ids(self.ccnode) + for assessment_id in answerless_ids: + logger.warning( + f"Input question {assessment_id} on node {self.ccnode.pk} " + f"has no correct answer and will be excluded from the Perseus archive" + ) + return answerless_ids + def _process_formulas(self, content): return _DOUBLE_DOLLAR_RE.sub(r"$\1$", content) @@ -64,6 +108,8 @@ def _process_content(self, content): return super()._process_content(content) def process_assessment_item(self, assessment_item): + if assessment_item.assessment_id in self._answerless_input_ids: + return if assessment_item.type == exercises.QTI: derived = self._derived_items().get(assessment_item.assessment_id) if derived is None: @@ -78,15 +124,13 @@ def process_assessment_item(self, assessment_item): assessment_item.assessment_id = hex_to_qti_id(assessment_item.assessment_id) return super().process_assessment_item(assessment_item) - def _process_input_answers(self, processed_data): - """Extract input answer processing logic""" - numeric_answers = [] - for answer in processed_data["answers"]: - answer["answer"] = extract_value(answer["answer"]) - if answer["answer"] is not None and math.isfinite(answer["answer"]): - numeric_answers.append(answer) - - return {**processed_data, "answers": numeric_answers} + def _process_answers(self, assessment_item): + answers = super()._process_answers(assessment_item) + if assessment_item.type != exercises.INPUT_QUESTION: + return answers + # The template marks every rendered answer correct. + values = map(_perseus_input_value, accepted_answers(answers)) + return [{"answer": value} for value in values if value is not None] def create_assessment_item(self, assessment_item, processed_data): template = self.TEMPLATE_MAP.get(assessment_item.type) @@ -95,10 +139,6 @@ def create_assessment_item(self, assessment_item, processed_data): f"Unrecognized question type on item {assessment_item.assessment_id}: {assessment_item.type}" ) - # Handle input question special case - if assessment_item.type == exercises.INPUT_QUESTION: - processed_data = self._process_input_answers(processed_data) - filename = f"{assessment_item.assessment_id}.json" content = render_to_string(template, processed_data).encode("utf-8", "ignore") return filename, content @@ -118,14 +158,16 @@ def _exercise_data_for_archive(self): and raises ``FileNotFoundError``, and ``generate_assessment_item`` receives a ``qti`` type it cannot map. """ - derived = self._derived_items() - if not derived: - return self.exercise_data - + assessment_ids = [ + assessment_id + for assessment_id in self.exercise_data.get("all_assessment_items", []) + if assessment_id not in self._answerless_input_ids + ] original_mapping = self.exercise_data.get("assessment_mapping", {}) + derived = self._derived_items() new_ids = [] new_mapping = {} - for assessment_id in self.exercise_data.get("all_assessment_items", []): + for assessment_id in assessment_ids: if assessment_id in derived: proxy = derived[assessment_id] if proxy is None: @@ -135,10 +177,10 @@ def _exercise_data_for_archive(self): new_ids.append(proxy.assessment_id) new_mapping[proxy.assessment_id] = proxy.type else: - qti_id = hex_to_qti_id(assessment_id) - new_ids.append(qti_id) + item_id = hex_to_qti_id(assessment_id) if derived else assessment_id + new_ids.append(item_id) if assessment_id in original_mapping: - new_mapping[qti_id] = original_mapping[assessment_id] + new_mapping[item_id] = original_mapping[assessment_id] return { **self.exercise_data, "all_assessment_items": new_ids, diff --git a/contentcuration/contentcuration/utils/assessment/qti/archive.py b/contentcuration/contentcuration/utils/assessment/qti/archive.py index a979125658..dfb2ef5ae6 100644 --- a/contentcuration/contentcuration/utils/assessment/qti/archive.py +++ b/contentcuration/contentcuration/utils/assessment/qti/archive.py @@ -14,7 +14,6 @@ from contentcuration.utils.assessment.base import ExerciseArchiveGenerator from contentcuration.utils.assessment.qti.constants import ResourceType -from contentcuration.utils.assessment.qti.convert import accepted_answers from contentcuration.utils.assessment.qti.convert import ( build_perseus_custom_interaction_item, ) @@ -22,6 +21,7 @@ convert_legacy_assessment_item_to_qti, ) from contentcuration.utils.assessment.qti.convert import hex_to_qti_id +from contentcuration.utils.assessment.qti.convert import is_answerless_input from contentcuration.utils.assessment.qti.convert import LegacyAssessmentItem from contentcuration.utils.assessment.qti.imsmanifest import File as ManifestFile from contentcuration.utils.assessment.qti.imsmanifest import Manifest @@ -103,9 +103,7 @@ def _warn_answerless(self, assessment_item): def _skip_answerless_input(self, assessment_item, answers) -> bool: # Conversion makes an input question with no accepted answer an # answerless float entry. - is_answerless = assessment_item.type == exercises.INPUT_QUESTION and not ( - accepted_answers(answers) - ) + is_answerless = is_answerless_input(assessment_item.type, answers) if is_answerless: self._warn_answerless(assessment_item) return is_answerless @@ -219,19 +217,6 @@ def process_assessment_item(self, assessment_item): return None return super().process_assessment_item(assessment_item) - def _process_answers(self, assessment_item): - # The base drops every falsy answer, including a JSON 0; conversion - # drops the blank and false input answers itself. - if assessment_item.type != exercises.INPUT_QUESTION: - return super()._process_answers(assessment_item) - answers = json.loads(assessment_item.answers) - for answer in answers: - if isinstance(answer.get("answer"), str): - answer["answer"], answer["images"] = self._process_content( - answer["answer"] - ) - return self._sort_by_order(answers, "answers") - def _create_perseus_custom_interaction(self, assessment_item) -> None: """Embed a raw Perseus question as a ``qti-custom-interaction``. diff --git a/contentcuration/contentcuration/utils/assessment/qti/convert.py b/contentcuration/contentcuration/utils/assessment/qti/convert.py index 987211b9f7..c8aacd956c 100644 --- a/contentcuration/contentcuration/utils/assessment/qti/convert.py +++ b/contentcuration/contentcuration/utils/assessment/qti/convert.py @@ -266,11 +266,15 @@ def accepted_answers(answers: List[Dict[str, Any]]) -> List[str]: return accepted +def is_answerless_input(item_type: str, answers: List[Dict[str, Any]]) -> bool: + return item_type == exercises.INPUT_QUESTION and not accepted_answers(answers) + + # Decimal literals Kolibri's Number() reads that extract_value does not, e.g. +3, 1E3. _JS_NUMBER = re.compile(r"[+-]?(\d+\.?\d*|\.\d+)([eE][+-]?\d+)?", re.ASCII) -def _format_number(answer: str) -> Optional[str]: +def format_number(answer: str) -> Optional[str]: """ The answer in xsd:double form, or None if it is not a finite number. Written as JS Number#toString writes it, since Kolibri looks up map keys that way. @@ -352,7 +356,7 @@ def _create_text_entry_interaction_and_response( interaction = Div(children=prompt) answers = accepted_answers(item.answers) - numbers = [_format_number(answer) for answer in answers] + numbers = [format_number(answer) for answer in answers] # An answerless input question is float, so publish can tell it from free # response and skip it. is_numeric = all(numbers) and (answers or item.type == exercises.INPUT_QUESTION) diff --git a/contentcuration/contentcuration/utils/assessment/qti/perseus_derive.py b/contentcuration/contentcuration/utils/assessment/qti/perseus_derive.py index 9e5d955487..5f754447c6 100644 --- a/contentcuration/contentcuration/utils/assessment/qti/perseus_derive.py +++ b/contentcuration/contentcuration/utils/assessment/qti/perseus_derive.py @@ -22,7 +22,6 @@ from contentcuration.utils.assessment.qti.constants import BaseType from contentcuration.utils.assessment.qti.html_to_markdown import html_to_markdown from contentcuration.utils.assessment.qti.validation import parse_qti_xml -from contentcuration.utils.parser import extract_value logger = logging.getLogger(__name__) @@ -195,17 +194,10 @@ def _derive_choice(interaction, item_body, declaration): return item_type, question, answers -def _perseus_number(value): - """A leading "+" and capital "E" are valid xsd:double but not for - ``extract_value``.""" - normalised = value.removeprefix("+").lower() - return normalised if extract_value(normalised) is not None else value - - def _derive_text(interaction, item_body, declaration): question = html_to_markdown([item_body]) answers = [ - {"answer": _perseus_number(value), "correct": True, "order": order} + {"answer": value, "correct": True, "order": order} for order, value in enumerate(_accepted_values(declaration)) ] return exercises.INPUT_QUESTION, question, answers diff --git a/contentcuration/contentcuration/utils/publish.py b/contentcuration/contentcuration/utils/publish.py index 060bf63304..67d83f7626 100644 --- a/contentcuration/contentcuration/utils/publish.py +++ b/contentcuration/contentcuration/utils/publish.py @@ -46,6 +46,8 @@ from contentcuration import models as ccmodels from contentcuration.decorators import delay_user_storage_calculation +from contentcuration.utils.assessment.perseus import answerless_input_ids +from contentcuration.utils.assessment.perseus import is_perseus_readable_input_item from contentcuration.utils.assessment.perseus import PerseusExerciseGenerator from contentcuration.utils.assessment.qti.archive import QTIExerciseGenerator from contentcuration.utils.assessment.qti.imsmanifest import ( @@ -271,6 +273,11 @@ def _node_is_perseus_derivable(node): return False elif item.type not in PERSEUS_EXPRESSIBLE_LEGACY_TYPES: return False + elif ( + item.type == exercises.INPUT_QUESTION + and not is_perseus_readable_input_item(item) + ): + return False return has_native_qti @@ -838,15 +845,21 @@ def process_assessment_metadata(ccnode): def create_kolibri_assessment_metadata(ccnode, kolibrinode): assessment_items = ccnode.assessment_items.all().order_by("order") - assessment_item_ids = [a.assessment_id for a in assessment_items] randomize, _, mastery_model = _get_exercise_data_from_ccnode( - ccnode, len(assessment_item_ids) + ccnode, len(assessment_items) ) qti_file = ccnode.files.filter(preset_id=format_presets.QTI_ZIP).first() if qti_file: # Open the zip file from Django storage with qti_file.file_on_disk.open("rb") as file_handle: assessment_item_ids = get_assessment_ids_from_manifest(file_handle) + else: + answerless_ids = answerless_input_ids(ccnode) + assessment_item_ids = [ + a.assessment_id + for a in assessment_items + if a.assessment_id not in answerless_ids + ] kolibrimodels.AssessmentMetaData.objects.create( id=uuid.uuid4(),