From cf1f0eaac3b72c56b42b381aecb055b937a009d6 Mon Sep 17 00:00:00 2001 From: Zach Aysan Date: Wed, 29 Jan 2025 20:52:10 +0000 Subject: [PATCH 1/6] Set existing environments to different names --- api/environments/views.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/api/environments/views.py b/api/environments/views.py index c035e8bc6218..d8d96418428c 100644 --- a/api/environments/views.py +++ b/api/environments/views.py @@ -140,6 +140,11 @@ def get_queryset(self): return queryset def perform_create(self, serializer): + existing_environment = Environment.objects.filter( + name=serializer.validated_data["name"] + ) + if existing_environment: + raise ValidationError("Existing environment for given name.") environment = serializer.save() if getattr(self.request.user, "is_master_api_key_user", False) is False: UserEnvironmentPermission.objects.create( From cf12c63b6ec8d575f4f51fdafcb8e1f48a4345e0 Mon Sep 17 00:00:00 2001 From: Zach Aysan Date: Wed, 29 Jan 2025 20:53:04 +0000 Subject: [PATCH 2/6] Add test for conflicting environment names --- .../test_unit_environments_views.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/api/tests/unit/environments/test_unit_environments_views.py b/api/tests/unit/environments/test_unit_environments_views.py index 41c39911b4e6..e87e46ca7516 100644 --- a/api/tests/unit/environments/test_unit_environments_views.py +++ b/api/tests/unit/environments/test_unit_environments_views.py @@ -588,6 +588,31 @@ def test_should_create_environments( ).exists() +def test_environment_matches_existing_environment_name( + project: Project, + admin_client: APIClient, +) -> None: + # Given + url = reverse("api-v1:environments:environment-list") + description = "This is the description" + name = "Test environment" + data = { + "name": name, + "project": project.id, + "description": description, + } + Environment.objects.create(name=name, description=description, project=project) + + # When + response = admin_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == 400 + assert response.json() == ["Existing environment for given name."] + + def test_create_environment_without_required_metadata_returns_400( project, admin_client_new, From de08e32747a79910c6d5785465338d73a0a2b668 Mon Sep 17 00:00:00 2001 From: Zach Aysan Date: Thu, 30 Jan 2025 14:12:56 +0000 Subject: [PATCH 3/6] Add project to filter --- api/environments/views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/environments/views.py b/api/environments/views.py index d8d96418428c..6ee918b087c2 100644 --- a/api/environments/views.py +++ b/api/environments/views.py @@ -141,7 +141,8 @@ def get_queryset(self): def perform_create(self, serializer): existing_environment = Environment.objects.filter( - name=serializer.validated_data["name"] + name=serializer.validated_data["name"], + project=serializer.validated_data["project"], ) if existing_environment: raise ValidationError("Existing environment for given name.") From 6d12996ec1168b363ab3677cd65903499802bf70 Mon Sep 17 00:00:00 2001 From: Zach Aysan Date: Fri, 31 Jan 2025 14:21:16 +0000 Subject: [PATCH 4/6] Update environment exists error message --- api/tests/unit/environments/test_unit_environments_views.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/api/tests/unit/environments/test_unit_environments_views.py b/api/tests/unit/environments/test_unit_environments_views.py index e87e46ca7516..6c0d6f6e5eed 100644 --- a/api/tests/unit/environments/test_unit_environments_views.py +++ b/api/tests/unit/environments/test_unit_environments_views.py @@ -610,7 +610,9 @@ def test_environment_matches_existing_environment_name( # Then assert response.status_code == 400 - assert response.json() == ["Existing environment for given name."] + assert response.json() == { + "non_field_errors": ["An environment with this name already exists."] + } def test_create_environment_without_required_metadata_returns_400( From d8ee62c435af01a05985d10fe72bbdf8fdf68b34 Mon Sep 17 00:00:00 2001 From: Zach Aysan Date: Fri, 31 Jan 2025 14:21:41 +0000 Subject: [PATCH 5/6] Remove views enforcement of environment names --- api/environments/views.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/api/environments/views.py b/api/environments/views.py index 6ee918b087c2..c035e8bc6218 100644 --- a/api/environments/views.py +++ b/api/environments/views.py @@ -140,12 +140,6 @@ def get_queryset(self): return queryset def perform_create(self, serializer): - existing_environment = Environment.objects.filter( - name=serializer.validated_data["name"], - project=serializer.validated_data["project"], - ) - if existing_environment: - raise ValidationError("Existing environment for given name.") environment = serializer.save() if getattr(self.request.user, "is_master_api_key_user", False) is False: UserEnvironmentPermission.objects.create( From c4e2bbd8ed1b17f8e38c9faa1f77a0488b119fc3 Mon Sep 17 00:00:00 2001 From: Zach Aysan Date: Fri, 31 Jan 2025 14:22:06 +0000 Subject: [PATCH 6/6] Create a validator for update and create methods to catch name collisions --- api/environments/serializers.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/api/environments/serializers.py b/api/environments/serializers.py index 70ac6f9be8cc..e00788ec5089 100644 --- a/api/environments/serializers.py +++ b/api/environments/serializers.py @@ -111,6 +111,15 @@ class CreateUpdateEnvironmentSerializer( invalid_plans = ("free",) field_names = ("minimum_change_request_approvals",) + class Meta(EnvironmentSerializerWithMetadata.Meta): + validators = [ + serializers.UniqueTogetherValidator( + queryset=EnvironmentSerializerWithMetadata.Meta.model.objects.all(), + fields=("name", "project"), + message="An environment with this name already exists.", + ) + ] + def get_subscription(self) -> typing.Optional[Subscription]: view = self.context["view"]