diff --git a/contentcuration/contentcuration/models.py b/contentcuration/contentcuration/models.py index b9e7864619..f1a334fefe 100644 --- a/contentcuration/contentcuration/models.py +++ b/contentcuration/contentcuration/models.py @@ -1881,6 +1881,13 @@ def mark_complete(self): # noqa C901 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.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") self.complete = not errors return errors diff --git a/contentcuration/contentcuration/tests/test_contentnodes.py b/contentcuration/contentcuration/tests/test_contentnodes.py index 5dfad3d48d..6c24498bfb 100644 --- a/contentcuration/contentcuration/tests/test_contentnodes.py +++ b/contentcuration/contentcuration/tests/test_contentnodes.py @@ -1040,6 +1040,33 @@ def test_create_video_thumbnail_only(self): new_obj.mark_complete() self.assertFalse(new_obj.complete) + def test_create_video_invalid_completion_criterion(self): + licenses = list(License.objects.filter(copyright_holder_required=False, is_custom=False).values_list("pk", flat=True)) + channel = testdata.channel() + new_obj = ContentNode( + title="yes", + kind_id=content_kinds.VIDEO, + parent=channel.main_tree, + license_id=licenses[0], + copyright_holder="Some person", + extra_fields={ + "randomize": False, + "options": { + "completion_criteria": { + "threshold": { + "mastery_model": exercises.M_OF_N, + "n": 5, + }, + "model": completion_criteria.MASTERY, + } + } + }, + ) + new_obj.save() + File.objects.create(contentnode=new_obj, preset_id=format_presets.VIDEO_HIGH_RES, checksum=uuid.uuid4().hex) + new_obj.mark_complete() + self.assertFalse(new_obj.complete) + def test_create_exercise_no_assessment_items(self): licenses = list(License.objects.filter(copyright_holder_required=False, is_custom=False).values_list("pk", flat=True)) channel = testdata.channel() diff --git a/contentcuration/contentcuration/tests/views/test_views_internal.py b/contentcuration/contentcuration/tests/views/test_views_internal.py index 30680b0cfb..7689bace3a 100644 --- a/contentcuration/contentcuration/tests/views/test_views_internal.py +++ b/contentcuration/contentcuration/tests/views/test_views_internal.py @@ -871,7 +871,7 @@ def _make_node_data(self): "license_description": "This is a fake license", "copyright_holder": random_data.copyright_holder, "questions": [], - "extra_fields": "{}", + "extra_fields": {}, "role": "learner", } diff --git a/contentcuration/contentcuration/tests/viewsets/test_contentnode.py b/contentcuration/contentcuration/tests/viewsets/test_contentnode.py index 11d4dd6147..98178b1601 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_contentnode.py +++ b/contentcuration/contentcuration/tests/viewsets/test_contentnode.py @@ -33,6 +33,7 @@ from contentcuration.viewsets.contentnode import ContentNodeFilter from contentcuration.viewsets.sync.constants import CONTENTNODE from contentcuration.viewsets.sync.constants import CONTENTNODE_PREREQUISITE +from contentcuration.viewsets.sync.constants import UPDATED nested_subjects = [subject for subject in SUBJECTSLIST if "." in subject] @@ -661,6 +662,24 @@ def test_update_contentnode_exercise_mastery_model_old(self): models.ContentNode.objects.get(id=contentnode.id).extra_fields["options"]["completion_criteria"]["model"], completion_criteria.MASTERY ) + def test_update_contentnode_exercise_incomplete_mastery_model_marked_complete(self): + metadata = self.contentnode_db_metadata + metadata["kind_id"] = content_kinds.EXERCISE + contentnode = models.ContentNode.objects.create(**metadata) + + response = self.sync_changes( + [generate_update_event(contentnode.id, CONTENTNODE, { + "complete": True, + }, channel_id=self.channel.id)], + ) + + self.assertEqual(response.status_code, 200, response.content) + self.assertFalse( + models.ContentNode.objects.get(id=contentnode.id).complete + ) + change = models.Change.objects.filter(channel=self.channel, change_type=UPDATED, table=CONTENTNODE).last() + self.assertFalse(change.kwargs["mods"]["complete"]) + def test_update_contentnode_extra_fields(self): contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata) # Update extra_fields.randomize diff --git a/contentcuration/contentcuration/viewsets/contentnode.py b/contentcuration/contentcuration/viewsets/contentnode.py index 229d4f81ad..591eccdcd9 100644 --- a/contentcuration/contentcuration/viewsets/contentnode.py +++ b/contentcuration/contentcuration/viewsets/contentnode.py @@ -16,9 +16,9 @@ from django.http import Http404 from django.utils.timezone import now from django_cte import CTEQuerySet +from django_filters.rest_framework import BooleanFilter from django_filters.rest_framework import CharFilter from django_filters.rest_framework import UUIDFilter -from django_filters.rest_framework import BooleanFilter from le_utils.constants import completion_criteria from le_utils.constants import content_kinds from le_utils.constants import roles @@ -412,6 +412,24 @@ def _check_completion_criteria(self, kind, complete, validated_data): except DjangoValidationError as e: raise ValidationError(e) + def _ensure_complete(self, instance): + """ + If an instance is marked as complete, ensure that it is actually complete. + If it is not, update the value, save, and issue a change event. + """ + if instance.complete: + instance.mark_complete() + if not instance.complete: + instance.save() + user_id = None + if "request" in self.context: + user_id = self.context["request"].user.id + Change.create_change( + generate_update_event( + instance.id, CONTENTNODE, {"complete": False}, channel_id=instance.get_channel_id() + ), created_by_id=user_id, applied=True + ) + def create(self, validated_data): tags = None if "tags" in validated_data: @@ -424,6 +442,8 @@ def create(self, validated_data): if tags: set_tags({instance.id: tags}) + self._ensure_complete(instance) + return instance def update(self, instance, validated_data): @@ -439,7 +459,10 @@ def update(self, instance, validated_data): self._check_completion_criteria(validated_data.get("kind", instance.kind_id), validated_data.get("complete", instance.complete), validated_data) - return super(ContentNodeSerializer, self).update(instance, validated_data) + instance = super(ContentNodeSerializer, self).update(instance, validated_data) + + self._ensure_complete(instance) + return instance def retrieve_thumbail_src(item):