From 4e9039ffbb462150a972dd2e3ceef6b836b9de9d Mon Sep 17 00:00:00 2001 From: rtibblesbot Date: Mon, 26 Oct 2026 17:08:55 -0700 Subject: [PATCH 1/2] Read derived numeric answers as finite xsd:double literals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `1_000`, `٥`, `５`, `1/2`, `1,000` and `50%` no longer derive a Perseus `numeric-input` with no answers. Co-Authored-By: Claude Opus 5.5 --- .../tests/utils/qti/test_perseus_derive.py | 14 +++++++++++++- .../utils/assessment/qti/convert.py | 4 ++++ .../utils/assessment/qti/perseus_derive.py | 9 ++------- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/contentcuration/contentcuration/tests/utils/qti/test_perseus_derive.py b/contentcuration/contentcuration/tests/utils/qti/test_perseus_derive.py index bad9f9b516..8449a4a44a 100644 --- a/contentcuration/contentcuration/tests/utils/qti/test_perseus_derive.py +++ b/contentcuration/contentcuration/tests/utils/qti/test_perseus_derive.py @@ -63,6 +63,7 @@ def _text_item( prompt="

What is 6 times 7?

", base_type="float", map_entries=(), + identifier="item_text", ): correct = "" if correct_values: @@ -78,7 +79,7 @@ def _text_item( ) ) return _item_xml( - "item_text", + identifier, "Text Item", '{}{}'.format( @@ -510,6 +511,17 @@ def test_comment_inside_prompt_markup_does_not_change_question(prompt): pytest.param( _text_item("single", ["1" + "0" * 400]), True, id="float_overflow_integer" ), + pytest.param(_text_item("single", ["1_000"]), True, id="float_underscore"), + pytest.param( + _text_item("single", ["1_0E3"]), True, id="float_underscore_exponent" + ), + pytest.param(_text_item("single", ["٥"]), True, id="float_arabic_indic"), + pytest.param(_text_item("single", ["５"]), True, id="float_fullwidth"), + pytest.param(_text_item("single", ["1/2"]), True, id="float_fraction"), + pytest.param(_text_item("single", ["1,000"]), True, id="float_comma"), + pytest.param(_text_item("single", ["50%"]), True, id="float_percent"), + pytest.param(_text_item("single", ["+3"]), False, id="float_plus_sign"), + pytest.param(_text_item("single", ["1E3"]), False, id="float_exponent"), pytest.param( _text_item("single", [], map_entries=[("13", "1.0"), ("12", "1.0")]), False, diff --git a/contentcuration/contentcuration/utils/assessment/qti/convert.py b/contentcuration/contentcuration/utils/assessment/qti/convert.py index c8aacd956c..2b90424308 100644 --- a/contentcuration/contentcuration/utils/assessment/qti/convert.py +++ b/contentcuration/contentcuration/utils/assessment/qti/convert.py @@ -292,6 +292,10 @@ def format_number(answer: str) -> Optional[str]: return f"{mantissa}e{int(exponent):+d}" +def is_finite_double(value: str) -> bool: + return bool(_JS_NUMBER.fullmatch(value)) and format_number(value) is not None + + def _create_choice_interaction_and_response( item: LegacyAssessmentItem, ) -> Tuple[ChoiceInteraction, ResponseDeclaration]: diff --git a/contentcuration/contentcuration/utils/assessment/qti/perseus_derive.py b/contentcuration/contentcuration/utils/assessment/qti/perseus_derive.py index 5f754447c6..74bd3818bd 100644 --- a/contentcuration/contentcuration/utils/assessment/qti/perseus_derive.py +++ b/contentcuration/contentcuration/utils/assessment/qti/perseus_derive.py @@ -10,7 +10,6 @@ """ import json import logging -import math from dataclasses import dataclass from typing import List from typing import Optional @@ -20,6 +19,7 @@ from contentcuration.utils.assessment.qti.catalog import KOLIBRI_HINT_SUPPORT from contentcuration.utils.assessment.qti.constants import BaseType +from contentcuration.utils.assessment.qti.convert import is_finite_double from contentcuration.utils.assessment.qti.html_to_markdown import html_to_markdown from contentcuration.utils.assessment.qti.validation import parse_qti_xml @@ -137,7 +137,7 @@ def _accepted_values(declaration) -> List[str]: for entry in _children_by_localname(mappings[0], "qti-map-entry") if (_as_float(entry.get("mapped-value")) or 0) >= 1 ] - return list(dict.fromkeys(value for value in values if _is_number(value))) + return list(dict.fromkeys(value for value in values if is_finite_double(value))) def _as_float(value) -> Optional[float]: @@ -147,11 +147,6 @@ def _as_float(value) -> Optional[float]: return None -def _is_number(value) -> bool: - number = _as_float(value) - return number is not None and math.isfinite(number) - - def _children_by_localname(el, localname): return [child for child in el if _local(child) == localname] From 00c4fc0e3ce724906ff0284bc0c64185f543a022 Mon Sep 17 00:00:00 2001 From: rtibblesbot Date: Mon, 26 Oct 2026 17:08:56 -0700 Subject: [PATCH 2/2] Route exercises to the archives their items can be expressed in - Perseus archive ships when every item has a Perseus equivalent; QTI package ships for native QTI items or items without one. - When both ship, Perseus items are named by their QTI manifest ids. - Unit pre/post test lists are rewritten to the ids the package ships, leaving out skipped items. Co-Authored-By: Claude Opus 5.5 --- .../tests/test_exportchannel.py | 913 ++++++++++++++---- .../tests/test_restore_channel.py | 18 +- .../tests/utils/qti/test_perseus_derive.py | 57 +- .../tests/utils/test_exercise_creation.py | 7 +- .../utils/assessment/perseus.py | 29 +- .../utils/assessment/qti/archive.py | 70 +- .../utils/assessment/qti/perseus_derive.py | 23 +- .../contentcuration/utils/publish.py | 174 ++-- 8 files changed, 971 insertions(+), 320 deletions(-) diff --git a/contentcuration/contentcuration/tests/test_exportchannel.py b/contentcuration/contentcuration/tests/test_exportchannel.py index b7ab99beda..cc634d447a 100644 --- a/contentcuration/contentcuration/tests/test_exportchannel.py +++ b/contentcuration/contentcuration/tests/test_exportchannel.py @@ -40,6 +40,8 @@ from .testdata import thumbnail_bytes from .testdata import tree from .utils.qti.test_perseus_derive import _text_item +from .utils.qti.test_perseus_derive import MATCH_CORRECT_PROCESSING +from .utils.qti.test_perseus_derive import TOLERANCE_PROCESSING from .utils.qti.test_validation import _item_xml from .utils.qti.test_validation import ENTITY_CHOICE_ITEM from .utils.qti.test_validation import VALID_CHOICE_ITEM @@ -47,6 +49,9 @@ from contentcuration import models as cc from contentcuration.models import CustomTaskMetadata from contentcuration.utils.assessment.qti.archive import hex_to_qti_id +from contentcuration.utils.assessment.qti.imsmanifest import ( + get_assessment_ids_from_manifest, +) from contentcuration.utils.assessment.qti.validation import parse_qti_xml from contentcuration.utils.assessment.qti.validation import validate_qti_item from contentcuration.utils.celery.tasks import generate_task_signature @@ -54,7 +59,6 @@ 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 @@ -106,6 +110,30 @@ ([ZERO_SIZE_IMAGE, "4"], [4]), ] +# Native float answers Kolibri's Number() does not read. +UNREADABLE_NATIVE_FLOAT_ANSWERS = ["1_000", "1_0E3", "٥", "５", "1/2", "1,000", "50%"] + + +QTI_EXTRA_FIELDS = { + "options": { + "completion_criteria": { + "model": "mastery", + "threshold": {"m": 1, "n": 2, "mastery_model": exercises.M_OF_N}, + } + } +} + +NATIVE_LEAD = (exercises.QTI, VALID_CHOICE_ITEM) +PERSEUS_LEAD = (exercises.PERSEUS_QUESTION, "{}") +READABLE_INPUTS = [ + (exercises.INPUT_QUESTION, answers) for answers, _ in READABLE_PERSEUS_INPUT_ANSWERS +] + +# Schema-invalid, yet Perseus can still express it. +SCHEMA_INVALID_CHOICE_ITEM = VALID_CHOICE_ITEM.replace( + 'max-choices="1"', 'max-choices="abc"', 1 +) + # Larger than the signed 32-bit maximum (2_147_483_647); ~3 GB. LARGE_FILE_SIZE = 3 * 1024 ** 3 @@ -120,18 +148,174 @@ def description(): ) -class ExportChannelTestCase(StudioTestCase): +class ExportChannelTestBase(StudioTestCase): @classmethod def setUpClass(cls): - super(ExportChannelTestCase, cls).setUpClass() + super().setUpClass() cls.patch_copy_db = patch("contentcuration.utils.publish.save_export_database") cls.patch_copy_db.start() @classmethod def tearDownClass(cls): - super(ExportChannelTestCase, cls).tearDownClass() + super().tearDownClass() cls.patch_copy_db.stop() + def _create_native_qti_exercise(self, title, raw_items, parent): + exercise = create_node( + {"kind_id": "exercise", "title": title, "extra_fields": QTI_EXTRA_FIELDS}, + parent=parent, + ) + for item_order, raw_data in enumerate(raw_items, start=1): + cc.AssessmentItem.objects.create( + contentnode=exercise, + assessment_id=uuid.uuid4().hex, + type=exercises.QTI, + question="", + answers="[]", + hints="[]", + raw_data=raw_data, + order=item_order, + randomize=False, + ) + + def _create_mixed_exercise(self, title, lead, items, parent): + lead_type, lead_raw_data = lead + mixed_exercise = create_node( + {"kind_id": "exercise", "title": title, "extra_fields": QTI_EXTRA_FIELDS}, + parent=parent, + ) + cc.AssessmentItem.objects.create( + contentnode=mixed_exercise, + assessment_id=uuid.uuid4().hex, + type=lead_type, + question="", + answers="[]", + hints="[]", + raw_data=lead_raw_data, + order=1, + randomize=False, + ) + for item_order, (item_type, answers) in enumerate(items, start=2): + cc.AssessmentItem.objects.create( + contentnode=mixed_exercise, + assessment_id=uuid.uuid4().hex, + type=item_type, + 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, + ) + + def tearDown(self): + # Clean up datbase connection after the test + cleanup_content_database_connection(self.tempdb) + super().tearDown() + set_active_content_database(None) + if os.path.exists(self.tempdb): + os.remove(self.tempdb) + + def _read_qti_manifest_ids(self, node): + qti_file = node.files.get(preset_id=format_presets.QTI_ZIP) + with qti_file.file_on_disk.open("rb") as file_handle: + return get_assessment_ids_from_manifest(file_handle) + + 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, zipfile.ZipFile( + file_handle + ) as archive: + item_stems = { + name[: -len(".json")] + for name in archive.namelist() + if name.endswith(".json") and name != "exercise.json" + } + return item_stems, json.loads(archive.read("exercise.json")) + + def _numeric_input_answers(self, node): + 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 + item = json.loads(archive.read(name)) + widgets = item.get("question", {}).get("widgets", {}) + for widget in widgets.values(): + if widget["type"] == "numeric-input": + yield name, [ + answer["value"] for answer in widget["options"]["answers"] + ] + + def _assert_archive_ids_match_assessment_metadata(self, title, item_count): + node = cc.ContentNode.objects.get(title=title) + manifest_ids = self._read_qti_manifest_ids(node) + item_stems, exercise_data = self._read_perseus_archive(node) + + assessment_item_ids = ( + kolibri_models.ContentNode.objects.get(title=title) + .assessmentmetadata.first() + .assessment_item_ids + ) + + self.assertEqual(len(assessment_item_ids), item_count) + self.assertEqual(manifest_ids, assessment_item_ids) + self.assertEqual(item_stems, set(assessment_item_ids)) + self.assertEqual(exercise_data["all_assessment_items"], assessment_item_ids) + self.assertEqual( + set(exercise_data["assessment_mapping"]), set(assessment_item_ids) + ) + + def _store_archive(self, node, preset, item_ids): + archive_bytes = io.BytesIO() + with zipfile.ZipFile(archive_bytes, "w") as archive: + if preset == format_presets.QTI_ZIP: + resources = "".join( + f'' + for item_id in item_ids + ) + archive.writestr( + "imsmanifest.xml", + '' + f"{resources}", + ) + else: + archive.writestr( + "exercise.json", json.dumps({"all_assessment_items": item_ids}) + ) + for item_id in item_ids: + archive.writestr(f"{item_id}.json", "{}") + self._attach_file(node, preset, archive_bytes.getvalue()) + + def _attach_file(self, node, preset, content): + node.files.filter(preset_id=preset).delete() + db_file = create_studio_file( + content, + preset=preset, + ext="zip" if preset == format_presets.QTI_ZIP else "perseus", + )["db_file"] + db_file.contentnode = node + db_file.save() + return db_file + + def _republish_unchanged(self, node): + cc.ContentNode.objects.filter(pk=node.pk).update(changed=False) + cleanup_content_database_connection(self.tempdb) + os.remove(self.tempdb) + self.tempdb = create_content_database( + self.content_channel, True, self.admin_user.id, False + ) + set_active_content_database(self.tempdb) + + +class ExportChannelTestCase(ExportChannelTestBase): def setUp(self): super(ExportChannelTestCase, self).setUp() self.content_channel = channel() @@ -277,23 +461,11 @@ def setUp(self): ai.save() # Add an exercise with free response question to test QTI generation - qti_extra_fields = { - "options": { - "completion_criteria": { - "model": "mastery", - "threshold": { - "m": 1, - "n": 2, - "mastery_model": exercises.M_OF_N, - }, - } - } - } qti_exercise = create_node( { "kind_id": "exercise", "title": "QTI Free Response Exercise", - "extra_fields": qti_extra_fields, + "extra_fields": QTI_EXTRA_FIELDS, } ) qti_exercise.complete = True @@ -323,7 +495,7 @@ def setUp(self): { "kind_id": "exercise", "title": "Native QTI Exercise", - "extra_fields": qti_extra_fields, + "extra_fields": QTI_EXTRA_FIELDS, } ) native_qti_exercise.complete = True @@ -345,7 +517,7 @@ def setUp(self): { "kind_id": "exercise", "title": "Native QTI Commented Exercise", - "extra_fields": qti_extra_fields, + "extra_fields": QTI_EXTRA_FIELDS, } ) native_qti_commented_exercise.complete = True @@ -368,7 +540,7 @@ def setUp(self): { "kind_id": "exercise", "title": "Native QTI Unsupported Exercise", - "extra_fields": qti_extra_fields, + "extra_fields": QTI_EXTRA_FIELDS, } ) native_qti_unsupported_exercise.complete = True @@ -390,7 +562,7 @@ def setUp(self): { "kind_id": "exercise", "title": "Native QTI Entity Exercise", - "extra_fields": qti_extra_fields, + "extra_fields": QTI_EXTRA_FIELDS, } ) native_qti_entity_exercise.complete = True @@ -412,7 +584,7 @@ def setUp(self): { "kind_id": "exercise", "title": "Native QTI String Entry Exercise", - "extra_fields": qti_extra_fields, + "extra_fields": QTI_EXTRA_FIELDS, } ) native_qti_string_entry_exercise.complete = True @@ -430,35 +602,52 @@ def setUp(self): randomize=False, ) - # Only legacy structured-field items, no perseus_question -> must now route to QTI (was Perseus) legacy_no_perseus_exercise = create_node( { "kind_id": "exercise", "title": "Legacy No Perseus Exercise", - "extra_fields": qti_extra_fields, + "extra_fields": QTI_EXTRA_FIELDS, } ) legacy_no_perseus_exercise.complete = True legacy_no_perseus_exercise.parent = current_exercise.parent legacy_no_perseus_exercise.save() - cc.AssessmentItem.objects.create( - contentnode=legacy_no_perseus_exercise, - assessment_id=uuid.uuid4().hex, - type=exercises.SINGLE_SELECTION, - question="What is 2+2?", - answers=json.dumps([{"answer": "4", "correct": True, "order": 1}]), - hints=json.dumps([]), - raw_data="{}", - order=1, - randomize=False, - ) + legacy_items = [ + (exercises.SINGLE_SELECTION, [{"answer": "4", "correct": True}]), + ( + exercises.MULTIPLE_SELECTION, + [{"answer": "4", "correct": True}, {"answer": "2*2", "correct": True}], + ), + ( + "true_false", + [{"answer": "True", "correct": True}, {"answer": "False"}], + ), + (exercises.INPUT_QUESTION, [{"answer": "4", "correct": True}]), + ] + for item_order, (item_type, answers) in enumerate(legacy_items, start=1): + cc.AssessmentItem.objects.create( + contentnode=legacy_no_perseus_exercise, + assessment_id=uuid.uuid4().hex, + type=item_type, + question="What is 2+2?", + answers=json.dumps( + [ + {"correct": False, **answer, "order": order} + for order, answer in enumerate(answers, start=1) + ] + ), + hints=json.dumps([]), + raw_data="{}", + order=item_order, + randomize=False, + ) # A perseus_question item present -> must still route to Perseus packaging perseus_only_exercise = create_node( { "kind_id": "exercise", "title": "Perseus Only Exercise", - "extra_fields": qti_extra_fields, + "extra_fields": QTI_EXTRA_FIELDS, } ) perseus_only_exercise.complete = True @@ -481,7 +670,7 @@ def setUp(self): { "kind_id": "exercise", "title": "Perseus Answerless Input Exercise", - "extra_fields": qti_extra_fields, + "extra_fields": QTI_EXTRA_FIELDS, } ) perseus_answerless_input_exercise.complete = True @@ -515,7 +704,7 @@ def setUp(self): { "kind_id": "exercise", "title": "Native QTI Answerless Input Exercise", - "extra_fields": qti_extra_fields, + "extra_fields": QTI_EXTRA_FIELDS, } ) native_qti_answerless_input_exercise.complete = True @@ -568,14 +757,11 @@ def setUp(self): 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. mixed_perseus_qti_exercise = create_node( { "kind_id": "exercise", "title": "Perseus + Native QTI Mixed Exercise", - "extra_fields": qti_extra_fields, + "extra_fields": QTI_EXTRA_FIELDS, } ) mixed_perseus_qti_exercise.complete = True @@ -609,7 +795,7 @@ def setUp(self): { "kind_id": "exercise", "title": "Native QTI + Legacy Mixed Exercise", - "extra_fields": qti_extra_fields, + "extra_fields": QTI_EXTRA_FIELDS, } ) native_qti_legacy_mixed_exercise.complete = True @@ -640,52 +826,19 @@ def setUp(self): # 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 answer in UNREADABLE_PERSEUS_INPUT_ANSWERS: + self._create_mixed_exercise( + f"Native QTI + Input {answer!r} Mixed Exercise", + NATIVE_LEAD, + [(exercises.INPUT_QUESTION, [answer])], + current_exercise.parent, ) - 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, - ) + self._create_mixed_exercise( + "Native QTI + Readable Inputs Mixed Exercise", + NATIVE_LEAD, + READABLE_INPUTS, + current_exercise.parent, + ) first_topic = self.content_channel.main_tree.get_descendants().first() @@ -741,6 +894,9 @@ def setUp(self): # Add a UNIT topic with directly attached assessment items unit_assessment_id_1 = uuid.uuid4().hex unit_assessment_id_2 = uuid.uuid4().hex + unit_native_id = uuid.uuid4().hex + unit_answerless_id = uuid.uuid4().hex + unit_lo_id = uuid.uuid4().hex unit_topic = create_node( {"kind_id": "topic", "title": "Test Unit Topic", "children": []}, @@ -757,12 +913,27 @@ def setUp(self): "assessment_item_ids": [ unit_assessment_id_1, unit_assessment_id_2, + unit_native_id, + unit_answerless_id, + ], + "version_a_item_ids": [ + unit_assessment_id_1, + unit_native_id, + ], + "version_b_item_ids": [ + unit_assessment_id_2, + unit_answerless_id, ], - "version_a_item_ids": [unit_assessment_id_1], - "version_b_item_ids": [unit_assessment_id_2], }, }, }, + "learning_objectives": [{"id": unit_lo_id, "text": "Add"}], + "assessment_objectives": { + unit_assessment_id_1: [unit_lo_id], + unit_assessment_id_2: [unit_lo_id], + unit_native_id: [unit_lo_id], + unit_answerless_id: [unit_lo_id], + }, } } unit_topic.save() @@ -801,6 +972,30 @@ def setUp(self): randomize=False, ) + cc.AssessmentItem.objects.create( + contentnode=unit_topic, + assessment_id=unit_native_id, + type=exercises.QTI, + question="", + answers="[]", + hints="[]", + raw_data=VALID_CHOICE_ITEM, + order=3, + randomize=False, + ) + + cc.AssessmentItem.objects.create( + contentnode=unit_topic, + assessment_id=unit_answerless_id, + type=exercises.INPUT_QUESTION, + question="What is 4+4?", + answers=json.dumps([{"answer": "", "correct": True, "order": 1}]), + hints=json.dumps([]), + raw_data="{}", + order=4, + randomize=False, + ) + # Add a LESSON child topic under the UNIT with a video child lesson_topic = create_node( { @@ -852,14 +1047,6 @@ def setUp(self): set_active_content_database(self.tempdb) - def tearDown(self): - # Clean up datbase connection after the test - cleanup_content_database_connection(self.tempdb) - super(ExportChannelTestCase, self).tearDown() - set_active_content_database(None) - if os.path.exists(self.tempdb): - os.remove(self.tempdb) - def test_channel_rootnode_data(self): channel = kolibri_models.ChannelMetadata.objects.first() self.assertEqual(channel.root_pk, channel.root_id) @@ -1230,6 +1417,9 @@ def test_qti_exercise_generates_qti_archive(self): qti_file.original_filename.endswith(".zip"), "QTI file should be a zip archive", ) + self.assertFalse( + qti_exercise.files.filter(preset_id=format_presets.EXERCISE).exists() + ) def test_native_qti_choice_item_publishes_both_archives(self): node = cc.ContentNode.objects.get(title="Native QTI Exercise") @@ -1266,18 +1456,6 @@ def item_contents(title): self.assertTrue(commented) self.assertEqual(commented, item_contents("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, zipfile.ZipFile( - file_handle - ) as archive: - item_stems = { - name[: -len(".json")] - 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 @@ -1297,52 +1475,27 @@ def test_native_qti_perseus_ids_match_assessment_metadata(self): 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) - item_stems, exercise_data = self._read_perseus_archive(node) - - assessment_item_ids = ( - kolibri_models.ContentNode.objects.get(title=title) - .assessmentmetadata.first() - .assessment_item_ids + self._assert_archive_ids_match_assessment_metadata( + "Native QTI + Legacy Mixed Exercise", 2 ) - self.assertEqual(len(assessment_item_ids), 2) - self.assertEqual(item_stems, set(assessment_item_ids)) - self.assertEqual(exercise_data["all_assessment_items"], assessment_item_ids) - self.assertEqual( - set(exercise_data["assessment_mapping"]), set(assessment_item_ids) + def test_published_numeric_inputs_have_answers(self): + perseus_published = cc.ContentNode.objects.filter( + files__preset_id=format_presets.EXERCISE ) - - 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) + for node in perseus_published: + for name, answers in self._numeric_input_answers(node): + 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 ], + "Legacy No Perseus Exercise": [[4]], } for title, answers in expected.items(): with self.subTest(node=title): @@ -1403,10 +1556,10 @@ def test_mixed_native_unreadable_input_publishes_qti_only(self): 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()) - self.assertFalse(node.files.filter(preset_id=format_presets.EXERCISE).exists()) + def test_legacy_items_without_perseus_question_publish_both_archives(self): + self._assert_archive_ids_match_assessment_metadata( + "Legacy No Perseus Exercise", 4 + ) def test_perseus_question_item_routes_to_perseus_packaging(self): node = cc.ContentNode.objects.get(title="Perseus Only Exercise") @@ -1442,29 +1595,41 @@ def test_unchanged_republish_metadata_leaves_out_answerless_input(self): 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() + self._store_archive(node, format_presets.EXERCISE, all_ids) - create_kolibri_assessment_metadata(node, published_node) + self._republish_unchanged(node) self.assertEqual( - published_node.assessmentmetadata.get().assessment_item_ids, + kolibri_models.ContentNode.objects.get(title=title) + .assessmentmetadata.get() + .assessment_item_ids, [perseus_id], ) + def test_unchanged_republish_rebuilds_hex_id_perseus_archive(self): + title = "Legacy No Perseus Exercise" + node = cc.ContentNode.objects.get(title=title) + hex_ids = list( + node.assessment_items.order_by("order").values_list( + "assessment_id", flat=True + ) + ) + node.files.all().delete() + self._store_archive(node, format_presets.EXERCISE, hex_ids) + + self._republish_unchanged(node) + + self._assert_archive_ids_match_assessment_metadata(title, 4) + + def test_unchanged_republish_does_not_read_stored_perseus_archive(self): + title = "Perseus Answerless Input Exercise" + node = cc.ContentNode.objects.get(title=title) + db_file = self._attach_file(node, format_presets.EXERCISE, b"not a zip") + + self._republish_unchanged(node) + + self.assertEqual(list(node.files.values_list("id", flat=True)), [db_file.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) @@ -1483,10 +1648,10 @@ def test_answerless_input_left_out_of_dual_published_archives(self): 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()) - self.assertFalse(node.files.filter(preset_id=format_presets.EXERCISE).exists()) + def test_mixed_perseus_and_native_qti_publishes_both_archives(self): + self._assert_archive_ids_match_assessment_metadata( + "Perseus + Native QTI Mixed Exercise", 2 + ) def test_qti_archive_contains_manifest_and_assessment_ids(self): @@ -1512,9 +1677,6 @@ def test_unit_topic_publishes_with_exercise_zip(self): attached assessment items compiled into a zip file during publishing.""" unit_topic = cc.ContentNode.objects.get(title="Test Unit Topic") - # Assert UNIT topic has a QTI archive file in Studio. Its assessment - # items are legacy structured-field (SINGLE_SELECTION) with no - # perseus_question item, so they route to QTI packaging. unit_files = cc.File.objects.filter( contentnode=unit_topic, preset_id=format_presets.QTI_ZIP, @@ -1533,6 +1695,409 @@ def test_unit_topic_publishes_with_exercise_zip(self): "UNIT topic should NOT have assessment metadata", ) + def test_unit_pre_post_test_lists_package_ids(self): + unit_topic = cc.ContentNode.objects.get(title="Test Unit Topic") + first, second = ( + hex_to_qti_id(item.assessment_id) + for item in unit_topic.assessment_items.filter( + type=exercises.SINGLE_SELECTION + ).order_by("order") + ) + published = kolibri_models.ContentNode.objects.get(title="Test Unit Topic") + pre_post_test = published.options["completion_criteria"]["threshold"][ + "pre_post_test" + ] + item_stems, _ = self._read_perseus_archive(unit_topic) + for key, expected in ( + ("assessment_item_ids", [first, second, "item_1"]), + ("version_a_item_ids", [first, "item_1"]), + ("version_b_item_ids", [second]), + ): + with self.subTest(key=key): + self.assertEqual(pre_post_test[key], expected) + self.assertLessEqual(set(expected), item_stems) + self.assertEqual( + pre_post_test["assessment_item_ids"], + self._read_qti_manifest_ids(unit_topic), + ) + self.assertEqual( + set(published.options["assessment_objectives"]), + set(pre_post_test["assessment_item_ids"]), + ) + + +class ExerciseArchiveRoutingTestCase(ExportChannelTestBase): + def setUp(self): + super().setUp() + self.content_channel = channel() + parent = self.content_channel.main_tree + + for answer in UNREADABLE_PERSEUS_INPUT_ANSWERS: + self._create_mixed_exercise( + f"Perseus + Input {answer!r} Exercise", + PERSEUS_LEAD, + [(exercises.INPUT_QUESTION, [answer])], + parent, + ) + self._create_mixed_exercise( + "Perseus + Readable Inputs Exercise", PERSEUS_LEAD, READABLE_INPUTS, parent + ) + self._create_mixed_exercise( + "Perseus + Free Response Exercise", + PERSEUS_LEAD, + [(exercises.FREE_RESPONSE, ["Paris"])], + parent, + ) + self._create_native_qti_exercise( + "Native QTI Schema Invalid Exercise", + [VALID_CHOICE_ITEM, SCHEMA_INVALID_CHOICE_ITEM], + parent, + ) + self._create_native_qti_exercise( + "Native QTI Unreadable Floats Exercise", + [ + VALID_CHOICE_ITEM, + _text_item("single", ["42", "1/2"], identifier="item_text_mixed"), + ] + + [ + _text_item("single", [answer], identifier=f"item_text_{index}") + for index, answer in enumerate(UNREADABLE_NATIVE_FLOAT_ANSWERS) + ], + parent, + ) + self._create_native_qti_exercise( + "Native QTI Unreadable Tolerance Exercise", + [ + VALID_CHOICE_ITEM, + _text_item("single", ["1/2"]).replace( + MATCH_CORRECT_PROCESSING, TOLERANCE_PROCESSING + ), + ], + parent, + ) + + shared_identifier_exercise = create_node( + { + "kind_id": "exercise", + "title": "Native QTI Shared Identifier Exercise", + "extra_fields": QTI_EXTRA_FIELDS, + }, + parent=parent, + ) + for item_order, (item_type, question, answers, raw_data) in enumerate( + [ + (exercises.QTI, "", [], _text_item("single", [])), + ( + exercises.INPUT_QUESTION, + "What is 2+2?", + [{"answer": 4, "correct": True, "order": 1}], + "{}", + ), + (exercises.QTI, "", [], _text_item("single", ["3"])), + ], + start=1, + ): + cc.AssessmentItem.objects.create( + contentnode=shared_identifier_exercise, + assessment_id=uuid.uuid4().hex, + type=item_type, + question=question, + answers=json.dumps(answers), + hints="[]", + raw_data=raw_data, + order=item_order, + randomize=False, + ) + + entity_unit_legacy_id = uuid.uuid4().hex + entity_unit_entity_id = uuid.uuid4().hex + entity_unit_ids = [entity_unit_legacy_id, entity_unit_entity_id] + entity_unit_topic = create_node( + { + "kind_id": "topic", + "title": "Entity Unit Topic", + "children": [ + {"kind_id": "video", "title": "Entity Unit Video", "children": []}, + ], + }, + parent=self.content_channel.main_tree, + ) + entity_unit_topic.extra_fields = { + "options": { + "modality": modalities.UNIT, + "completion_criteria": { + "model": "mastery", + "threshold": { + "mastery_model": exercises.PRE_POST_TEST, + "pre_post_test": { + "assessment_item_ids": entity_unit_ids, + "version_a_item_ids": entity_unit_ids, + "version_b_item_ids": entity_unit_ids, + }, + }, + }, + } + } + entity_unit_topic.save() + cc.AssessmentItem.objects.create( + contentnode=entity_unit_topic, + assessment_id=entity_unit_legacy_id, + type=exercises.SINGLE_SELECTION, + question="What is 5+5?", + answers=json.dumps( + [ + {"answer": "10", "correct": True, "order": 1}, + {"answer": "9", "correct": False, "order": 2}, + ] + ), + hints=json.dumps([]), + raw_data="{}", + order=1, + randomize=False, + ) + cc.AssessmentItem.objects.create( + contentnode=entity_unit_topic, + assessment_id=entity_unit_entity_id, + type=exercises.QTI, + question="", + answers="[]", + hints="[]", + raw_data=ENTITY_CHOICE_ITEM, + order=2, + randomize=False, + ) + + self._create_shared_identifier_unit() + + set_channel_icon_encoding(self.content_channel) + self.tempdb = create_content_database( + self.content_channel, True, self.admin_user.id, True + ) + set_active_content_database(self.tempdb) + + def _create_shared_identifier_unit(self): + shared_unit_ids = [uuid.uuid4().hex for _ in range(3)] + shared_unit_topic = create_node( + { + "kind_id": "topic", + "title": "Shared Identifier Unit Topic", + "children": [ + { + "kind_id": "video", + "title": "Shared Identifier Unit Video", + "children": [], + }, + ], + }, + parent=self.content_channel.main_tree, + ) + shared_unit_topic.extra_fields = { + "options": { + "modality": modalities.UNIT, + "completion_criteria": { + "model": "mastery", + "threshold": { + "mastery_model": exercises.PRE_POST_TEST, + "pre_post_test": { + "assessment_item_ids": shared_unit_ids, + "version_a_item_ids": shared_unit_ids[1:], + "version_b_item_ids": [ + shared_unit_ids[0], + shared_unit_ids[2], + ], + }, + }, + }, + } + } + shared_unit_topic.save() + for item_order, (assessment_id, raw_data) in enumerate( + zip( + shared_unit_ids[:2], + [_text_item("single", []), _text_item("single", ["3"])], + ), + start=1, + ): + cc.AssessmentItem.objects.create( + contentnode=shared_unit_topic, + assessment_id=assessment_id, + type=exercises.QTI, + question="", + answers="[]", + hints="[]", + raw_data=raw_data, + order=item_order, + randomize=False, + ) + cc.AssessmentItem.objects.create( + contentnode=shared_unit_topic, + assessment_id=shared_unit_ids[2], + type=exercises.SINGLE_SELECTION, + question="What is 7+7?", + answers=json.dumps( + [ + {"answer": "14", "correct": True, "order": 1}, + {"answer": "13", "correct": False, "order": 2}, + ] + ), + hints=json.dumps([]), + raw_data="{}", + order=3, + randomize=False, + ) + + def test_native_unreadable_float_entry_left_out_of_both_archives(self): + title = "Native QTI Unreadable Floats Exercise" + self._assert_archive_ids_match_assessment_metadata(title, 1) + node = cc.ContentNode.objects.get(title=title) + self.assertEqual(self._read_qti_manifest_ids(node), ["item_1"]) + for item in node.assessment_items.all(): + self.assertTrue(validate_qti_item(item.raw_data).is_valid) + + def test_native_unreadable_tolerance_entry_left_out_of_qti_package(self): + title = "Native QTI Unreadable Tolerance Exercise" + node = cc.ContentNode.objects.get(title=title) + self.assertTrue( + validate_qti_item(node.assessment_items.get(order=2).raw_data).is_valid + ) + self.assertEqual(self._read_qti_manifest_ids(node), ["item_1"]) + self.assertEqual( + kolibri_models.ContentNode.objects.get(title=title) + .assessmentmetadata.first() + .assessment_item_ids, + ["item_1"], + ) + + def test_skipped_item_sharing_an_identifier_is_not_listed(self): + self._assert_archive_ids_match_assessment_metadata( + "Native QTI Shared Identifier Exercise", 2 + ) + + def test_perseus_with_unreadable_input_publishes_qti_only(self): + qti_bit = 2 ** RENDERABLE_PRESETS_ORDER.index(format_presets.QTI_ZIP) + exercise_bit = 2 ** RENDERABLE_PRESETS_ORDER.index(format_presets.EXERCISE) + for answer in UNREADABLE_PERSEUS_INPUT_ANSWERS: + title = f"Perseus + Input {answer!r} Exercise" + with self.subTest(answer=answer): + node = cc.ContentNode.objects.get(title=title) + self.assertFalse( + node.files.filter(preset_id=format_presets.EXERCISE).exists() + ) + manifest_ids = self._read_qti_manifest_ids(node) + self.assertEqual( + manifest_ids, + [ + hex_to_qti_id(assessment_id) + for assessment_id in node.assessment_items.order_by( + "order" + ).values_list("assessment_id", flat=True) + ], + ) + published_qti_file = kolibri_models.File.objects.get( + contentnode__title=title, preset=format_presets.QTI_ZIP + ) + self.assertEqual( + published_qti_file.included_presets, qti_bit | exercise_bit + ) + + def test_perseus_with_readable_inputs_publishes_perseus_only(self): + node = cc.ContentNode.objects.get(title="Perseus + Readable Inputs Exercise") + self.assertFalse(node.files.filter(preset_id=format_presets.QTI_ZIP).exists()) + self.assertCountEqual( + [answers for _, answers in self._numeric_input_answers(node)], + [values for _, values in READABLE_PERSEUS_INPUT_ANSWERS], + ) + + def test_perseus_with_free_response_publishes_qti_only(self): + node = cc.ContentNode.objects.get(title="Perseus + Free Response 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_unchanged_republish_rebuilds_perseus_archive_when_qti_dropped(self): + title = "Perseus + Readable Inputs Exercise" + node = cc.ContentNode.objects.get(title=title) + hex_ids = list( + node.assessment_items.order_by("order").values_list( + "assessment_id", flat=True + ) + ) + qti_ids = [hex_to_qti_id(assessment_id) for assessment_id in hex_ids] + self._store_archive(node, format_presets.EXERCISE, qti_ids) + self._store_archive(node, format_presets.QTI_ZIP, qti_ids) + + self._republish_unchanged(node) + + 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, set(hex_ids)) + self.assertEqual(exercise_data["all_assessment_items"], hex_ids) + self.assertEqual( + kolibri_models.ContentNode.objects.get(title=title) + .assessmentmetadata.first() + .assessment_item_ids, + hex_ids, + ) + + def test_unchanged_republish_rebuilds_archive_holding_skipped_native_items(self): + title = "Native QTI Unreadable Floats Exercise" + node = cc.ContentNode.objects.get(title=title) + stored_ids = [ + parse_qti_xml(item.raw_data).getroot().get("identifier") + for item in node.assessment_items.order_by("order") + ] + self._store_archive(node, format_presets.QTI_ZIP, stored_ids) + + self._republish_unchanged(node) + + self._assert_archive_ids_match_assessment_metadata(title, 1) + + def test_unchanged_republish_keeps_archives_with_schema_invalid_item(self): + title = "Native QTI Schema Invalid Exercise" + node = cc.ContentNode.objects.get(title=title) + self._assert_archive_ids_match_assessment_metadata(title, 1) + file_ids = set(node.files.values_list("id", flat=True)) + + self._republish_unchanged(node) + + self.assertEqual(set(node.files.values_list("id", flat=True)), file_ids) + + def test_unit_pre_post_test_leaves_out_unparseable_item(self): + unit_topic = cc.ContentNode.objects.get(title="Entity Unit Topic") + legacy_item = unit_topic.assessment_items.get(type=exercises.SINGLE_SELECTION) + published = kolibri_models.ContentNode.objects.get(title="Entity Unit Topic") + pre_post_test = published.options["completion_criteria"]["threshold"][ + "pre_post_test" + ] + for key, ids in pre_post_test.items(): + with self.subTest(key=key): + self.assertEqual(ids, [hex_to_qti_id(legacy_item.assessment_id)]) + + def test_unit_pre_post_test_skipped_item_sharing_an_identifier_is_not_listed( + self, + ): + unit_topic = cc.ContentNode.objects.get(title="Shared Identifier Unit Topic") + legacy_id = hex_to_qti_id( + unit_topic.assessment_items.get( + type=exercises.SINGLE_SELECTION + ).assessment_id + ) + published = kolibri_models.ContentNode.objects.get( + title="Shared Identifier Unit Topic" + ) + pre_post_test = published.options["completion_criteria"]["threshold"][ + "pre_post_test" + ] + self.assertEqual( + self._read_qti_manifest_ids(unit_topic), ["item_text", legacy_id] + ) + for key, expected in ( + ("assessment_item_ids", ["item_text", legacy_id]), + ("version_a_item_ids", ["item_text", legacy_id]), + ("version_b_item_ids", [legacy_id]), + ): + with self.subTest(key=key): + self.assertEqual(pre_post_test[key], expected) + class EmptyChannelTestCase(StudioTestCase): @classmethod diff --git a/contentcuration/contentcuration/tests/test_restore_channel.py b/contentcuration/contentcuration/tests/test_restore_channel.py index 6e2010d170..0029cddfbc 100644 --- a/contentcuration/contentcuration/tests/test_restore_channel.py +++ b/contentcuration/contentcuration/tests/test_restore_channel.py @@ -317,7 +317,7 @@ def _add_perseus_item(self, order, image): image.save() return item - def _publish(self, generator_class): + def _publish(self, generator_class, **options): items = self.source.assessment_items.order_by("order") generator_class( self.source, @@ -332,6 +332,7 @@ def _publish(self, generator_class): self.channel.id, "en-US", user_id=self.user.id, + **options, ).create_exercise_archive() def _restore(self, *presets, package=None, download_url="http://studio"): @@ -464,7 +465,7 @@ def test_dual_published_node_restores_each_question_once(self): self._add_legacy_item(1) self._add_editor_item_with_encoded_id(2) self._publish(QTIExerciseGenerator) - self._publish(PerseusExerciseGenerator) + self._publish(PerseusExerciseGenerator, qti_item_ids=True) items = self._restore(format_presets.QTI_ZIP, format_presets.EXERCISE) @@ -548,7 +549,7 @@ def test_unusable_package_restores_no_items(self): # The dual node's exercise archive holds QTI ids, which cannot republish. self._add_editor_item(0) self._publish(QTIExerciseGenerator) - self._publish(PerseusExerciseGenerator) + self._publish(PerseusExerciseGenerator, qti_item_ids=True) empty_package = BytesIO() with zipfile.ZipFile(empty_package, "w") as zf: @@ -635,8 +636,7 @@ def _published_presets(self, node): os.remove(create_content_database(self.channel, True, self.user.id, False)) return set(node.files.values_list("preset_id", flat=True)) - def test_restored_legacy_only_node_also_publishes_derived_exercise_archive(self): - # Legacy items restore as QTI, which publish can also derive Perseus from. + def test_restored_legacy_only_node_publishes_both_archives(self): self._add_legacy_item(0) self._add_item( exercises.MULTIPLE_SELECTION, @@ -649,11 +649,9 @@ def test_restored_legacy_only_node_also_publishes_derived_exercise_archive(self) ] ), ) - self.assertEqual(self._published_presets(self.source), {format_presets.QTI_ZIP}) + both_archives = {format_presets.QTI_ZIP, format_presets.EXERCISE} + self.assertEqual(self._published_presets(self.source), both_archives) self._restore(format_presets.QTI_ZIP) - self.assertEqual( - self._published_presets(self.restored), - {format_presets.QTI_ZIP, format_presets.EXERCISE}, - ) + self.assertEqual(self._published_presets(self.restored), both_archives) diff --git a/contentcuration/contentcuration/tests/utils/qti/test_perseus_derive.py b/contentcuration/contentcuration/tests/utils/qti/test_perseus_derive.py index 8449a4a44a..deb971616e 100644 --- a/contentcuration/contentcuration/tests/utils/qti/test_perseus_derive.py +++ b/contentcuration/contentcuration/tests/utils/qti/test_perseus_derive.py @@ -6,6 +6,7 @@ from contentcuration.tests.utils.qti.test_validation import _item_xml from contentcuration.tests.utils.qti.test_validation import ENTITY_CHOICE_ITEM from contentcuration.tests.utils.qti.test_validation import HINTED_EDITOR_ITEM +from contentcuration.utils.assessment.qti.archive import qti_package_identifier from contentcuration.utils.assessment.qti.perseus_derive import derive_perseus_item from contentcuration.utils.assessment.qti.perseus_derive import ( is_answerless_numeric_entry, @@ -20,6 +21,7 @@ def __init__(self, raw_data, randomize=False, assessment_id="a" * 32): self.raw_data = raw_data self.randomize = randomize self.assessment_id = assessment_id + self.type = exercises.QTI def _choice_item( @@ -200,23 +202,53 @@ def test_choice_padded_correct_value_matches_identifier(): assert [a["correct"] for a in answers] == [True, False] -def test_overflowing_correct_value_is_dropped(): - raw_data = _text_item("single", ["42", "1e400"]) +@pytest.mark.parametrize( + "correct_values", + [ + pytest.param(["42", "1e400"], id="overflowing_value"), + pytest.param(["42", "1/2"], id="fraction_value"), + pytest.param(["42", "False"], id="non_number_value"), + ], +) +def test_unreadable_correct_value_is_left_out_of_package(correct_values): + raw_data = _text_item("single", correct_values) assert is_perseus_derivable(raw_data) is True - answers = json.loads(derive_perseus_item(_Item(raw_data)).answers) - assert [a["answer"] for a in answers] == ["42"] + assert qti_package_identifier(_Item(raw_data)) is None + + +@pytest.mark.parametrize( + "raw_data", + [ + pytest.param(_text_item("single", ["42", "+3", "1E3"]), id="numbers"), + pytest.param(_text_item("single", ["cat"], base_type="string"), id="string"), + pytest.param( + _text_item("single", ["42"], map_entries=[("1/2", "1.0")]), + id="unreadable_map_key", + ), + ], +) +def test_readable_declarations_ship_in_package(raw_data): + assert qti_package_identifier(_Item(raw_data)) is not None -def test_answerless_float_entry_is_skipped_but_node_stays_derivable(caplog): +def test_answerless_float_entry_is_left_out_but_node_stays_derivable(): raw_data = _text_item("single", []) - assert derive_perseus_item(_Item(raw_data)) is None - assert "no correct answer" in caplog.text + assert qti_package_identifier(_Item(raw_data)) is None + assert is_perseus_derivable(raw_data) is True + + +def test_schema_invalid_item_is_left_out_but_node_stays_derivable(): + raw_data = _text_item("single", ["4"]).replace( + 'expected-length="50"', 'expected-length="abc"' + ) + assert qti_package_identifier(_Item(raw_data)) is None assert is_perseus_derivable(raw_data) is True def test_math_prompt_survives_into_question(): prompt = ( - "

