From 5924ad5121744210099b63c73c99b678b69b8da7 Mon Sep 17 00:00:00 2001 From: Sidney Richards Date: Wed, 9 Oct 2024 15:43:33 +0200 Subject: [PATCH 1/2] Add test to illustrate expected validation on object PATCH --- src/objects/tests/v2/test_object_api.py | 36 +++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/objects/tests/v2/test_object_api.py b/src/objects/tests/v2/test_object_api.py index c80c6696..1c2cd2f4 100644 --- a/src/objects/tests/v2/test_object_api.py +++ b/src/objects/tests/v2/test_object_api.py @@ -333,6 +333,42 @@ def test_patch_object_record(self, m): self.assertEqual(initial_record.corrected, current_record) self.assertEqual(initial_record.end_at, date(2020, 1, 1)) + def test_patch_validates_merged_object_rather_than_partial_object(self, m): + mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") + m.get( + f"{self.object_type.url}/versions/1", + json=mock_objecttype_version(self.object_type.url), + ) + + initial_record = ObjectRecordFactory.create( + version=1, + object__object_type=self.object_type, + start_at=date.today(), + data={"name": "Name", "diameter": 20}, + ) + + url = reverse("object-detail", args=[initial_record.object.uuid]) + data = { + "record": { + "data": { + # Note the required fields are missing, and that should be fine: + # the _merged_ object should be valid according to the schema, not + # the partial. + "plantDate": "2024-10-09" + }, + }, + } + + response = self.client.patch(url, data, **GEO_WRITE_KWARGS) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + initial_record.refresh_from_db() + self.assertEqual( + initial_record.data, + {"plantDate": "2024-10-09", "diameter": 20, "name": "Name"}, + ) + + def test_delete_object(self, m): record = ObjectRecordFactory.create(object__object_type=self.object_type) object = record.object From ab75c7d4ace0fc4736c4e66ee4b7e9575e691d77 Mon Sep 17 00:00:00 2001 From: Anna Shamray Date: Tue, 29 Oct 2024 10:47:18 +0100 Subject: [PATCH 2/2] :bug: [#466] fix validation for merge PATCH --- src/objects/api/serializers.py | 4 ++++ src/objects/api/validators.py | 5 ++++- src/objects/tests/v2/test_object_api.py | 9 ++++++--- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/objects/api/serializers.py b/src/objects/api/serializers.py index f6221636..750d1dfe 100644 --- a/src/objects/api/serializers.py +++ b/src/objects/api/serializers.py @@ -131,6 +131,10 @@ def update(self, instance, validated_data): # version should be set if "version" not in validated_data: validated_data["version"] = instance.version + # start_at should be set + if "start_at" not in validated_data: + validated_data["start_at"] = instance.start_at + if self.partial and "data" in validated_data: # Apply JSON Merge Patch for record data validated_data["data"] = merge_patch(instance.data, validated_data["data"]) diff --git a/src/objects/api/validators.py b/src/objects/api/validators.py index 50c2cd2c..11b86567 100644 --- a/src/objects/api/validators.py +++ b/src/objects/api/validators.py @@ -9,7 +9,7 @@ from objects.core.utils import check_objecttype from .constants import Operators -from .utils import string_to_value +from .utils import merge_patch, string_to_value class JsonSchemaValidator: @@ -34,6 +34,9 @@ def __call__(self, attrs, serializer): ) version = attrs.get("version") if "version" in attrs else instance.version data = attrs.get("data", {}) if "data" in attrs else instance.data + if serializer.partial and "data" in attrs: + # Apply JSON Merge Patch for record data + data = merge_patch(instance.data, attrs["data"]) if not object_type or not version: return diff --git a/src/objects/tests/v2/test_object_api.py b/src/objects/tests/v2/test_object_api.py index 1c2cd2f4..ba572946 100644 --- a/src/objects/tests/v2/test_object_api.py +++ b/src/objects/tests/v2/test_object_api.py @@ -361,13 +361,16 @@ def test_patch_validates_merged_object_rather_than_partial_object(self, m): response = self.client.patch(url, data, **GEO_WRITE_KWARGS) self.assertEqual(response.status_code, status.HTTP_200_OK) - - initial_record.refresh_from_db() self.assertEqual( - initial_record.data, + response.json()["record"]["data"], {"plantDate": "2024-10-09", "diameter": 20, "name": "Name"}, ) + last_record = initial_record.object.last_record + self.assertEqual( + last_record.data, + {"plantDate": "2024-10-09", "diameter": 20, "name": "Name"}, + ) def test_delete_object(self, m): record = ObjectRecordFactory.create(object__object_type=self.object_type)