From 6cb83f0d3c971187bb0765d2f28acd7db0a8f82e Mon Sep 17 00:00:00 2001
From: Richard Tibbles <richard@learningequality.org>
Date: Thu, 12 May 2022 17:04:47 -0700
Subject: [PATCH 1/2] Add test for partial updates to completion criteria. Move
 validation of completion criteria to update method to run validation, on the
 update version.

---
 .../tests/viewsets/test_contentnode.py        | 34 +++++++++++++++++++
 .../contentcuration/viewsets/contentnode.py   |  8 ++---
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/contentcuration/contentcuration/tests/viewsets/test_contentnode.py b/contentcuration/contentcuration/tests/viewsets/test_contentnode.py
index da64292cd9..74f784fdcf 100644
--- a/contentcuration/contentcuration/tests/viewsets/test_contentnode.py
+++ b/contentcuration/contentcuration/tests/viewsets/test_contentnode.py
@@ -708,6 +708,40 @@ def test_update_contentnode_update_options_completion_criteria(self):
         self.assertEqual(c.extra_fields["options"]["completion_criteria"]["model"], completion_criteria.TIME)
         self.assertEqual(c.extra_fields["options"]["completion_criteria"]["threshold"], 10)
 
+    def test_update_contentnode_update_options_completion_criteria_threshold_only(self):
+        user = testdata.user()
+        metadata = self.contentnode_db_metadata
+        metadata["extra_fields"] = {
+            "options": {
+                "completion_criteria": {
+                    "model": completion_criteria.TIME,
+                    "threshold": 5,
+                }
+            },
+        }
+        contentnode = models.ContentNode.objects.create(**metadata)
+        self.client.force_authenticate(user=user)
+        # Change extra_fields.options.completion_criteria.model
+        # and extra_fields.options.completion_criteria.threshold
+        response = self.client.post(
+            self.sync_url,
+            [
+                generate_update_event(
+                    contentnode.id,
+                    CONTENTNODE,
+                    {
+                        "extra_fields.options.completion_criteria.threshold": 10
+                    }
+                )
+            ],
+            format="json",
+        )
+        self.assertEqual(response.status_code, 200, response.content)
+        c = models.ContentNode.objects.get(id=contentnode.id)
+
+        self.assertEqual(c.extra_fields["options"]["completion_criteria"]["model"], completion_criteria.TIME)
+        self.assertEqual(c.extra_fields["options"]["completion_criteria"]["threshold"], 10)
+
     def test_update_contentnode_add_multiple_metadata_labels(self):
         user = testdata.user()
 
diff --git a/contentcuration/contentcuration/viewsets/contentnode.py b/contentcuration/contentcuration/viewsets/contentnode.py
index 61284cd43f..79676716a5 100644
--- a/contentcuration/contentcuration/viewsets/contentnode.py
+++ b/contentcuration/contentcuration/viewsets/contentnode.py
@@ -271,10 +271,10 @@ class CompletionCriteriaSerializer(JSONFieldDictSerializer):
     model = CharField()
     learner_managed = BooleanField(required=False)
 
-    def to_internal_value(self, data):
-        output = super().to_internal_value(data)
-        completion_criteria_validator.validate(output)
-        return output
+    def update(self, instance, validated_data):
+        instance = super(CompletionCriteriaSerializer, self).update(instance, validated_data)
+        completion_criteria_validator.validate(instance)
+        return instance
 
 
 class ExtraFieldsOptionsSerializer(JSONFieldDictSerializer):

From 4634bf76654e4cadfca003d84aca96dfe1f61957 Mon Sep 17 00:00:00 2001
From: Richard Tibbles <richard@learningequality.org>
Date: Thu, 12 May 2022 17:33:30 -0700
Subject: [PATCH 2/2] Update serializer tests to check for validation error
 during update.

---
 .../contentcuration/tests/test_serializers.py       | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/contentcuration/contentcuration/tests/test_serializers.py b/contentcuration/contentcuration/tests/test_serializers.py
index 943121b4de..2b71c291a6 100644
--- a/contentcuration/contentcuration/tests/test_serializers.py
+++ b/contentcuration/contentcuration/tests/test_serializers.py
@@ -1,5 +1,6 @@
 from __future__ import absolute_import
 
+from django.core.exceptions import ValidationError
 from django.db.models.query import QuerySet
 from django.test.testcases import SimpleTestCase
 
@@ -37,11 +38,19 @@ def test_no_completion_criteria(self):
 
     def test_completion_criteria__valid(self):
         self.data.update(completion_criteria={"model": "time", "threshold": 10, "learner_managed": True})
-        self.assertTrue(self.serializer.is_valid())
+        serializer = self.serializer
+        serializer.is_valid()
+        try:
+            serializer.update({}, serializer.validated_data)
+        except ValidationError:
+            self.fail("Completion criteria should be valid")
 
     def test_completion_criteria__invalid(self):
         self.data.update(completion_criteria={"model": "time", "threshold": "test"})
-        self.assertFalse(self.serializer.is_valid())
+        serializer = self.serializer
+        serializer.is_valid()
+        with self.assertRaises(ValidationError):
+            serializer.update({}, serializer.validated_data)
 
 
 class ContentNodeSerializerTestCase(BaseAPITestCase):