Solve x" + '

Solve ' + "x" 'x^2' "

" ) @@ -357,8 +389,7 @@ def test_response_identifier_mismatch_degrades_to_qti_only(): 'rptemplates/match_correct.xml" />' ) -TOLERANCE_ITEM = _text_item("single", []).replace( - MATCH_CORRECT_PROCESSING, +TOLERANCE_PROCESSING = ( "" '' '' @@ -366,7 +397,11 @@ def test_response_identifier_mismatch_degrades_to_qti_only(): '' '1' "" - "", + "" +) + +TOLERANCE_ITEM = _text_item("single", []).replace( + MATCH_CORRECT_PROCESSING, TOLERANCE_PROCESSING ) diff --git a/contentcuration/contentcuration/tests/utils/test_exercise_creation.py b/contentcuration/contentcuration/tests/utils/test_exercise_creation.py index 071bb1545e..12cfe2fdce 100644 --- a/contentcuration/contentcuration/tests/utils/test_exercise_creation.py +++ b/contentcuration/contentcuration/tests/utils/test_exercise_creation.py @@ -943,7 +943,7 @@ def test_bad_checksum_image_input_answer_is_answerless_outside_master(self): answerless_input_ids(self.exercise_node), {image_only.assessment_id} ) - def test_unit_keeps_answerless_input_question(self): + def test_unit_leaves_out_answerless_input_question(self): self.exercise_node.kind_id = content_kinds.TOPIC self.exercise_node.save() perseus_item, _ = self._create_perseus_item() @@ -955,10 +955,9 @@ def test_unit_keeps_answerless_input_question(self): 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.assertNotIn(f"{answerless.assessment_id}.json", zip_file.namelist()) self.assertEqual( - exercise_data["all_assessment_items"], - [perseus_item.assessment_id, answerless.assessment_id], + exercise_data["all_assessment_items"], [perseus_item.assessment_id] ) def test_numeric_input_answers_are_read(self): diff --git a/contentcuration/contentcuration/utils/assessment/perseus.py b/contentcuration/contentcuration/utils/assessment/perseus.py index 21a108c165..239a50123f 100644 --- a/contentcuration/contentcuration/utils/assessment/perseus.py +++ b/contentcuration/contentcuration/utils/assessment/perseus.py @@ -6,7 +6,6 @@ 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 @@ -14,6 +13,7 @@ from contentcuration.utils.assessment.base import ExerciseArchiveGenerator from contentcuration.utils.assessment.base import image_filenames from contentcuration.utils.assessment.base import packageable_answers +from contentcuration.utils.assessment.qti.archive import qti_package_identifier 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 @@ -41,8 +41,6 @@ def is_perseus_readable_input_item(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) @@ -70,8 +68,9 @@ class PerseusExerciseGenerator(ExerciseArchiveGenerator): "true_false": "perseus/multiple_selection.json", } - def __init__(self, *args, **kwargs): + def __init__(self, *args, qti_item_ids=False, **kwargs): super().__init__(*args, **kwargs) + self.qti_item_ids = qti_item_ids # Derived proxies for native QTI items, keyed by source assessment_id. # Computed once and reused by both exercise.json and item processing. self._derived_cache = None @@ -82,7 +81,9 @@ def _derived_items(self): not be derived (already logged) and so is skipped from the archive.""" if self._derived_cache is None: self._derived_cache = { - item.assessment_id: derive_perseus_item(item) + item.assessment_id: ( + derive_perseus_item(item) if qti_package_identifier(item) else None + ) for item in self.ccnode.assessment_items.all() if item.type == exercises.QTI } @@ -90,9 +91,6 @@ def _derived_items(self): @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 item in self.ccnode.assessment_items.filter( assessment_id__in=answerless_ids @@ -121,12 +119,11 @@ def process_assessment_item(self, assessment_item): if assessment_item.type == exercises.QTI: derived = self._derived_items().get(assessment_item.assessment_id) if derived is None: - # derive_perseus_item already logs why it could not derive. return return super().process_assessment_item(derived) if assessment_item.type == exercises.PERSEUS_QUESTION: self._write_raw_perseus_assets(assessment_item, self.get_image_file_path()) - if self._derived_items(): + if self.qti_item_ids: # Dual-published: AssessmentMetaData records the QTI manifest id. assessment_item = copy.copy(assessment_item) assessment_item.assessment_id = hex_to_qti_id(assessment_item.assessment_id) @@ -158,14 +155,6 @@ def get_image_ref_prefix(self): return f"${exercises.IMG_PLACEHOLDER}/images" def _exercise_data_for_archive(self): - """``exercise.json`` must list the same item ids and types as the item - JSON files written into this archive. When the node has native QTI - items, every item is written by its QTI manifest id, and native items - carry their derived proxy's legacy Perseus ``type``. On a mismatch, - ``restore_channel``'s ``extract_assessment_items`` opens ``{hex}.json`` - and raises ``FileNotFoundError``, and ``generate_assessment_item`` - receives a ``qti`` type it cannot map. - """ assessment_ids = [ assessment_id for assessment_id in self.exercise_data.get("all_assessment_items", []) @@ -185,7 +174,9 @@ def _exercise_data_for_archive(self): new_ids.append(proxy.assessment_id) new_mapping[proxy.assessment_id] = proxy.type else: - item_id = hex_to_qti_id(assessment_id) if derived else assessment_id + item_id = ( + hex_to_qti_id(assessment_id) if self.qti_item_ids else assessment_id + ) new_ids.append(item_id) if assessment_id in original_mapping: new_mapping[item_id] = original_mapping[assessment_id] diff --git a/contentcuration/contentcuration/utils/assessment/qti/archive.py b/contentcuration/contentcuration/utils/assessment/qti/archive.py index dfb2ef5ae6..816ef30788 100644 --- a/contentcuration/contentcuration/utils/assessment/qti/archive.py +++ b/contentcuration/contentcuration/utils/assessment/qti/archive.py @@ -2,6 +2,7 @@ import logging import os from dataclasses import dataclass +from functools import lru_cache from typing import Any from typing import Dict from typing import List @@ -33,6 +34,9 @@ from contentcuration.utils.assessment.qti.media import rewrite_qti_sized_image_paths from contentcuration.utils.assessment.qti.media import set_qti_item_language from contentcuration.utils.assessment.qti.media import strip_studio_attributes +from contentcuration.utils.assessment.qti.perseus_derive import ( + declares_unreadable_number, +) from contentcuration.utils.assessment.qti.perseus_derive import ( is_answerless_numeric_entry, ) @@ -40,6 +44,9 @@ from contentcuration.utils.assessment.qti.validation import validate_qti_item +_ANSWERLESS_REASON = "has no correct answer" + + @dataclass class QTIResource: identifier: str @@ -47,6 +54,41 @@ class QTIResource: file_dependencies: List[str] +def _native_qti_item_identifier(raw_data): + return parse_qti_xml(raw_data).getroot().get("identifier") + + +@lru_cache(maxsize=256) +def _native_qti_item_exclusion(raw_data) -> Optional[Tuple[int, str]]: + """ + :returns: the log level and reason a native QTI item is left out of the + package, or None when it ships. + """ + result = validate_qti_item(raw_data) + if not result.is_valid: + error_messages = "; ".join( + f"line {e.line}, column {e.column}: {e.message}" for e in result.errors + ) + return logging.ERROR, f"failed schema validation ({error_messages})" + if declares_unreadable_number(raw_data): + return logging.WARNING, "has a correct response that is not a number" + if is_answerless_numeric_entry(raw_data): + return logging.WARNING, _ANSWERLESS_REASON + return None + + +def qti_package_identifier(assessment_item) -> Optional[str]: + """ + :returns: the item's identifier in the QTI package, or None when the + package leaves it out. + """ + if assessment_item.type != exercises.QTI: + return hex_to_qti_id(assessment_item.assessment_id) + if _native_qti_item_exclusion(assessment_item.raw_data): + return None + return _native_qti_item_identifier(assessment_item.raw_data) + + class QTIExerciseGenerator(ExerciseArchiveGenerator): """ Exercise zip generator for QTI format exercises. @@ -94,10 +136,11 @@ def _add_resource(self, resource: QTIResource) -> None: ) self.qti_resources.append(resource) - def _warn_answerless(self, assessment_item): - logging.warning( + def _log_exclusion(self, assessment_item, level, reason): + logging.log( + level, f"QTI item {assessment_item.assessment_id} on node {self.ccnode.pk} " - f"has no correct answer and will be excluded from the package" + f"{reason} and will be excluded from the package", ) def _skip_answerless_input(self, assessment_item, answers) -> bool: @@ -105,27 +148,16 @@ def _skip_answerless_input(self, assessment_item, answers) -> bool: # answerless float entry. is_answerless = is_answerless_input(assessment_item.type, answers) if is_answerless: - self._warn_answerless(assessment_item) + self._log_exclusion(assessment_item, logging.WARNING, _ANSWERLESS_REASON) return is_answerless def _create_native_qti_item(self, assessment_item) -> Optional[Tuple[str, bytes]]: - result = validate_qti_item(assessment_item.raw_data) - if not result.is_valid: - error_messages = "; ".join( - f"line {e.line}, column {e.column}: {e.message}" for e in result.errors - ) - logging.error( - f"QTI item {assessment_item.assessment_id} on node {self.ccnode.pk} " - f"failed schema validation and will be excluded from the package: " - f"{error_messages}" - ) - return None - - if is_answerless_numeric_entry(assessment_item.raw_data): - self._warn_answerless(assessment_item) + exclusion = _native_qti_item_exclusion(assessment_item.raw_data) + if exclusion: + self._log_exclusion(assessment_item, *exclusion) return None - identifier = parse_qti_xml(assessment_item.raw_data).getroot().get("identifier") + identifier = _native_qti_item_identifier(assessment_item.raw_data) if not identifier: raise ValueError( f"QTI item {assessment_item.assessment_id} is missing a root identifier attribute" diff --git a/contentcuration/contentcuration/utils/assessment/qti/perseus_derive.py b/contentcuration/contentcuration/utils/assessment/qti/perseus_derive.py index 74bd3818bd..94e74859cd 100644 --- a/contentcuration/contentcuration/utils/assessment/qti/perseus_derive.py +++ b/contentcuration/contentcuration/utils/assessment/qti/perseus_derive.py @@ -243,11 +243,24 @@ def is_answerless_numeric_entry(raw_data: str) -> bool: return derivable is not None and _is_answerless(derivable) +def declares_unreadable_number(raw_data: str) -> bool: + """Kolibri's QTI viewer throws on a numeric correct response ``Number()`` + rejects or reads as non-finite.""" + root, _ = _parse(raw_data) + return root is not None and any( + value and not is_finite_double(value) + for declaration in root.iter() + if _local(declaration) == "qti-response-declaration" + and declaration.get("base-type") in _NUMERIC_BASE_TYPES + for value in _correct_values(declaration) + ) + + def derive_perseus_item(assessment_item) -> Optional[DerivedAssessmentItem]: """Django ``AssessmentItem`` → ``DerivedAssessmentItem`` proxy, or ``None``. Returns ``None`` (with a warning) when the item is unparseable or its - interaction is not Perseus-expressible, or it is an answerless numeric entry. + interaction is not Perseus-expressible. The proxy's ``assessment_id`` is the QTI item's root ``identifier`` — the same id the QTI archive records for the item in the manifest, and hence in @@ -273,14 +286,6 @@ def derive_perseus_item(assessment_item) -> Optional[DerivedAssessmentItem]: ) return None - # The QTI archive leaves this item out too, so both archives list the same ids. - if _is_answerless(derivable): - logger.warning( - "QTI item %s has no correct answer; skipping derivation", - assessment_item.assessment_id, - ) - return None - identifier = root.get("identifier") if not identifier: logger.warning( diff --git a/contentcuration/contentcuration/utils/publish.py b/contentcuration/contentcuration/utils/publish.py index 67d83f7626..04aedc0835 100644 --- a/contentcuration/contentcuration/utils/publish.py +++ b/contentcuration/contentcuration/utils/publish.py @@ -49,6 +49,7 @@ 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 qti_package_identifier from contentcuration.utils.assessment.qti.archive import QTIExerciseGenerator from contentcuration.utils.assessment.qti.imsmanifest import ( get_assessment_ids_from_manifest, @@ -255,30 +256,12 @@ def assign_license_to_contentcuration_nodes(channel, license): ] -# Legacy structured-field item types Perseus can render, sourced from the -# generator's own template map so the two never drift. ``perseus_question`` is -# excluded: it takes the dedicated Perseus branch, never the derivation gate. -PERSEUS_EXPRESSIBLE_LEGACY_TYPES = frozenset(PerseusExerciseGenerator.TEMPLATE_MAP) - { - exercises.PERSEUS_QUESTION -} - - -def _node_is_perseus_derivable(node): - """True iff the node has >=1 native QTI item and every item is Perseus-expressible.""" - has_native_qti = False - for item in node.assessment_items.all(): - if item.type == exercises.QTI: - has_native_qti = True - if not is_perseus_derivable(item.raw_data): - 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 +def _has_perseus_equivalent(item): + if item.type == exercises.QTI: + return is_perseus_derivable(item.raw_data) + if item.type == exercises.INPUT_QUESTION: + return is_perseus_readable_input_item(item) + return item.type in PerseusExerciseGenerator.TEMPLATE_MAP def has_assessments(node): @@ -393,64 +376,67 @@ def recurse_nodes(self, node, inherited_fields): # noqa C901 metadata, ) - # A mixed node's QTI package embeds its raw Perseus questions as - # custom interactions, so that file also needs the Perseus/exercise - # renderer. Discovered here alongside generator selection and carried - # to create_associated_file_objects, so the included_presets bit is - # grounded in the same decision that drives the embedding. qti_embeds_perseus = False if has_assessments(node): exercise_data = process_assessment_metadata(node) - mapping_values = exercise_data["assessment_mapping"].values() - any_perseus_question = any( - t == exercises.PERSEUS_QUESTION for t in mapping_values - ) - any_native_qti = any(t == exercises.QTI for t in mapping_values) - qti_embeds_perseus = any_perseus_question and any_native_qti - if qti_embeds_perseus: - # Mixed node: one QTI package, raw Perseus embedded as - # custom interactions. - generator_classes = [QTIExerciseGenerator] - elif any_perseus_question: - generator_classes = [PerseusExerciseGenerator] - else: - generator_classes = [QTIExerciseGenerator] - # Also emit a Perseus archive when every item is a native QTI - # interaction Perseus can express, so older Kolibri renders it. - if _node_is_perseus_derivable(node): - generator_classes.append(PerseusExerciseGenerator) + items = node.assessment_items.all() + item_types = {item.type for item in items} + any_perseus_question = exercises.PERSEUS_QUESTION in item_types + any_native_qti = exercises.QTI in item_types + perseus = all(_has_perseus_equivalent(item) for item in items) + qti = any_native_qti or not (perseus and any_perseus_question) + qti_embeds_perseus = qti and any_perseus_question + generator_options = {} + if qti: + generator_options[QTIExerciseGenerator] = {} + if perseus: + generator_options[PerseusExerciseGenerator] = {"qti_item_ids": qti} # If this exercise previously had files generated by generators no # longer in use, make sure we clean them up here. - target_presets = {g.preset for g in generator_classes} + target_presets = {g.preset for g in generator_options} stale_presets = { PerseusExerciseGenerator.preset, QTIExerciseGenerator.preset, } - target_presets - node.files.filter(preset_id__in=stale_presets).delete() - - for generator_class in generator_classes: - if ( - self.force_exercises - or node.changed - or not node.files.filter( - preset_id=generator_class.preset - ).exists() - ): + stale_count, _ = node.files.filter(preset_id__in=stale_presets).delete() + + shipped_ids = _shipped_item_ids(node, qti_item_ids=qti) + rebuild = ( + self.force_exercises + or node.changed + or stale_count + or set( + node.files.filter(preset_id__in=target_presets).values_list( + "preset_id", flat=True + ) + ) + != target_presets + ) + manifest_ids = None if rebuild else _stored_manifest_ids(node) + if rebuild or not set(shipped_ids.values()).issuperset( + manifest_ids or () + ): + for generator_class, options in generator_options.items(): generator = generator_class( node, exercise_data, self.channel_id, self.default_language.lang_code, user_id=self.user_id, + **options, ) generator.create_exercise_archive() + manifest_ids = _stored_manifest_ids(node) + package_ids = _published_package_ids(shipped_ids, manifest_ids) # Only create assessment metadata for exercises, not UNIT topics # UNIT topics store their assessment config in options/completion_criteria if node.kind_id == content_kinds.EXERCISE: - create_kolibri_assessment_metadata(node, kolibrinode) + create_kolibri_assessment_metadata(node, kolibrinode, package_ids) + else: + map_unit_pre_post_test_ids(kolibrinode, package_ids) elif node.kind_id == content_kinds.SLIDESHOW: create_slideshow_manifest(node, user_id=self.user_id) @@ -843,23 +829,11 @@ def process_assessment_metadata(ccnode): return exercise_data -def create_kolibri_assessment_metadata(ccnode, kolibrinode): - assessment_items = ccnode.assessment_items.all().order_by("order") +def create_kolibri_assessment_metadata(ccnode, kolibrinode, package_ids): randomize, _, mastery_model = _get_exercise_data_from_ccnode( - ccnode, len(assessment_items) + ccnode, ccnode.assessment_items.count() ) - 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 - ] + assessment_item_ids = list(package_ids.values()) kolibrimodels.AssessmentMetaData.objects.create( id=uuid.uuid4(), @@ -872,6 +846,58 @@ def create_kolibri_assessment_metadata(ccnode, kolibrinode): ) +def _stored_manifest_ids(ccnode): + qti_file = ccnode.files.filter(preset_id=format_presets.QTI_ZIP).first() + if not qti_file: + return None + with qti_file.file_on_disk.open("rb") as file_handle: + return get_assessment_ids_from_manifest(file_handle) + + +def _shipped_item_ids(ccnode, qti_item_ids): + answerless_ids = answerless_input_ids(ccnode) + return { + item.assessment_id: ( + qti_package_identifier(item) if qti_item_ids else item.assessment_id + ) + for item in ccnode.assessment_items.all().order_by("order") + if item.assessment_id not in answerless_ids + } + + +def _published_package_ids(shipped_ids, manifest_ids): + if manifest_ids is None: + return shipped_ids + unmatched_manifest_ids = set(manifest_ids) + package_ids = {} + for assessment_id, package_id in shipped_ids.items(): + if package_id in unmatched_manifest_ids: + unmatched_manifest_ids.remove(package_id) + package_ids[assessment_id] = package_id + return package_ids + + +def map_unit_pre_post_test_ids(kolibrinode, package_ids): + options = deepcopy(kolibrinode.options) + pre_post_test = options["completion_criteria"]["threshold"].get("pre_post_test") + if not pre_post_test: + return + for key in ("assessment_item_ids", "version_a_item_ids", "version_b_item_ids"): + pre_post_test[key] = [ + package_ids[assessment_id] + for assessment_id in pre_post_test[key] + if assessment_id in package_ids + ] + if "assessment_objectives" in options: + options["assessment_objectives"] = { + package_ids[assessment_id]: objectives + for assessment_id, objectives in options["assessment_objectives"].items() + if assessment_id in package_ids + } + kolibrinode.options = options + kolibrinode.save(update_fields=["options"]) + + def map_prerequisites(root_node): for n in ccmodels.PrerequisiteContentRelationship.objects.filter(