diff --git a/api/features/constants.py b/api/features/constants.py index 92bc3f7be16c..1970279e7c67 100644 --- a/api/features/constants.py +++ b/api/features/constants.py @@ -6,3 +6,7 @@ # Feature state statuses COMMITTED = "COMMITTED" DRAFT = "DRAFT" + +# Tag filtering strategy +UNION = "UNION" +INTERSECTION = "INTERSECTION" diff --git a/api/features/import_export/permissions.py b/api/features/import_export/permissions.py index c9183f4a1693..4618e0816501 100644 --- a/api/features/import_export/permissions.py +++ b/api/features/import_export/permissions.py @@ -15,8 +15,13 @@ def has_permission( if not super().has_permission(request, view): return False - environment = Environment.objects.get(id=view.kwargs["environment_id"]) - return request.user.is_environment_admin(environment) + environment = Environment.objects.select_related( + "project__organisation", + ).get(id=view.kwargs["environment_id"]) + organisation = environment.project.organisation + + # Since feature imports can be destructive, use org admin. + return request.user.is_organisation_admin(organisation) class CreateFeatureExportPermissions(IsAuthenticated): diff --git a/api/features/import_export/views.py b/api/features/import_export/views.py index a02215903c3e..b39d9dd34f3d 100644 --- a/api/features/import_export/views.py +++ b/api/features/import_export/views.py @@ -120,7 +120,6 @@ def get_queryset(self) -> QuerySet[FeatureExport]: if user.is_environment_admin(environment): environment_ids.append(environment.id) - # Order by environment name to match environment list order. return FeatureExport.objects.filter(environment__in=environment_ids).order_by( - "environment__name" + "-created_at" ) diff --git a/api/features/serializers.py b/api/features/serializers.py index 82c50f1d0652..26b6ea771c7c 100644 --- a/api/features/serializers.py +++ b/api/features/serializers.py @@ -20,6 +20,7 @@ DeleteBeforeUpdateWritableNestedModelSerializer, ) +from .constants import INTERSECTION, UNION from .feature_segments.serializers import ( CreateSegmentOverrideFeatureSegmentSerializer, ) @@ -81,8 +82,16 @@ class FeatureQuerySerializer(serializers.Serializer): sort_direction = serializers.ChoiceField(choices=("ASC", "DESC"), default="ASC") tags = serializers.CharField( - required=False, help_text="Comma separated list of tag ids to filter on (AND)" + required=False, + help_text=( + "Comma separated list of tag ids to filter on (AND with " + "INTERSECTION, and OR with UNION via tag_strategy)" + ), + ) + tag_strategy = serializers.ChoiceField( + choices=(UNION, INTERSECTION), default=INTERSECTION ) + is_archived = serializers.BooleanField(required=False) environment = serializers.IntegerField( required=False, diff --git a/api/features/views.py b/api/features/views.py index 8b8642e14219..0a651414bfe1 100644 --- a/api/features/views.py +++ b/api/features/views.py @@ -38,6 +38,7 @@ from users.models import FFAdminUser, UserPermissionGroup from webhooks.webhooks import WebhookEventType +from .constants import INTERSECTION, UNION from .features_service import get_overrides_data from .models import Feature, FeatureState from .permissions import ( @@ -290,7 +291,10 @@ def _filter_queryset(self, queryset: QuerySet) -> QuerySet: if "tags" in query_serializer.initial_data: if query_data.get("tags", "") == "": queryset = queryset.filter(tags__isnull=True) + elif query_data["tag_strategy"] == UNION: + queryset = queryset.filter(tags__in=query_data["tags"]) else: + assert query_data["tag_strategy"] == INTERSECTION queryset = reduce( lambda qs, tag_id: qs.filter(tags=tag_id), query_data["tags"], diff --git a/api/tests/unit/features/import_export/test_unit_features_import_export_views.py b/api/tests/unit/features/import_export/test_unit_features_import_export_views.py index 6c377eb96db5..34b5f51552f5 100644 --- a/api/tests/unit/features/import_export/test_unit_features_import_export_views.py +++ b/api/tests/unit/features/import_export/test_unit_features_import_export_views.py @@ -52,13 +52,13 @@ def test_list_feature_exports( # Then assert response.status_code == 200 - assert response.data["results"][0]["environment_id"] == environment.id - assert response.data["results"][0]["id"] == feature_export1.id - assert response.data["results"][0]["name"].startswith(f"{environment.name} | ") + assert response.data["results"][0]["environment_id"] == environment2.id + assert response.data["results"][0]["id"] == feature_export2.id + assert response.data["results"][0]["name"].startswith(f"{environment2.name} | ") - assert response.data["results"][1]["environment_id"] == environment2.id - assert response.data["results"][1]["id"] == feature_export2.id - assert response.data["results"][1]["name"].startswith(f"{environment2.name} | ") + assert response.data["results"][1]["environment_id"] == environment.id + assert response.data["results"][1]["id"] == feature_export1.id + assert response.data["results"][1]["name"].startswith(f"{environment.name} | ") def test_list_feature_export_with_filtered_environments( diff --git a/api/tests/unit/features/test_unit_features_views.py b/api/tests/unit/features/test_unit_features_views.py index b8094f728edc..06b4bcf40480 100644 --- a/api/tests/unit/features/test_unit_features_views.py +++ b/api/tests/unit/features/test_unit_features_views.py @@ -2404,3 +2404,105 @@ def test_list_features_n_plus_1( # Then assert response.status_code == status.HTTP_200_OK + + +def test_list_features_with_union_tag( + staff_client: APIClient, + project: Project, + feature: Feature, + with_project_permissions: Callable, + django_assert_num_queries: DjangoAssertNumQueries, + environment: Environment, +) -> None: + # Given + with_project_permissions([VIEW_PROJECT]) + + tag1 = Tag.objects.create( + label="Test Tag", + color="#fffff", + description="Test Tag description", + project=project, + ) + tag2 = Tag.objects.create( + label="Test Tag2", + color="#fffff", + description="Test Tag2 description", + project=project, + ) + feature2 = Feature.objects.create( + name="another_feature", project=project, initial_value="initial_value" + ) + feature3 = Feature.objects.create( + name="missing_feature", project=project, initial_value="gone" + ) + feature2.tags.add(tag1) + feature3.tags.add(tag2) + + base_url = reverse("api-v1:projects:project-features-list", args=[project.id]) + url = ( + f"{base_url}?environment={environment.id}&tags={tag1.id}" + f",{tag2.id}&tag_strategy=UNION" + ) + # When + response = staff_client.get(url) + + # Then + assert response.status_code == status.HTTP_200_OK + + # The first feature has been filtered out of the results. + assert len(response.data["results"]) == 2 + + assert response.data["results"][0]["id"] == feature2.id + assert response.data["results"][0]["tags"] == [tag1.id] + assert response.data["results"][1]["id"] == feature3.id + assert response.data["results"][1]["tags"] == [tag2.id] + + +def test_list_features_with_intersection_tag( + staff_client: APIClient, + project: Project, + feature: Feature, + with_project_permissions: Callable, + django_assert_num_queries: DjangoAssertNumQueries, + environment: Environment, +) -> None: + # Given + with_project_permissions([VIEW_PROJECT]) + + tag1 = Tag.objects.create( + label="Test Tag", + color="#fffff", + description="Test Tag description", + project=project, + ) + tag2 = Tag.objects.create( + label="Test Tag2", + color="#fffff", + description="Test Tag2 description", + project=project, + ) + feature2 = Feature.objects.create( + name="another_feature", project=project, initial_value="initial_value" + ) + feature3 = Feature.objects.create( + name="missing_feature", project=project, initial_value="gone" + ) + feature2.tags.add(tag1, tag2) + feature3.tags.add(tag2) + + base_url = reverse("api-v1:projects:project-features-list", args=[project.id]) + url = ( + f"{base_url}?environment={environment.id}&tags={tag1.id}" + f",{tag2.id}&tag_strategy=INTERSECTION" + ) + # When + response = staff_client.get(url) + + # Then + assert response.status_code == status.HTTP_200_OK + + # Only feature2 has both tags, so it is the only result. + assert len(response.data["results"]) == 1 + + assert response.data["results"][0]["id"] == feature2.id + assert response.data["results"][0]["tags"] == [tag1.id, tag2.id]