Merge pull request #266 from maykinmedia/issue/filter-only-on-actual-…

…values

Issue/filter only on actual values

src/objects/api/v1/filters.py

@@ -1,11 +1,13 @@
+import datetime
+
 from django import forms
 from django.utils.translation import ugettext_lazy as _
 
 from django_filters import filters
 from rest_framework import serializers
 from vng_api_common.filtersets import FilterSet
 
-from objects.core.models import Object, ObjectType
+from objects.core.models import ObjectRecord, ObjectType
 from objects.utils.filters import ObjectTypeFilter
 
 from ..constants import Operators
@@ -30,9 +32,9 @@ def clean(self):
         return cleaned_data
 
 
-class ObjectFilterSet(FilterSet):
+class ObjectRecordFilterSet(FilterSet):
     type = ObjectTypeFilter(
-        field_name="object_type",
+        field_name="object__object_type",
         help_text=_("Url reference to OBJECTTYPE in Objecttypes API"),
         queryset=ObjectType.objects.all(),
         min_length=1,
@@ -77,7 +79,7 @@ class ObjectFilterSet(FilterSet):
     )
 
     class Meta:
-        model = Object
+        model = ObjectRecord
         fields = ("type", "data_attrs", "date", "registrationDate")
         form = ObjectFilterForm
 
@@ -93,19 +95,17 @@ def filter_data_attrs(self, queryset, name, value: str):
                 in_vals = [str_value]
                 if real_value != value:
                     in_vals.append(real_value)
-                queryset = queryset.filter(
-                    **{f"records__data__{variable}__in": in_vals}
-                )
+                queryset = queryset.filter(**{f"data__{variable}__in": in_vals})
             elif operator == "icontains":
                 # icontains treats everything like strings
                 queryset = queryset.filter(
-                    **{f"records__data__{variable}__icontains": str_value}
+                    **{f"data__{variable}__icontains": str_value}
                 )
 
             else:
                 # gt, gte, lt, lte operators
                 queryset = queryset.filter(
-                    **{f"records__data__{variable}__{operator}": real_value}
+                    **{f"data__{variable}__{operator}": real_value}
                 )
 
         return queryset

src/objects/api/v1/views.py

@@ -21,7 +21,7 @@
     ObjectSearchSerializer,
     ObjectSerializer,
 )
