fix: Fine tune feature import export (#3163)

api/features/constants.py

@@ -6,3 +6,7 @@
 # Feature state statuses
 COMMITTED = "COMMITTED"
 DRAFT = "DRAFT"
+
+# Tag filtering strategy
+UNION = "UNION"
+INTERSECTION = "INTERSECTION"

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):

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"
         )

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,

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"],

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(

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]