Merge pull request #4627 from rtibbles/do_you_actually_complete_me

Ensure that complete is not incorrectly set via the API

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
 

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()

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",
         }
 

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

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):