add evaluation data filters, improve coverage

Author: khvn26

api/app_analytics/serializers.py

@@ -1,4 +1,4 @@
-from typing import Any, get_args
+from typing import TYPE_CHECKING, Any, get_args
 
 from django.conf import settings
 from rest_framework import serializers
@@ -12,6 +12,30 @@
 from environments.models import Environment
 from features.models import FeatureState
 
+if TYPE_CHECKING:
+    _SerializerType = serializers.Serializer[Any]
+else:
+    _SerializerType = object
+
+
+class LabelsQuerySerializerMixin(_SerializerType):
+    def get_fields(self) -> dict[str, serializers.Field[Any, Any, Any, Any]]:
+        return {**super().get_fields(), **_get_label_fields()}
+
+    def validate(self, attrs: dict[str, Any]) -> dict[str, Any]:
+        """
+        Yank label filters from the top level and place them under a
+        separate `labels_filter` key
+        """
+        attrs = super().validate(attrs)
+        if labels_filter := {
+            label: attrs.pop(label)
+            for label in LABELS
+            if label in attrs and attrs[label] is not None
+        }:
+            attrs["labels_filter"] = labels_filter
+        return attrs
+
 
 class LabelsSerializer(serializers.Serializer[Labels]):
     def get_fields(self) -> dict[str, serializers.Field[Any, Any, Any, Any]]:
@@ -27,7 +51,7 @@ class UsageDataSerializer(serializers.Serializer):  # type: ignore[type-arg]
     labels = LabelsSerializer(allow_null=True, required=False)
 
 
-class UsageDataQuerySerializer(serializers.Serializer):  # type: ignore[type-arg]
+class UsageDataQuerySerializer(LabelsQuerySerializerMixin, serializers.Serializer):  # type: ignore[type-arg]
     project_id = serializers.IntegerField(required=False)
     environment_id = serializers.IntegerField(required=False)
     period = serializers.ChoiceField(
@@ -37,23 +61,6 @@ class UsageDataQuerySerializer(serializers.Serializer):  # type: ignore[type-arg
         required=False,
     )
 
-    def get_fields(self) -> dict[str, serializers.Field[Any, Any, Any, Any]]:
-        return {**super().get_fields(), **_get_label_fields()}
-
-    def validate(self, attrs: dict[str, Any]) -> dict[str, Any]:
-        """
-        Yank label filters from the top level and place them under a
-        separate `labels_filter` key
-        """
-        attrs = super().validate(attrs)
-        if labels_filter := {
-            label: attrs.pop(label)
-            for label in LABELS
-            if label in attrs and attrs[label] is not None
-        }:
-            attrs["labels_filter"] = labels_filter
-        return attrs
-
 
 class UsageTotalCountSerializer(serializers.Serializer):  # type: ignore[type-arg]
     count = serializers.IntegerField()

api/features/serializers.py

@@ -21,6 +21,7 @@
 from rest_framework import serializers
 from rest_framework.exceptions import PermissionDenied
 