-from .filters import ObjectFilterSet
+from .filters import ObjectRecordFilterSet
 
 
 @extend_schema_view(
@@ -53,7 +53,7 @@ class ObjectViewSet(
         "object_type", "object_type__service"
     ).order_by("-pk")
     serializer_class = ObjectSerializer
-    filterset_class = ObjectFilterSet
+    filterset_class = ObjectRecordFilterSet
     lookup_field = "uuid"
     search_input_serializer_class = ObjectSearchSerializer
     permission_classes = [ObjectTypeBasedPermission]
@@ -92,12 +92,23 @@ def get_queryset(self):
         if self.action not in ("list", "search"):
             return base
 
-        # default filtering on current day
-        if not date:
-            base = base.filter_for_date(datetime.date.today())
-
         return base.filter_for_token(self.request.auth)
 
+    def filter_queryset(self, queryset):
+        """ filter records first"""
+        filtered_records = super().filter_queryset(
+            ObjectRecord.objects.select_related("object")
+        )
+
+        date = getattr(self.request, "query_params", {}).get("date", None)
+        registration_date = getattr(self.request, "query_params", {}).get(
+            "registrationDate", None
+        )
+        if self.action in ("list", "search") and not date and not registration_date:
+            filtered_records = filtered_records.filter_for_date(datetime.date.today())
+
+        return queryset.filter(records__in=filtered_records).distinct()
+
     @extend_schema(
         description="Retrieve all RECORDs of an OBJECT.",
         responses={"200": HistoryRecordSerializer(many=True)},

src/objects/api/v2/filters.py

@@ -1,11 +1,13 @@
+import datetime
+
 from django import forms
 from django.utils.translation import ugettext_lazy as _
 
 from django_filters import filters
 from rest_framework import serializers
 from vng_api_common.filtersets import FilterSet
 
-from objects.core.models import Object, ObjectType
+from objects.core.models import ObjectRecord, ObjectType
 from objects.utils.filters import ObjectTypeFilter
 
 from ..constants import Operators
@@ -30,9 +32,9 @@ def clean(self):
         return cleaned_data
 
 
-class ObjectFilterSet(FilterSet):
+class ObjectRecordFilterSet(FilterSet):
     type = ObjectTypeFilter(
-        field_name="object_type",
+        field_name="object__object_type",
         help_text=_("Url reference to OBJECTTYPE in Objecttypes API"),
         queryset=ObjectType.objects.all(),
         min_length=1,
@@ -81,7 +83,7 @@ class ObjectFilterSet(FilterSet):
     )
 
     class Meta:
-        model = Object
+        model = ObjectRecord
         fields = ("type", "data_attrs", "date", "registrationDate")
         form = ObjectFilterForm
 
@@ -97,26 +99,24 @@ def filter_data_attrs(self, queryset, name, value: str):
                 in_vals = [str_value]
                 if real_value != value:
                     in_vals.append(real_value)
-                queryset = queryset.filter(
-                    **{f"records__data__{variable}__in": in_vals}
-                )
+                queryset = queryset.filter(**{f"data__{variable}__in": in_vals})
             elif operator == "icontains":
                 # icontains treats everything like strings
                 queryset = queryset.filter(
-                    **{f"records__data__{variable}__icontains": str_value}
+                    **{f"data__{variable}__icontains": str_value}
                 )
 
             else:
                 # gt, gte, lt, lte operators
                 queryset = queryset.filter(
-                    **{f"records__data__{variable}__{operator}": real_value}
+                    **{f"data__{variable}__{operator}": real_value}
                 )
 
         return queryset
 
     def filter_data_icontains(self, queryset, name, value: str):
         # WHERE clause has jsonpath: where data @? '$.** ? (@ like_regex "$value" flag "i")'
-        where_str = "core_objectrecord.data @? CONCAT('$.** ? (@ like_regex \"',%s::text,'\" flag \"i\")')::jsonpath"
+        where_str = "data @? CONCAT('$.** ? (@ like_regex \"',%s::text,'\" flag \"i\")')::jsonpath"
         return queryset.extra(where=[where_str], params=[value])
 
     def filter_date(self, queryset, name, value: date):

src/objects/api/v2/views.py

@@ -23,7 +23,7 @@
     ObjectSerializer,
     PermissionSerializer,
 )
-from .filters import ObjectFilterSet
+from .filters import ObjectRecordFilterSet
 
 
 @extend_schema_view(
@@ -55,7 +55,7 @@ class ObjectViewSet(
         "object_type", "object_type__service"
     ).order_by("-pk")
     serializer_class = ObjectSerializer
-    filterset_class = ObjectFilterSet
+    filterset_class = ObjectRecordFilterSet
     lookup_field = "uuid"
     search_input_serializer_class = ObjectSearchSerializer
     permission_classes = [ObjectTypeBasedPermission]
@@ -95,12 +95,23 @@ def get_queryset(self):
         if self.action not in ("list", "search"):
             return base
 
-        # default filtering on current day
-        if not date:
-            base = base.filter_for_date(datetime.date.today())
-
         return base.filter_for_token(self.request.auth)
 
+    def filter_queryset(self, queryset):
+        """ filter records first"""
+        filtered_records = super().filter_queryset(
+            ObjectRecord.objects.select_related("object")
+        )
+
+        date = getattr(self.request, "query_params", {}).get("date", None)
+        registration_date = getattr(self.request, "query_params", {}).get(
+            "registrationDate", None
+        )
+        if self.action in ("list", "search") and not date and not registration_date:
+            filtered_records = filtered_records.filter_for_date(datetime.date.today())
+
+        return queryset.filter(records__in=filtered_records).distinct()
+
     @extend_schema(
         description="Retrieve all RECORDs of an OBJECT.",
         responses={"200": HistoryRecordSerializer(many=True)},

src/objects/core/query.py

@@ -41,24 +41,36 @@ def filter_for_date(self, date):
         the given `date`. If there is no `end_at` date, it means the record is
         still actual.
 
-        If there are multiple records returned, the last added record (ie. with
-        the highest `index`) is the most actual record from a material historical
+        Return records with the largest index for the object as the most actual
+        record from a material historical
         perspective.
         """
-        return (
+        filtered_records = (
             self.filter(start_at__lte=date)
             .filter(models.Q(end_at__gte=date) | models.Q(end_at__isnull=True))
-            .order_by("-index")
+            .order_by()
+        )
+        grouped_records = (
+            filtered_records.filter(object=models.OuterRef("object"))
+            .values("object")
+            .annotate(max_index=models.Max("index"))
+            .values("max_index")
         )
+        return self.filter(index=models.Subquery(grouped_records))
 
     def filter_for_registration_date(self, date):
         """
         Return records as seen on `date` and later, from a formal historical
         perspective.
 
-        Typically, the first record in the result set represents the record as
-        it is most actual from a formal historical perspective.
+        Return records with the largest index for the object as the most actual
+        from a formal historical perspective.
         """
-        return self.filter(registration_at__lte=date).order_by(
-            "-registration_at", "-index"
+        filtered_records = self.filter(registration_at__lte=date).order_by()
+        grouped_records = (
+            filtered_records.filter(object=models.OuterRef("object"))
+            .values("object")
+            .annotate(max_index=models.Max("index"))
+            .values("max_index")
         )
+        return self.filter(index=models.Subquery(grouped_records))

src/objects/tests/v1/test_auth.py

@@ -49,6 +49,7 @@ def setUpTestData(cls):
 
     def test_retrieve_no_object_permission(self):
         object = ObjectFactory.create()
+        ObjectRecordFactory.create(object=object)
         url = reverse("object-detail", args=[object.uuid])
 
         response = self.client.get(url)
@@ -62,6 +63,7 @@ def test_retrieve_with_read_only_permission(self):
             token_auth=self.token_auth,
         )
         object = ObjectFactory.create(object_type=self.object_type)
+        ObjectRecordFactory.create(object=object)
         url = reverse("object-detail", args=[object.uuid])
 
         response = self.client.get(url)
@@ -70,6 +72,7 @@ def test_retrieve_with_read_only_permission(self):
 
     def test_history_no_object_permissions(self):
         object = ObjectFactory.create()
+        ObjectRecordFactory.create(object=object)
         url = reverse("object-history", args=[object.uuid])
 
         response = self.client.get(url)
@@ -83,6 +86,7 @@ def test_history_with_read_only_permissions(self):
             token_auth=self.token_auth,
         )
         object = ObjectFactory.create(object_type=self.object_type)
+        ObjectRecordFactory.create(object=object)
         url = reverse("object-history", args=[object.uuid])
 
         response = self.client.get(url)
@@ -98,6 +102,7 @@ def test_history_with_fields_permissions(self):
             fields=["url"],
         )
         object = ObjectFactory.create(object_type=self.object_type)
+        ObjectRecordFactory.create(object=object)
         url = reverse("object-history", args=[object.uuid])
 
         response = self.client.get(url)
@@ -111,6 +116,7 @@ def test_update_with_read_only_perm(self):
             token_auth=self.token_auth,
         )
         object = ObjectFactory.create(object_type=self.object_type)
+        ObjectRecordFactory.create(object=object)
         url = reverse("object-detail", args=[object.uuid])
 
         response = self.client.put(url, **GEO_WRITE_KWARGS)
@@ -124,6 +130,7 @@ def test_patch_with_read_only_perm(self):
             token_auth=self.token_auth,
         )
         object = ObjectFactory.create(object_type=self.object_type)
+        ObjectRecordFactory.create(object=object)
         url = reverse("object-detail", args=[object.uuid])
 
         response = self.client.patch(url, **GEO_WRITE_KWARGS)
@@ -137,6 +144,7 @@ def test_destroy_with_read_only_perm(self):
             token_auth=self.token_auth,
         )
         object = ObjectFactory.create(object_type=self.object_type)
+        ObjectRecordFactory.create(object=object)
         url = reverse("object-detail", args=[object.uuid])
 
         response = self.client.delete(url, **GEO_WRITE_KWARGS)

src/objects/tests/v1/test_filters.py

@@ -1,6 +1,4 @@
-from unittest.mock import patch
-
-from django.db.utils import ProgrammingError
+from datetime import date, timedelta
 
 from rest_framework import status
 from rest_framework.test import APITestCase
@@ -312,6 +310,24 @@ def test_filter_icontains_numeric(self):
             f"http://testserver{reverse('object-detail', args=[record.object.uuid])}",
         )
 
+    def test_filter_exclude_old_records(self):
+        record_old = ObjectRecordFactory.create(
+            data={"diameter": 45},
+            object__object_type=self.object_type,
+            start_at=date.today() - timedelta(days=10),
+            end_at=date.today() - timedelta(days=1),
+        )
+        record_new = ObjectRecordFactory.create(
+            data={"diameter": 50}, object=record_old.object, start_at=record_old.end_at
+        )
+
+        response = self.client.get(self.url, {"data_attrs": "diameter__exact__45"})
+
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+        data = response.json()
+        self.assertEqual(len(data), 0)
+
 
 class FilterDateTests(TokenAuthMixin, APITestCase):
     @classmethod

src/objects/tests/v1/test_geo_headers.py

@@ -1,7 +1,11 @@
 from rest_framework import status
 from rest_framework.test import APITestCase
 
-from objects.core.tests.factories import ObjectFactory, ObjectTypeFactory
+from objects.core.tests.factories import (
+    ObjectFactory,
+    ObjectRecordFactory,
+    ObjectTypeFactory,
+)
 from objects.token.constants import PermissionModes
 from objects.token.tests.factories import PermissionFactory
 from objects.utils.test import TokenAuthMixin
@@ -30,6 +34,7 @@ def assertResponseHasGeoHeaders(self, response):
 
     def test_get_without_geo_headers(self):
         object = ObjectFactory.create(object_type=self.object_type)
+        ObjectRecordFactory.create(object=object)
         url = reverse("object-detail", args=[object.uuid])
 
         response = self.client.get(url)
@@ -39,6 +44,7 @@ def test_get_without_geo_headers(self):
 
     def test_get_with_geo_headers(self):
         object = ObjectFactory.create(object_type=self.object_type)
+        ObjectRecordFactory.create(object=object)
         url = reverse("object-detail", args=[object.uuid])
 
         response = self.client.get(url, **GEO_READ_KWARGS)
@@ -93,6 +99,7 @@ def test_update_without_geo_headers(self):
 
     def test_delete_without_geo_headers(self):
         object = ObjectFactory.create(object_type=self.object_type)
+        ObjectRecordFactory.create(object=object)
         url = reverse("object-detail", args=[object.uuid])
 
         response = self.client.delete(url)