From 77056dcf0ba6375b32e543580a09e3cfa70f44c7 Mon Sep 17 00:00:00 2001 From: Alex de Landgraaf Date: Thu, 23 Mar 2023 10:56:22 +0100 Subject: [PATCH] :bug: [#324] Fixed retrieval of overlapping objects by filtering with exclusive end date. --- src/objects/api/v1/openapi.yaml | 8 +-- src/objects/api/v2/openapi.yaml | 8 +-- .../migrations/0028_auto_20230331_1239.py | 30 ++++++++ src/objects/core/models.py | 6 +- src/objects/core/query.py | 4 +- src/objects/tests/v2/test_filters.py | 47 ++++++++++++ src/objects/tests/v2/test_object_api.py | 72 +++++++++++++++++++ 7 files changed, 163 insertions(+), 12 deletions(-) create mode 100644 src/objects/core/migrations/0028_auto_20230331_1239.py diff --git a/src/objects/api/v1/openapi.yaml b/src/objects/api/v1/openapi.yaml index 4892a747..880fffd2 100644 --- a/src/objects/api/v1/openapi.yaml +++ b/src/objects/api/v1/openapi.yaml @@ -654,12 +654,12 @@ components: startAt: type: string format: date - description: Legal start date of the object record + description: Legal start date of the object record (inclusive) endAt: type: string format: date readOnly: true - description: Legal end date of the object record + description: Legal end date of the object record (exclusive) registrationAt: type: string format: date @@ -801,12 +801,12 @@ components: startAt: type: string format: date - description: Legal start date of the object record + description: Legal start date of the object record (inclusive) endAt: type: string format: date readOnly: true - description: Legal end date of the object record + description: Legal end date of the object record (exclusive) registrationAt: type: string format: date diff --git a/src/objects/api/v2/openapi.yaml b/src/objects/api/v2/openapi.yaml index 0bd6bf1d..6540a71b 100644 --- a/src/objects/api/v2/openapi.yaml +++ b/src/objects/api/v2/openapi.yaml @@ -742,12 +742,12 @@ components: startAt: type: string format: date - description: Legal start date of the object record + description: Legal start date of the object record (inclusive) endAt: type: string format: date readOnly: true - description: Legal end date of the object record + description: Legal end date of the object record (exclusive) registrationAt: type: string format: date @@ -894,12 +894,12 @@ components: startAt: type: string format: date - description: Legal start date of the object record + description: Legal start date of the object record (inclusive) endAt: type: string format: date readOnly: true - description: Legal end date of the object record + description: Legal end date of the object record (exclusive) registrationAt: type: string format: date diff --git a/src/objects/core/migrations/0028_auto_20230331_1239.py b/src/objects/core/migrations/0028_auto_20230331_1239.py new file mode 100644 index 00000000..5d47b4b4 --- /dev/null +++ b/src/objects/core/migrations/0028_auto_20230331_1239.py @@ -0,0 +1,30 @@ +# Generated by Django 2.2.28 on 2023-03-31 10:39 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("core", "0027_auto_20211203_1209"), + ] + + operations = [ + migrations.AlterField( + model_name="objectrecord", + name="end_at", + field=models.DateField( + help_text="Legal end date of the object record (exclusive)", + null=True, + verbose_name="end at", + ), + ), + migrations.AlterField( + model_name="objectrecord", + name="start_at", + field=models.DateField( + help_text="Legal start date of the object record (inclusive)", + verbose_name="start at", + ), + ), + ] diff --git a/src/objects/core/models.py b/src/objects/core/models.py index 10894698..122bb71c 100644 --- a/src/objects/core/models.py +++ b/src/objects/core/models.py @@ -98,10 +98,12 @@ class ObjectRecord(models.Model): encoder=DjangoJSONEncoder, ) start_at = models.DateField( - _("start at"), help_text=_("Legal start date of the object record") + _("start at"), help_text=_("Legal start date of the object record (inclusive)") ) end_at = models.DateField( - _("end at"), null=True, help_text=_("Legal end date of the object record") + _("end at"), + null=True, + help_text=_("Legal end date of the object record (exclusive)"), ) registration_at = models.DateField( _("registration at"), diff --git a/src/objects/core/query.py b/src/objects/core/query.py index 84514c6e..bc9a5346 100644 --- a/src/objects/core/query.py +++ b/src/objects/core/query.py @@ -16,7 +16,7 @@ def filter_for_date(self, date): return ( self.filter(records__start_at__lte=date) .filter( - models.Q(records__end_at__gte=date) + models.Q(records__end_at__gt=date) | models.Q(records__end_at__isnull=True) ) .distinct() @@ -59,7 +59,7 @@ def filter_for_date(self, date): """ return self.filter(start_at__lte=date).filter( - models.Q(end_at__gte=date) | models.Q(end_at__isnull=True) + models.Q(end_at__gt=date) | models.Q(end_at__isnull=True) ) def filter_for_registration_date(self, date): diff --git a/src/objects/tests/v2/test_filters.py b/src/objects/tests/v2/test_filters.py index 52ed5ddc..e8f3d9c2 100644 --- a/src/objects/tests/v2/test_filters.py +++ b/src/objects/tests/v2/test_filters.py @@ -377,6 +377,53 @@ def test_filter_exclude_old_records(self): data = response.json()["results"] self.assertEqual(len(data), 0) + response = self.client.get(self.url, {"data_attrs": "diameter__exact__50"}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + data = response.json()["results"] + self.assertEqual(len(data), 1) + + def test_filter_exclude_old_records_issue_324(self): + """ + Filter and conflict resolution. + + If record A ends at X and a newer record B starts at X, both match on + date X. Normally, conflict resolution returns only record B. However, + when you add a filter, it first finds the record matching this filter. + If the filter only results in record A, there are no conflicts and + record A is returned. + """ + record_old = ObjectRecordFactory.create( + data={"adres": {"straat": "Bospad"}}, + object__object_type=self.object_type, + start_at=date.today() - timedelta(days=1), + end_at=date.today(), + ) + record_new = ObjectRecordFactory.create( + data={"adres": {"straat": "Dorpsstraat"}}, + object=record_old.object, + start_at=record_old.end_at, + ) + + response = self.client.get( + self.url, {"data_attrs": "adres__straat__exact__Bospad"} + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + data = response.json()["results"] + self.assertEqual(len(data), 0) + + response = self.client.get( + self.url, {"data_attrs": "adres__straat__exact__Dorpsstraat"} + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + data = response.json()["results"] + self.assertEqual(len(data), 1) + def test_filter_date_field_gte(self): record = ObjectRecordFactory.create( data={"dateField": "2000-10-10"}, object__object_type=self.object_type diff --git a/src/objects/tests/v2/test_object_api.py b/src/objects/tests/v2/test_object_api.py index 12b5a43a..cec92bb9 100644 --- a/src/objects/tests/v2/test_object_api.py +++ b/src/objects/tests/v2/test_object_api.py @@ -380,3 +380,75 @@ def test_updating_object_after_changing_the_startAt_value_returns_200(self, m): response_updating_data_after_startAt_modification.status_code, status.HTTP_200_OK, ) + + +@requests_mock.Mocker() +class ObjectApiTodayTests(TokenAuthMixin, APITestCase): + maxDiff = None + + @classmethod + def setUpTestData(cls): + super().setUpTestData() + + cls.object_type = ObjectTypeFactory(service__api_root=OBJECT_TYPES_API) + PermissionFactory.create( + object_type=cls.object_type, + mode=PermissionModes.read_and_write, + token_auth=cls.token_auth, + ) + + def test_update_object_issue_324(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), + ) + m.get(self.object_type.url, json=mock_objecttype(self.object_type.url)) + today = date.today() + + data = { + "type": self.object_type.url, + "record": { + "typeVersion": 1, + "data": {"adres": {"straat": "Bospad"}, "diameter": 30}, + "startAt": today, + }, + } + url = reverse("object-list") + response = self.client.post(url, data, **GEO_WRITE_KWARGS) + + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + object = Object.objects.get() + + url = reverse("object-detail", args=[object.uuid]) + data = { + "type": object.object_type.url, + "record": { + "typeVersion": 1, + "data": {"adres": {"straat": "Dorpsstraat"}, "diameter": 30}, + "startAt": today, + }, + } + + response = self.client.put(url, data, **GEO_WRITE_KWARGS) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + object.refresh_from_db() + self.assertEqual(object.object_type, self.object_type) + self.assertEqual(object.records.count(), 2) + url = reverse("object-list") + response = self.client.get(url, {"data_attrs": "adres__straat__exact__Bospad"}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + data = response.json()["results"] + self.assertEqual(len(data), 0) + + response = self.client.get( + url, {"data_attrs": "adres__straat__exact__Dorpsstraat"} + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + data = response.json()["results"] + self.assertEqual(len(data), 1)