+from app_analytics.serializers import LabelsQuerySerializerMixin, LabelsSerializer
 from environments.identities.models import Identity
 from environments.sdk.serializers_mixins import (
     HideSensitiveFieldsSerializerMixin,
@@ -597,6 +598,7 @@ class FeatureInfluxDataSerializer(serializers.Serializer):  # type: ignore[type-
 class FeatureEvaluationDataSerializer(serializers.Serializer):  # type: ignore[type-arg]
     day = serializers.CharField()
     count = serializers.IntegerField()
+    labels = LabelsSerializer(allow_null=True)
 
 
 class GetInfluxDataQuerySerializer(serializers.Serializer):  # type: ignore[type-arg]
@@ -605,7 +607,7 @@ class GetInfluxDataQuerySerializer(serializers.Serializer):  # type: ignore[type
     aggregate_every = serializers.CharField(required=False, default="24h")
 
 
-class GetUsageDataQuerySerializer(serializers.Serializer):  # type: ignore[type-arg]
+class GetUsageDataQuerySerializer(LabelsQuerySerializerMixin, serializers.Serializer):  # type: ignore[type-arg]
     period = serializers.IntegerField(
         required=False, default=30, help_text="number of days"
     )

api/features/views.py

@@ -382,11 +382,11 @@ def get_influx_data(self, request, pk, project_pk):  # type: ignore[no-untyped-d
     def get_evaluation_data(self, request, pk, project_pk):  # type: ignore[no-untyped-def]
         feature = get_object_or_404(Feature, pk=pk)
 
-        query_serializer = GetUsageDataQuerySerializer(data=request.query_params)
-        query_serializer.is_valid(raise_exception=True)
+        filters = GetUsageDataQuerySerializer(data=request.query_params)
+        filters.is_valid(raise_exception=True)
 
         usage_data = get_feature_evaluation_data(
-            feature=feature, **query_serializer.data
+            feature=feature, **filters.validated_data
         )
         serializer = FeatureEvaluationDataSerializer(usage_data, many=True)
 

api/tests/unit/app_analytics/test_unit_app_analytics_views.py

@@ -307,6 +307,55 @@ def test_get_usage_data__ninety_day_period(
     )
 
 
+def test_get_usage_data__labels_filter__returns_expected(
+    mocker: MockerFixture,
+    admin_client_new: APIClient,
+    organisation: Organisation,
+) -> None:
+    # Given
+    today = date.today()
+
+    url = reverse("api-v1:organisations:usage-data", args=[organisation.id])
+    url += "?client_application_name=test-app"
+
+    mocked_get_usage_data = mocker.patch(
+        "app_analytics.views.get_usage_data",
+        autospec=True,
+        return_value=[
+            UsageData(
+                flags=10,
+                day=today,
+                labels={"client_application_name": "test-app"},
+            ),
+        ],
+    )
+
+    # When
+    response = admin_client_new.get(url)
+
+    # Then
+    assert response.status_code == status.HTTP_200_OK
+    assert response.json() == [
+        {
+            "flags": 10,
+            "day": str(today),
+            "identities": 0,
+            "traits": 0,
+            "environment_document": 0,
+            "labels": {
+                "client_application_name": "test-app",
+                "client_application_version": None,
+            },
+        },
+    ]
+
+    mocked_get_usage_data.assert_called_once_with(
+        organisation=organisation,
+        period=None,
+        labels_filter={"client_application_name": "test-app"},
+    )
+
+
 def test_get_usage_data_for_non_admin_user_returns_403(  # type: ignore[no-untyped-def]
     mocker, test_user_client, organisation
 ):

api/tests/unit/features/test_unit_features_views.py

@@ -872,28 +872,98 @@ def test_get_feature_evaluation_data(
     )
     url = f"{base_url}?environment_id={environment.id}"
     mocked_get_feature_evaluation_data = mocker.patch(
-        "features.views.get_feature_evaluation_data", autospec=True
+        "features.views.get_feature_evaluation_data",
+        autospec=True,
     )
+    today = date.today()
     mocked_get_feature_evaluation_data.return_value = [
-        FeatureEvaluationData(count=10, day=date.today()),
-        FeatureEvaluationData(count=10, day=date.today() - timedelta(days=1)),
+        FeatureEvaluationData(count=10, day=today),
+        FeatureEvaluationData(
+            count=10,
+            day=today,
+            labels={"client_application_name": "web"},
+        ),
+        FeatureEvaluationData(count=10, day=today - timedelta(days=1)),
     ]
     # When
     response = admin_client_new.get(url)
 
     # Then
     assert response.status_code == status.HTTP_200_OK
-    assert len(response.json()) == 2
-    assert response.json()[0] == {"day": str(date.today()), "count": 10}
-    assert response.json()[1] == {
-        "day": str(date.today() - timedelta(days=1)),
-        "count": 10,
-    }
+    assert response.json() == [
+        {
+            "count": 10,
+            "day": str(today),
+            "labels": None,
+        },
+        {
+            "count": 10,
+            "day": str(today),
+            "labels": {
+                "client_application_name": "web",
+                "client_application_version": None,
+            },
+        },
+        {
+            "count": 10,
+            "day": str(today - timedelta(days=1)),
+            "labels": None,
+        },
+    ]
     mocked_get_feature_evaluation_data.assert_called_with(
         feature=feature, period=30, environment_id=environment.id
     )
 
 
+def test_get_feature_evaluation_data__labels_filter__returns_expected(
+    project: Project,
+    feature: Feature,
+    environment: Environment,
+    mocker: MockerFixture,
+    admin_client_new: APIClient,
+) -> None:
+    # Given
+    base_url = reverse(
+        "api-v1:projects:project-features-get-evaluation-data",
+        args=[project.id, feature.id],
+    )
+    url = f"{base_url}?environment_id={environment.id}&client_application_name=web"
+    mocked_get_feature_evaluation_data = mocker.patch(
+        "features.views.get_feature_evaluation_data",
+        autospec=True,
+    )
+    today = date.today()
+    mocked_get_feature_evaluation_data.return_value = [
+        FeatureEvaluationData(
+            count=10,
+            day=today,
+            labels={"client_application_name": "web"},
+        ),
+    ]
+
+    # When
+    response = admin_client_new.get(url)
+
+    # Then
+    assert response.status_code == status.HTTP_200_OK
+    assert response.json() == [
+        {
+            "count": 10,
+            "day": str(today),
+            "labels": {
+                "client_application_name": "web",
+                "client_application_version": None,
+            },
+        },
+    ]
+    mocked_get_feature_evaluation_data.assert_called_with(
+        feature=feature,
+        period=30,
+        environment_id=environment.id,
+        labels_filter={"client_application_name": "web"},
+    )
+
+
 def test_create_segment_override_forbidden(
     feature: Feature,
     segment: Segment,