Merge pull request #3354 from rtibbles/dotpath_deletion

Tests and updates for addition/deletion of metadata labels

Author: rtibbles

contentcuration/contentcuration/tests/viewsets/test_contentnode.py

@@ -15,6 +15,7 @@
 from django_concurrent_tests.helpers import make_concurrent_calls
 from le_utils.constants import content_kinds
 from le_utils.constants import roles
+from le_utils.constants.labels.accessibility_categories import ACCESSIBILITYCATEGORIESLIST
 
 from contentcuration import models
 from contentcuration.tests import testdata
@@ -615,6 +616,64 @@ def test_update_contentnode_extra_fields(self):
             models.ContentNode.objects.get(id=contentnode.id).extra_fields["randomize"], randomize
         )
 
+    def test_update_contentnode_remove_from_extra_fields(self):
+        user = testdata.user()
+        metadata = self.contentnode_db_metadata
+        metadata["extra_fields"] = {
+            "m": 5,
+        }
+        contentnode = models.ContentNode.objects.create(**metadata)
+        self.client.force_authenticate(user=user)
+        # Remove extra_fields.m
+        response = self.client.post(
+            self.sync_url,
+            [generate_update_event(contentnode.id, CONTENTNODE, {"extra_fields.m": None})],
+            format="json",
+        )
+        self.assertEqual(response.status_code, 200, response.content)
+        with self.assertRaises(KeyError):
+            models.ContentNode.objects.get(id=contentnode.id).extra_fields["m"]
+
+    def test_update_contentnode_add_multiple_metadata_labels(self):
+        user = testdata.user()
+
+        contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata)
+        self.client.force_authenticate(user=user)
+        # Add metadata label to accessibility_labels
+        response = self.client.post(
+            self.sync_url,
+            [generate_update_event(contentnode.id, CONTENTNODE, {"accessibility_labels.{}".format(ACCESSIBILITYCATEGORIESLIST[0]): True})],
+            format="json",
+        )
+        self.assertEqual(response.status_code, 200, response.content)
+        self.assertTrue(models.ContentNode.objects.get(id=contentnode.id).accessibility_labels[ACCESSIBILITYCATEGORIESLIST[0]])
+
+        response = self.client.post(
+            self.sync_url,
+            [generate_update_event(contentnode.id, CONTENTNODE, {"accessibility_labels.{}".format(ACCESSIBILITYCATEGORIESLIST[1]): True})],
+            format="json",
+        )
+        self.assertEqual(response.status_code, 200, response.content)
+        self.assertTrue(models.ContentNode.objects.get(id=contentnode.id).accessibility_labels[ACCESSIBILITYCATEGORIESLIST[0]])
+        self.assertTrue(models.ContentNode.objects.get(id=contentnode.id).accessibility_labels[ACCESSIBILITYCATEGORIESLIST[1]])
+
+    def test_update_contentnode_remove_metadata_label(self):
+        user = testdata.user()
+        metadata = self.contentnode_db_metadata
+        metadata["accessibility_labels"] = {ACCESSIBILITYCATEGORIESLIST[0]: True}
+
+        contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata)
+        self.client.force_authenticate(user=user)
+        # Add metadata label to accessibility_labels
+        response = self.client.post(
+            self.sync_url,
+            [generate_update_event(contentnode.id, CONTENTNODE, {"accessibility_labels.{}".format(ACCESSIBILITYCATEGORIESLIST[0]): None})],
+            format="json",
+        )
+        self.assertEqual(response.status_code, 200, response.content)
+        with self.assertRaises(KeyError):
+            models.ContentNode.objects.get(id=contentnode.id).accessibility_labels[ACCESSIBILITYCATEGORIESLIST[0]]
+
     def test_update_contentnode_tags(self):
         user = testdata.user()
         contentnode = models.ContentNode.objects.create(**self.contentnode_db_metadata)

contentcuration/contentcuration/viewsets/common.py

@@ -125,6 +125,8 @@ def get_value(self, dictionary):
         # get just field name
         value = dictionary.get(self.field_name, {})
 
+        self.initial_value = value
+
         if value is None:
             return empty
 
@@ -151,7 +153,31 @@ def create(self, validated_data):
         return instance
 
     def update(self, instance, validated_data):
+        instance = instance or self.default_value()
         instance.update(validated_data)
+        # This should have been set when get_value was invoked
+        # But could be `None`, so we check if it is truthy here.
+        if getattr(self, "initial_value", None):
+            # Iterate through each field
+            for key in self.initial_value:
+                # If the field is explicitly being set as None, then
+                # we need to delete it from the instance.
+                if self.initial_value[key] is None:
+                    # Follow the dot path to find the nested object
+                    obj = instance
+                    # Iterate through each part of the dot path
+                    # up until, but not including the final key
+                    for part in key.split(".")[:-1]:
+                        if isinstance(obj, dict):
+                            # If it's a dict use get to get the next level object
+                            obj = obj.get(part)
+                        elif isinstance(obj, list):
+                            # If it's a list, use the index to get the next level object
+                            obj = obj[int(part)]
+                        else:
+                            raise ValidationError("Tried to access a dot path in an invalid type")
+                    # Finally, delete the final key
+                    obj.pop(key.split(".")[-1])
         return instance
 
 

contentcuration/contentcuration/viewsets/contentnode.py

@@ -280,7 +280,7 @@ def __init__(self, choices, *args, **kwargs):
     def get_fields(self):
         fields = {}
         for label_id, label_name in self.choices:
-            field = BooleanField(required=False, label=label_name)
+            field = BooleanField(required=False, label=label_name, allow_null=True)
             fields[label_id] = field
         return fields
 
@@ -306,6 +306,16 @@ class ContentNodeSerializer(BulkModelSerializer):
     categories = MetadataLabelsField(subjects.choices, required=False)
     learner_needs = MetadataLabelsField(needs.choices, required=False)
 
+    dict_fields = [
+        "extra_fields",
+        "grade_levels",
+        "resource_types",
+        "learning_activities",
+        "accessibility_labels",
+        "categories",
+        "learner_needs",
+    ]
+
     class Meta:
         model = ContentNode
         fields = (
@@ -368,11 +378,12 @@ def update(self, instance, validated_data):
                 {"parent": "This field should only be changed by a move operation"}
             )
 
-        extra_fields = validated_data.pop("extra_fields", None)
-        if extra_fields is not None:
-            validated_data["extra_fields"] = self.fields["extra_fields"].update(
-                instance.extra_fields, extra_fields
-            )
+        for field in self.dict_fields:
+            field_data = validated_data.pop(field, None)
+            if field_data is not None:
+                validated_data[field] = self.fields[field].update(
+                    getattr(instance, field), field_data
+                )
         if "tags" in validated_data:
             tags = validated_data.pop("tags")
             set_tags({instance.id: tags})