From f17438677eae2ecd958a3c81810374b32e5e3335 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 7 Jan 2026 14:52:49 -0800 Subject: [PATCH 1/5] Upgrade le-utils to version 0.2.14 --- requirements.in | 2 +- requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements.in b/requirements.in index 27ca67281c..c86a1d37c3 100644 --- a/requirements.in +++ b/requirements.in @@ -5,7 +5,7 @@ djangorestframework==3.15.1 psycopg2-binary==2.9.10 django-js-reverse==0.10.2 django-registration==3.4 -le-utils>=0.2.12 +le-utils==0.2.14 gunicorn==23.0.0 django-postmark==0.1.6 jsonfield==3.1.0 diff --git a/requirements.txt b/requirements.txt index 9863c77097..2e227661d1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -162,7 +162,7 @@ language-data==1.3.0 # via langcodes latex2mathml==3.78.0 # via -r requirements.in -le-utils==0.2.12 +le-utils==0.2.14 # via -r requirements.in marisa-trie==1.2.1 # via language-data From 5890cf8d549d0a53407f3f46287981b0aed09cab Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 7 Jan 2026 14:54:00 -0800 Subject: [PATCH 2/5] Add test for and update publish logic to publish pre/post tests for UNIT modality topics. --- .../constants/completion_criteria.py | 1 + .../tests/test_exportchannel.py | 106 ++++++++++++++++++ .../contentcuration/utils/publish.py | 40 +++++-- 3 files changed, 139 insertions(+), 8 deletions(-) diff --git a/contentcuration/contentcuration/constants/completion_criteria.py b/contentcuration/contentcuration/constants/completion_criteria.py index 1a8c101e38..2aabe53262 100644 --- a/contentcuration/contentcuration/constants/completion_criteria.py +++ b/contentcuration/contentcuration/constants/completion_criteria.py @@ -52,6 +52,7 @@ def _build_validator(): completion_criteria.APPROX_TIME, completion_criteria.REFERENCE, }, + content_kinds.TOPIC: {completion_criteria.MASTERY}, } diff --git a/contentcuration/contentcuration/tests/test_exportchannel.py b/contentcuration/contentcuration/tests/test_exportchannel.py index 5c850597d7..b87b310344 100644 --- a/contentcuration/contentcuration/tests/test_exportchannel.py +++ b/contentcuration/contentcuration/tests/test_exportchannel.py @@ -16,6 +16,7 @@ from kolibri_content.router import set_active_content_database from le_utils.constants import exercises from le_utils.constants import format_presets +from le_utils.constants import modalities from le_utils.constants.labels import accessibility_categories from le_utils.constants.labels import learning_activities from le_utils.constants.labels import levels @@ -304,6 +305,83 @@ def setUp(self): } first_topic_first_child.save() + # Add a UNIT topic with directly attached assessment items + unit_assessment_id_1 = uuid.uuid4().hex + unit_assessment_id_2 = uuid.uuid4().hex + + unit_topic = create_node( + {"kind_id": "topic", "title": "Test Unit Topic", "children": []}, + parent=self.content_channel.main_tree, + ) + 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": [ + unit_assessment_id_1, + unit_assessment_id_2, + ], + "version_a_item_ids": [unit_assessment_id_1], + "version_b_item_ids": [unit_assessment_id_2], + }, + }, + }, + } + } + unit_topic.save() + + cc.AssessmentItem.objects.create( + contentnode=unit_topic, + assessment_id=unit_assessment_id_1, + type=exercises.SINGLE_SELECTION, + question="What is 2+2?", + answers=json.dumps( + [ + {"answer": "4", "correct": True, "order": 1}, + {"answer": "3", "correct": False, "order": 2}, + ] + ), + hints=json.dumps([]), + raw_data="{}", + order=1, + randomize=False, + ) + + cc.AssessmentItem.objects.create( + contentnode=unit_topic, + assessment_id=unit_assessment_id_2, + type=exercises.SINGLE_SELECTION, + question="What is 3+3?", + answers=json.dumps( + [ + {"answer": "6", "correct": True, "order": 1}, + {"answer": "5", "correct": False, "order": 2}, + ] + ), + hints=json.dumps([]), + raw_data="{}", + order=2, + randomize=False, + ) + + # Add a LESSON child topic under the UNIT with a video child + lesson_topic = create_node( + { + "kind_id": "topic", + "title": "Test Lesson Topic", + "children": [ + {"kind_id": "video", "title": "Unit Lesson Video", "children": []}, + ], + }, + parent=unit_topic, + ) + lesson_topic.extra_fields = {"options": {"modality": modalities.LESSON}} + lesson_topic.save() + set_channel_icon_encoding(self.content_channel) self.tempdb = create_content_database( self.content_channel, True, self.admin_user.id, True @@ -348,6 +426,10 @@ def test_contentnode_incomplete_not_published(self): assert incomplete_nodes.count() > 0 for node in complete_nodes: + # Skip nodes that are known to fail validation and not be published: + # - "Bad mastery test" exercise has no mastery model (checked separately below) + if node.title == "Bad mastery test": + continue # if a parent node is incomplete, this node is excluded as well. if node.get_ancestors().filter(complete=False).count() == 0: assert kolibri_nodes.filter(pk=node.node_id).count() == 1 @@ -642,6 +724,30 @@ def test_qti_archive_contains_manifest_and_assessment_ids(self): for i, ai in enumerate(qti_exercise.assessment_items.order_by("order")): self.assertEqual(assessment_ids[i], hex_to_qti_id(ai.assessment_id)) + def test_unit_topic_publishes_with_exercise_zip(self): + """Test that a TOPIC node with UNIT modality gets its directly + attached assessment items compiled into a zip file during publishing.""" + unit_topic = cc.ContentNode.objects.get(title="Test Unit Topic") + + # Assert UNIT topic has exercise file in Studio + unit_files = cc.File.objects.filter( + contentnode=unit_topic, + preset_id=format_presets.EXERCISE, + ) + self.assertEqual( + unit_files.count(), + 1, + "UNIT topic should have exactly one exercise archive file", + ) + + # Assert NO assessment metadata in Kolibri export for UNIT topics + # UNIT topics store assessment config in options/completion_criteria instead + published_unit = kolibri_models.ContentNode.objects.get(title="Test Unit Topic") + self.assertFalse( + published_unit.assessmentmetadata.exists(), + "UNIT topic should NOT have assessment metadata", + ) + class EmptyChannelTestCase(StudioTestCase): @classmethod diff --git a/contentcuration/contentcuration/utils/publish.py b/contentcuration/contentcuration/utils/publish.py index 3e28f2d0e0..9e9e190105 100644 --- a/contentcuration/contentcuration/utils/publish.py +++ b/contentcuration/contentcuration/utils/publish.py @@ -35,6 +35,7 @@ from le_utils.constants import exercises from le_utils.constants import file_formats from le_utils.constants import format_presets +from le_utils.constants import modalities from le_utils.constants import roles from search.models import ChannelFullTextSearch from search.models import ContentNodeFullTextSearch @@ -227,6 +228,22 @@ def assign_license_to_contentcuration_nodes(channel, license): ] +def has_assessments(node): + """Check if a node should have its assessment items published. + + Returns True for EXERCISE nodes and TOPIC nodes with UNIT modality + that have assessment items. + """ + if node.kind_id == content_kinds.EXERCISE: + return True + if node.kind_id == content_kinds.TOPIC: + options = node.extra_fields.get("options", {}) if node.extra_fields else {} + if options.get("modality") == modalities.UNIT: + # Only return True if the UNIT has assessment items + return node.assessment_items.filter(deleted=False).exists() + return False + + class TreeMapper: def __init__( self, @@ -296,9 +313,10 @@ def recurse_nodes(self, node, inherited_fields): # noqa C901 # Only process nodes that are either non-topics or have non-topic descendants if node.is_publishable(): - # early validation to make sure we don't have any exercises without mastery models - # which should be unlikely when the node is complete, but just in case - if node.kind_id == content_kinds.EXERCISE: + # early validation to make sure we don't have any nodes with assessments + # without mastery models, which should be unlikely when the node is complete, + # but just in case + if has_assessments(node): try: # migrates and extracts the mastery model from the exercise _, mastery_model = parse_assessment_metadata(node) @@ -306,8 +324,8 @@ def recurse_nodes(self, node, inherited_fields): # noqa C901 raise ValueError("Exercise does not have a mastery model") except Exception as e: logging.warning( - "Unable to parse exercise {id} mastery model: {error}".format( - id=node.pk, error=str(e) + "Unable to parse exercise {id} {title} mastery model: {error}".format( + id=node.pk, title=node.title, error=str(e) ) ) return @@ -322,7 +340,7 @@ def recurse_nodes(self, node, inherited_fields): # noqa C901 metadata, ) - if node.kind_id == content_kinds.EXERCISE: + if has_assessments(node): exercise_data = process_assessment_metadata(node) any_free_response = any( t == exercises.FREE_RESPONSE @@ -359,10 +377,16 @@ def recurse_nodes(self, node, inherited_fields): # noqa C901 ) generator.create_exercise_archive() - create_kolibri_assessment_metadata(node, kolibrinode) + # 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) elif node.kind_id == content_kinds.SLIDESHOW: create_slideshow_manifest(node, user_id=self.user_id) - elif node.kind_id == content_kinds.TOPIC: + + # TOPIC nodes need to recurse into children, including UNIT topics + # that also had their assessments processed above + if node.kind_id == content_kinds.TOPIC: for child in node.children.all(): self.recurse_nodes(child, metadata) create_associated_file_objects(kolibrinode, node) From 0ad254f526f34d7d0d9a645fc433374e5e55984b Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Mon, 19 Jan 2026 16:47:04 -0800 Subject: [PATCH 3/5] Make topic completion criterion validation more rigorous. --- .../constants/completion_criteria.py | 34 ++++++++- .../tests/test_completion_criteria.py | 73 +++++++++++++++++++ .../contentcuration/views/internal.py | 4 +- 3 files changed, 107 insertions(+), 4 deletions(-) diff --git a/contentcuration/contentcuration/constants/completion_criteria.py b/contentcuration/contentcuration/constants/completion_criteria.py index 2aabe53262..49d351bd7d 100644 --- a/contentcuration/contentcuration/constants/completion_criteria.py +++ b/contentcuration/contentcuration/constants/completion_criteria.py @@ -3,7 +3,9 @@ from jsonschema.validators import validator_for from le_utils.constants import completion_criteria from le_utils.constants import content_kinds +from le_utils.constants import exercises from le_utils.constants import mastery_criteria +from le_utils.constants import modalities def _build_validator(): @@ -56,7 +58,7 @@ def _build_validator(): } -def check_model_for_kind(data, kind): +def check_model_for_kind(data, kind, modality=None): model = data.get("model") if kind is None or model is None or kind not in ALLOWED_MODELS_PER_KIND: return @@ -69,11 +71,37 @@ def check_model_for_kind(data, kind): ) ) + if kind == content_kinds.TOPIC: + check_topic_completion_criteria(data, modality) -def validate(data, kind=None): + +def check_topic_completion_criteria(data, modality): + """ + Validates topic-specific completion criteria rules: + - Topics can only have completion criteria if modality is UNIT + - Topics can only use PRE_POST_TEST mastery model + """ + # Topics can only have completion criteria with UNIT modality + if modality != modalities.UNIT: + raise ValidationError( + "Topics can only have completion criteria with UNIT modality" + ) + + # Topics can only use PRE_POST_TEST mastery model + threshold = data.get("threshold", {}) + mastery_model = threshold.get("mastery_model") + if mastery_model is not None and mastery_model != exercises.PRE_POST_TEST: + raise ValidationError( + "mastery_model '{}' is invalid for topic content kind; " + "only '{}' is allowed".format(mastery_model, exercises.PRE_POST_TEST) + ) + + +def validate(data, kind=None, modality=None): """ :param data: Dictionary of data to validate :param kind: A str of the node content kind + :param modality: A str of the node modality (required for topics with completion criteria) :raises: ValidationError: When invalid """ # empty dicts are okay @@ -105,4 +133,4 @@ def validate(data, kind=None): e.error_list.extend(error_descriptions) raise e - check_model_for_kind(data, kind) + check_model_for_kind(data, kind, modality) diff --git a/contentcuration/contentcuration/tests/test_completion_criteria.py b/contentcuration/contentcuration/tests/test_completion_criteria.py index a0daec10d7..09cb1529fc 100644 --- a/contentcuration/contentcuration/tests/test_completion_criteria.py +++ b/contentcuration/contentcuration/tests/test_completion_criteria.py @@ -2,7 +2,9 @@ from django.test import SimpleTestCase from le_utils.constants import completion_criteria from le_utils.constants import content_kinds +from le_utils.constants import exercises from le_utils.constants import mastery_criteria +from le_utils.constants import modalities from contentcuration.constants.completion_criteria import validate @@ -40,3 +42,74 @@ def test_validate__content_kind(self): }, kind=content_kinds.DOCUMENT, ) + + def _make_preposttest_threshold(self): + """Helper to create a valid pre_post_test threshold structure.""" + # UUIDs must be 32 hex characters + uuid_a = "a" * 32 + uuid_b = "b" * 32 + return { + "mastery_model": exercises.PRE_POST_TEST, + "pre_post_test": { + "assessment_item_ids": [uuid_a, uuid_b], + "version_a_item_ids": [uuid_a], + "version_b_item_ids": [uuid_b], + }, + } + + def test_validate__topic_with_unit_modality_and_preposttest__success(self): + """Topic with UNIT modality and PRE_POST_TEST mastery model should pass validation.""" + validate( + { + "model": completion_criteria.MASTERY, + "threshold": self._make_preposttest_threshold(), + }, + kind=content_kinds.TOPIC, + modality=modalities.UNIT, + ) + + def test_validate__topic_with_unit_modality_and_wrong_mastery_model__fail(self): + """Topic with UNIT modality but non-PRE_POST_TEST mastery model should fail.""" + with self.assertRaisesRegex( + ValidationError, "mastery_model.*invalid for.*topic" + ): + validate( + { + "model": completion_criteria.MASTERY, + "threshold": { + "mastery_model": mastery_criteria.M_OF_N, + "m": 3, + "n": 5, + }, + }, + kind=content_kinds.TOPIC, + modality=modalities.UNIT, + ) + + def test_validate__topic_with_non_unit_modality_and_completion_criteria__fail(self): + """Topic with non-UNIT modality (e.g., LESSON) should not have completion criteria.""" + with self.assertRaisesRegex( + ValidationError, "only.*completion criteria.*UNIT modality" + ): + validate( + { + "model": completion_criteria.MASTERY, + "threshold": self._make_preposttest_threshold(), + }, + kind=content_kinds.TOPIC, + modality=modalities.LESSON, + ) + + def test_validate__topic_with_no_modality_and_completion_criteria__fail(self): + """Topic with no modality should not have completion criteria.""" + with self.assertRaisesRegex( + ValidationError, "only.*completion criteria.*UNIT modality" + ): + validate( + { + "model": completion_criteria.MASTERY, + "threshold": self._make_preposttest_threshold(), + }, + kind=content_kinds.TOPIC, + modality=None, + ) diff --git a/contentcuration/contentcuration/views/internal.py b/contentcuration/contentcuration/views/internal.py index 93be3e3043..07b4014b00 100644 --- a/contentcuration/contentcuration/views/internal.py +++ b/contentcuration/contentcuration/views/internal.py @@ -839,7 +839,9 @@ def create_node(node_data, parent_node, sort_order): # noqa: C901 if "options" in extra_fields and "completion_criteria" in extra_fields["options"]: try: completion_criteria.validate( - extra_fields["options"]["completion_criteria"], kind=node_data["kind"] + extra_fields["options"]["completion_criteria"], + kind=node_data["kind"], + modality=extra_fields["options"].get("modality"), ) except completion_criteria.ValidationError: raise NodeValidationError( From 2d7d22c923c6de213f73cedd5d4c6b8e972428cd Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Mon, 19 Jan 2026 16:47:50 -0800 Subject: [PATCH 4/5] Run completion criterion checking on all node types. --- contentcuration/contentcuration/models.py | 25 ++---- .../tests/test_contentnodes.py | 89 +++++++++++++++++++ 2 files changed, 98 insertions(+), 16 deletions(-) diff --git a/contentcuration/contentcuration/models.py b/contentcuration/contentcuration/models.py index c2d94744e0..524da39cfc 100644 --- a/contentcuration/contentcuration/models.py +++ b/contentcuration/contentcuration/models.py @@ -2290,22 +2290,15 @@ def mark_complete(self): # noqa C901 ) if not (self.extra_fields.get("mastery_model") or criterion): errors.append("Missing mastery criterion") - if criterion: - try: - completion_criteria.validate( - criterion, kind=content_kinds.EXERCISE - ) - except completion_criteria.ValidationError: - errors.append("Mastery criterion is defined but is invalid") - else: - criterion = self.extra_fields and self.extra_fields.get( - "options", {} - ).get("completion_criteria", {}) - if criterion: - try: - completion_criteria.validate(criterion, kind=self.kind_id) - except completion_criteria.ValidationError: - errors.append("Completion criterion is defined but is invalid") + options = self.extra_fields and self.extra_fields.get("options", {}) or {} + criterion = options.get("completion_criteria", {}) + if criterion: + try: + completion_criteria.validate( + criterion, kind=self.kind_id, modality=options.get("modality") + ) + except completion_criteria.ValidationError: + errors.append("Completion criterion is defined but is invalid") self.complete = not errors return errors diff --git a/contentcuration/contentcuration/tests/test_contentnodes.py b/contentcuration/contentcuration/tests/test_contentnodes.py index bc4f73b0b9..4db7ffcbc4 100644 --- a/contentcuration/contentcuration/tests/test_contentnodes.py +++ b/contentcuration/contentcuration/tests/test_contentnodes.py @@ -10,6 +10,7 @@ from le_utils.constants import content_kinds from le_utils.constants import exercises from le_utils.constants import format_presets +from le_utils.constants import modalities from mixer.backend.django import mixer from mock import patch @@ -1481,3 +1482,91 @@ def test_create_video_null_extra_fields(self): new_obj.mark_complete() except AttributeError: self.fail("Null extra_fields not handled") + + def _make_preposttest_extra_fields(self, modality): + """Helper to create extra_fields with valid pre_post_test completion criteria.""" + uuid_a = "a" * 32 + uuid_b = "b" * 32 + return { + "options": { + "modality": modality, + "completion_criteria": { + "model": completion_criteria.MASTERY, + "threshold": { + "mastery_model": exercises.PRE_POST_TEST, + "pre_post_test": { + "assessment_item_ids": [uuid_a, uuid_b], + "version_a_item_ids": [uuid_a], + "version_b_item_ids": [uuid_b], + }, + }, + }, + } + } + + def test_create_topic_unit_modality_valid_preposttest_complete(self): + """Topic with UNIT modality and valid PRE_POST_TEST completion criteria should be complete.""" + channel = testdata.channel() + new_obj = ContentNode( + title="Unit Topic", + kind_id=content_kinds.TOPIC, + parent=channel.main_tree, + extra_fields=self._make_preposttest_extra_fields(modalities.UNIT), + ) + new_obj.save() + new_obj.mark_complete() + self.assertTrue(new_obj.complete) + + def test_create_topic_unit_modality_wrong_mastery_model_incomplete(self): + """Topic with UNIT modality but M_OF_N mastery model should be incomplete.""" + channel = testdata.channel() + new_obj = ContentNode( + title="Unit Topic", + kind_id=content_kinds.TOPIC, + parent=channel.main_tree, + extra_fields={ + "options": { + "modality": modalities.UNIT, + "completion_criteria": { + "model": completion_criteria.MASTERY, + "threshold": { + "mastery_model": exercises.M_OF_N, + "m": 3, + "n": 5, + }, + }, + } + }, + ) + new_obj.save() + new_obj.mark_complete() + self.assertFalse(new_obj.complete) + + def test_create_topic_lesson_modality_with_completion_criteria_incomplete(self): + """Topic with LESSON modality should not have completion criteria.""" + channel = testdata.channel() + new_obj = ContentNode( + title="Lesson Topic", + kind_id=content_kinds.TOPIC, + parent=channel.main_tree, + extra_fields=self._make_preposttest_extra_fields(modalities.LESSON), + ) + new_obj.save() + new_obj.mark_complete() + self.assertFalse(new_obj.complete) + + def test_create_topic_no_modality_with_completion_criteria_incomplete(self): + """Topic with no modality should not have completion criteria.""" + channel = testdata.channel() + extra_fields = self._make_preposttest_extra_fields(modalities.UNIT) + # Remove the modality + del extra_fields["options"]["modality"] + new_obj = ContentNode( + title="Topic Without Modality", + kind_id=content_kinds.TOPIC, + parent=channel.main_tree, + extra_fields=extra_fields, + ) + new_obj.save() + new_obj.mark_complete() + self.assertFalse(new_obj.complete) From 2d234d4ae242fab90b651bc7e1b674b8abfee562 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Tue, 20 Jan 2026 11:11:03 -0800 Subject: [PATCH 5/5] Add strong requirement for UNIT folders to require preposttest completion criteria. --- contentcuration/contentcuration/models.py | 11 ++++++++++- .../contentcuration/tests/test_contentnodes.py | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/models.py b/contentcuration/contentcuration/models.py index 524da39cfc..a3f15770cd 100644 --- a/contentcuration/contentcuration/models.py +++ b/contentcuration/contentcuration/models.py @@ -54,6 +54,7 @@ from le_utils.constants import file_formats from le_utils.constants import format_presets from le_utils.constants import languages +from le_utils.constants import modalities from le_utils.constants import roles from model_utils import FieldTracker from mptt.models import MPTTModel @@ -2292,10 +2293,18 @@ def mark_complete(self): # noqa C901 errors.append("Missing mastery criterion") options = self.extra_fields and self.extra_fields.get("options", {}) or {} criterion = options.get("completion_criteria", {}) + modality = options.get("modality") + # UNIT modality topics must have completion criteria + if ( + self.kind_id == content_kinds.TOPIC + and modality == modalities.UNIT + and not criterion + ): + errors.append("UNIT modality topics must have completion criteria") if criterion: try: completion_criteria.validate( - criterion, kind=self.kind_id, modality=options.get("modality") + criterion, kind=self.kind_id, modality=modality ) except completion_criteria.ValidationError: errors.append("Completion criterion is defined but is invalid") diff --git a/contentcuration/contentcuration/tests/test_contentnodes.py b/contentcuration/contentcuration/tests/test_contentnodes.py index 4db7ffcbc4..5ce8b472a4 100644 --- a/contentcuration/contentcuration/tests/test_contentnodes.py +++ b/contentcuration/contentcuration/tests/test_contentnodes.py @@ -1570,3 +1570,21 @@ def test_create_topic_no_modality_with_completion_criteria_incomplete(self): new_obj.save() new_obj.mark_complete() self.assertFalse(new_obj.complete) + + def test_create_topic_unit_modality_without_completion_criteria_incomplete(self): + """Topic with UNIT modality MUST have completion criteria - it's not optional.""" + channel = testdata.channel() + new_obj = ContentNode( + title="Unit Topic Without Criteria", + kind_id=content_kinds.TOPIC, + parent=channel.main_tree, + extra_fields={ + "options": { + "modality": modalities.UNIT, + # No completion_criteria + } + }, + ) + new_obj.save() + new_obj.mark_complete() + self.assertFalse(new_obj.complete)