diff --git a/api/features/import_export/permissions.py b/api/features/import_export/permissions.py index 4618e0816501..8325e5d1c15e 100644 --- a/api/features/import_export/permissions.py +++ b/api/features/import_export/permissions.py @@ -52,7 +52,18 @@ def has_permission(self, request: Request, view: ListAPIView) -> bool: if not super().has_permission(request, view): return False - project = Project.objects.get(id=view.kwargs["project_id"]) + project = Project.objects.get(id=view.kwargs["project_pk"]) # The user will only see environment feature exports # that the user is an environment admin. return request.user.has_project_permission(VIEW_PROJECT, project) + + +class FeatureImportListPermissions(IsAuthenticated): + def has_permission(self, request: Request, view: ListAPIView) -> bool: + if not super().has_permission(request, view): + return False + + project = Project.objects.get(id=view.kwargs["project_pk"]) + # The user will only see environment feature imports + # that the user is an environment admin. + return request.user.has_project_permission(VIEW_PROJECT, project) diff --git a/api/features/import_export/serializers.py b/api/features/import_export/serializers.py index c945a6f04ca8..4645c8d8c75f 100644 --- a/api/features/import_export/serializers.py +++ b/api/features/import_export/serializers.py @@ -86,6 +86,7 @@ class Meta: fields = ( "id", "environment_id", + "strategy", "status", "created_at", ) diff --git a/api/features/import_export/tasks.py b/api/features/import_export/tasks.py index 7d60dc3f3e2f..c93a900db994 100644 --- a/api/features/import_export/tasks.py +++ b/api/features/import_export/tasks.py @@ -105,6 +105,16 @@ def export_features_for_environment( @register_task_handler() def import_features_for_environment(feature_import_id: int) -> None: feature_import = FeatureImport.objects.get(id=feature_import_id) + try: + _import_features_for_environment(feature_import) + assert feature_import.status == SUCCESS + except Exception: + feature_import.status = FAILED + feature_import.save() + raise + + +def _import_features_for_environment(feature_import: FeatureImport) -> None: environment = feature_import.environment input_data = json.loads(feature_import.data) project = environment.project @@ -126,6 +136,9 @@ def import_features_for_environment(feature_import_id: int) -> None: _create_new_feature(feature_data, project, environment) + feature_import.status = SUCCESS + feature_import.save() + def _save_feature_state_value_with_type( value: Optional[Union[int, bool, str]], diff --git a/api/features/import_export/views.py b/api/features/import_export/views.py index b39d9dd34f3d..644094475f61 100644 --- a/api/features/import_export/views.py +++ b/api/features/import_export/views.py @@ -12,11 +12,16 @@ from environments.models import Environment -from .models import FeatureExport, FlagsmithOnFlagsmithFeatureExport +from .models import ( + FeatureExport, + FeatureImport, + FlagsmithOnFlagsmithFeatureExport, +) from .permissions import ( CreateFeatureExportPermissions, DownloadFeatureExportPermissions, FeatureExportListPermissions, + FeatureImportListPermissions, FeatureImportPermissions, ) from .serializers import ( @@ -115,7 +120,7 @@ def get_queryset(self) -> QuerySet[FeatureExport]: user = self.request.user for environment in Environment.objects.filter( - project_id=self.kwargs["project_id"], + project_id=self.kwargs["project_pk"], ): if user.is_environment_admin(environment): environment_ids.append(environment.id) @@ -123,3 +128,22 @@ def get_queryset(self) -> QuerySet[FeatureExport]: return FeatureExport.objects.filter(environment__in=environment_ids).order_by( "-created_at" ) + + +class FeatureImportListView(ListAPIView): + serializer_class = FeatureImportSerializer + permission_classes = [FeatureImportListPermissions] + + def get_queryset(self) -> QuerySet[FeatureImport]: + environment_ids = [] + user = self.request.user + + for environment in Environment.objects.filter( + project_id=self.kwargs["project_pk"], + ): + if user.is_environment_admin(environment): + environment_ids.append(environment.id) + + return FeatureImport.objects.filter(environment__in=environment_ids).order_by( + "-created_at" + ) diff --git a/api/features/urls.py b/api/features/urls.py index db5748fe9066..4da8b71cb29f 100644 --- a/api/features/urls.py +++ b/api/features/urls.py @@ -19,6 +19,7 @@ router = routers.DefaultRouter() router.register(r"featurestates", SimpleFeatureStateViewSet, basename="featurestates") router.register(r"feature-segments", FeatureSegmentViewSet, basename="feature-segment") + app_name = "features" urlpatterns = [ diff --git a/api/projects/urls.py b/api/projects/urls.py index 3cd9d0ed3869..fe2d55c38b24 100644 --- a/api/projects/urls.py +++ b/api/projects/urls.py @@ -3,7 +3,10 @@ from rest_framework_nested import routers from audit.views import ProjectAuditLogViewSet -from features.import_export.views import FeatureExportListView +from features.import_export.views import ( + FeatureExportListView, + FeatureImportListView, +) from features.multivariate.views import MultivariateFeatureOptionViewSet from features.views import FeatureViewSet from integrations.datadog.views import DataDogConfigurationViewSet @@ -56,7 +59,6 @@ ProjectAuditLogViewSet, basename="project-audit", ) - nested_features_router = routers.NestedSimpleRouter( projects_router, r"features", lookup="feature" ) @@ -76,8 +78,13 @@ name="all-user-permissions", ), path( - "/feature-exports/", + "/feature-exports/", FeatureExportListView.as_view(), name="feature-exports", ), + path( + "/feature-imports/", + FeatureImportListView.as_view(), + name="feature-imports", + ), ] diff --git a/api/tests/unit/features/import_export/test_unit_features_import_export_tasks.py b/api/tests/unit/features/import_export/test_unit_features_import_export_tasks.py index 343337559389..a86aa0d93906 100644 --- a/api/tests/unit/features/import_export/test_unit_features_import_export_tasks.py +++ b/api/tests/unit/features/import_export/test_unit_features_import_export_tasks.py @@ -132,6 +132,9 @@ def test_export_and_import_features_for_environment_with_skip( import_features_for_environment(feature_import.id) # Then + feature_import.refresh_from_db() + assert feature_import.status == SUCCESS + assert project2.features.count() == 4 overlapping_feature.refresh_from_db() assert overlapping_feature.deleted_at is None 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 4c63a9bda660..123c72809d71 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 @@ -339,3 +339,82 @@ def test_download_flagsmith_on_flagsmith_when_success( # Then assert response.status_code == 200 assert response.data == [{"feature": "data"}] + + +def test_list_feature_import_with_filtered_environments( + staff_client: APIClient, + staff_user: FFAdminUser, + project: Project, + environment: Environment, + with_project_permissions: WithProjectPermissionsCallable, +) -> None: + # Given + with_project_permissions([VIEW_PROJECT]) + environment2 = Environment.objects.create( + name="Allowed admin for this environment", + project=project, + ) + + # Staff user is only set as an admin on the second environment + UserEnvironmentPermission.objects.create( + user=staff_user, + environment=environment2, + admin=True, + ) + + # Create a FeatureImport that will be filtered out. + FeatureImport.objects.create( + environment=environment, + strategy=OVERWRITE_DESTRUCTIVE, + status=PROCESSING, + data="{}", + ) + # Create a FeatureImport that will be included + feature_import2 = FeatureImport.objects.create( + environment=environment2, + strategy=OVERWRITE_DESTRUCTIVE, + status=PROCESSING, + data="{}", + ) + + url = reverse( + "api-v1:projects:feature-imports", + args=[project.id], + ) + + # When + response = staff_client.get(url) + + # Then + assert response.status_code == 200 + assert response.data["count"] == 1 + + # Only the second environment is included in the results. + assert response.data["results"][0]["environment_id"] == environment2.id + assert response.data["results"][0]["id"] == feature_import2.id + assert response.data["results"][0]["status"] == PROCESSING + assert response.data["results"][0]["strategy"] == OVERWRITE_DESTRUCTIVE + + +def test_list_feature_import_unauthorized( + staff_client: APIClient, + project: Project, + environment: Environment, +) -> None: + # Given + FeatureImport.objects.create( + environment=environment, + strategy=OVERWRITE_DESTRUCTIVE, + status=PROCESSING, + data="{}", + ) + url = reverse( + "api-v1:projects:feature-imports", + args=[project.id], + ) + + # When + response = staff_client.get(url) + + # Then + assert response.status_code == 403