diff --git a/CHANGELOG.md b/CHANGELOG.md index d1a42a7b..dad15e3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Allow mobile app to access escalation options endpoints @imtoori ([#3847](https://github.com/grafana/oncall/pull/3847)) - Enable templating for alert escalation mobile app push notifications by @joeyorlando ([#3845](https://github.com/grafana/oncall/pull/3845)) - Change email notification template to not wrap user template @mderynck ([#3862](https://github.com/grafana/oncall/pull/3862)) +- Update integration name uniqueness check to be per team ([#3863](https://github.com/grafana/oncall/pull/3863)) ### Fixed diff --git a/engine/apps/api/serializers/alert_receive_channel.py b/engine/apps/api/serializers/alert_receive_channel.py index 0c1e4f87..1edd7bd3 100644 --- a/engine/apps/api/serializers/alert_receive_channel.py +++ b/engine/apps/api/serializers/alert_receive_channel.py @@ -287,6 +287,27 @@ class AlertReceiveChannelSerializer( ] extra_kwargs = {"integration": {"required": True}} + def validate(self, data): + validated_data = super().validate(data) + organization = self.context["request"].auth.organization + verbal_name = validated_data.get("verbal_name") + team = validated_data.get("team") + try: + obj = AlertReceiveChannel.objects.get(organization=organization, team=team, verbal_name=verbal_name) + except AlertReceiveChannel.DoesNotExist: + pass + except AlertReceiveChannel.MultipleObjectsReturned: + raise serializers.ValidationError( + {"verbal_name": "An integration with this name already exists for this team"} + ) + else: + if self.instance is None or obj.id != self.instance.id: + raise serializers.ValidationError( + {"verbal_name": "An integration with this name already exists for this team"} + ) + + return validated_data + def create(self, validated_data): organization = self.context["request"].auth.organization integration = validated_data.get("integration") @@ -356,19 +377,6 @@ class AlertReceiveChannelSerializer( return integration - def validate_verbal_name(self, verbal_name): - organization = self.context["request"].auth.organization - if verbal_name is None or (self.instance and verbal_name == self.instance.verbal_name): - return verbal_name - try: - obj = AlertReceiveChannel.objects.get(organization=organization, verbal_name=verbal_name) - except AlertReceiveChannel.DoesNotExist: - return verbal_name - if self.instance and obj.id == self.instance.id: - return verbal_name - else: - raise serializers.ValidationError(detail="Integration with this name already exists") - def get_allow_delete(self, obj: "AlertReceiveChannel") -> bool: # don't allow deleting direct paging integrations return obj.integration != AlertReceiveChannel.INTEGRATION_DIRECT_PAGING diff --git a/engine/apps/api/tests/test_alert_receive_channel.py b/engine/apps/api/tests/test_alert_receive_channel.py index 037e64bc..c1ba4e3c 100644 --- a/engine/apps/api/tests/test_alert_receive_channel.py +++ b/engine/apps/api/tests/test_alert_receive_channel.py @@ -150,6 +150,100 @@ def test_create_alert_receive_channel(alert_receive_channel_internal_api_setup, assert response.status_code == status.HTTP_201_CREATED +@pytest.mark.django_db +def test_alert_receive_channel_name_uniqueness( + alert_receive_channel_internal_api_setup, + make_team, + make_user_auth_headers, +): + user, token, alert_receive_channel = alert_receive_channel_internal_api_setup + client = APIClient() + + url = reverse("api-internal:alert_receive_channel-list") + data = { + "integration": AlertReceiveChannel.INTEGRATION_GRAFANA, + "team": alert_receive_channel.team.public_primary_key if alert_receive_channel.team else None, + "verbal_name": alert_receive_channel.verbal_name, + } + response = client.post(url, data, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + # name can be reused in a different team + another_team = make_team(alert_receive_channel.organization) + data = { + "integration": AlertReceiveChannel.INTEGRATION_GRAFANA, + "team": another_team.public_primary_key, + "verbal_name": alert_receive_channel.verbal_name, + } + response = client.post(url, data, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_201_CREATED + + # update works + url = reverse("api-internal:alert_receive_channel-detail", kwargs={"pk": alert_receive_channel.public_primary_key}) + response = client.put( + url, + data=json.dumps( + { + "team": alert_receive_channel.team, + "verbal_name": alert_receive_channel.verbal_name, + "description": "update description", + } + ), + content_type="application/json", + **make_user_auth_headers(user, token), + ) + assert response.status_code == status.HTTP_200_OK + + # but updating team will fail if name exists + response = client.put( + url, + data=json.dumps( + { + "team": another_team.public_primary_key, + "verbal_name": alert_receive_channel.verbal_name, + } + ), + content_type="application/json", + **make_user_auth_headers(user, token), + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + +@pytest.mark.django_db +def test_alert_receive_channel_name_duplicated( + alert_receive_channel_internal_api_setup, + make_alert_receive_channel, + make_user_auth_headers, +): + # this could happen in case a team is removed and integrations are set to have "no team" + user, token, alert_receive_channel = alert_receive_channel_internal_api_setup + # another integration with the same verbal name + make_alert_receive_channel( + alert_receive_channel.organization, + verbal_name=alert_receive_channel.verbal_name, + ) + + client = APIClient() + + # updating team will require changing the name or the team + url = reverse("api-internal:alert_receive_channel-detail", kwargs={"pk": alert_receive_channel.public_primary_key}) + response = client.put( + url, + data=json.dumps({"verbal_name": alert_receive_channel.verbal_name}), + content_type="application/json", + **make_user_auth_headers(user, token), + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + response = client.put( + url, + data=json.dumps({"verbal_name": "a new name"}), + content_type="application/json", + **make_user_auth_headers(user, token), + ) + assert response.status_code == status.HTTP_200_OK + + @pytest.mark.django_db def test_create_invalid_alert_receive_channel(alert_receive_channel_internal_api_setup, make_user_auth_headers): user, token, _ = alert_receive_channel_internal_api_setup diff --git a/engine/apps/public_api/serializers/integrations.py b/engine/apps/public_api/serializers/integrations.py index af612011..b2e42ec9 100644 --- a/engine/apps/public_api/serializers/integrations.py +++ b/engine/apps/public_api/serializers/integrations.py @@ -149,16 +149,24 @@ class IntegrationSerializer(EagerLoadingMixin, serializers.ModelSerializer, Main def validate(self, attrs): organization = self.context["request"].auth.organization verbal_name = attrs.get("verbal_name", None) + team = attrs.get("team", None) if verbal_name is None: return attrs try: - obj = AlertReceiveChannel.objects.get(organization=organization, verbal_name=verbal_name) + obj = AlertReceiveChannel.objects.get( + organization=organization, + team=team, + verbal_name=verbal_name, + ) except AlertReceiveChannel.DoesNotExist: return attrs + except AlertReceiveChannel.MultipleObjectsReturned: + raise BadRequest(detail="An integration with this name already exists for this team") + if self.instance and obj.id == self.instance.id: return attrs else: - raise BadRequest(detail="Integration with this name already exists") + raise BadRequest(detail="An integration with this name already exists for this team") def validate_templates(self, templates): if not isinstance(templates, dict): diff --git a/engine/apps/public_api/tests/test_integrations.py b/engine/apps/public_api/tests/test_integrations.py index 8f9e2473..4ff96c97 100644 --- a/engine/apps/public_api/tests/test_integrations.py +++ b/engine/apps/public_api/tests/test_integrations.py @@ -104,6 +104,73 @@ def test_create_integration( assert response.status_code == status.HTTP_201_CREATED +@pytest.mark.django_db +def test_integration_name_uniqueness( + make_organization_and_user_with_token, + make_team, +): + organization, _, token = make_organization_and_user_with_token() + + client = APIClient() + data = { + "type": "grafana", + "name": "grafana_created", + "team_id": None, + } + url = reverse("api-public:integrations-list") + response = client.post(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") + assert response.status_code == status.HTTP_201_CREATED + integration_pk = response.data["id"] + + # cannot create another one with the same name + response = client.post(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") + assert response.status_code == status.HTTP_400_BAD_REQUEST + + # but name can be reused in a different team + another_team = make_team(organization) + data["team_id"] = another_team.public_primary_key + response = client.post(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") + assert response.status_code == status.HTTP_201_CREATED + + # update works + url = reverse("api-public:integrations-detail", args=[integration_pk]) + data["team_id"] = None + data["description"] = "some description" + response = client.put(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") + assert response.status_code == status.HTTP_200_OK + + # but updating team will fail if name exists + data["team_id"] = another_team.public_primary_key + response = client.put(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") + assert response.status_code == status.HTTP_400_BAD_REQUEST + + +@pytest.mark.django_db +def test_integration_name_duplicated( + make_organization_and_user_with_token, + make_alert_receive_channel, + make_user_auth_headers, +): + # this could happen in case a team is removed and integrations are set to have "no team" + organization, _, token = make_organization_and_user_with_token() + integration = make_alert_receive_channel(organization) + # duplicated name integration + make_alert_receive_channel(organization, verbal_name=integration.verbal_name) + + client = APIClient() + + # updating team will require changing the name or the team + data = {"name": integration.verbal_name} + url = reverse("api-public:integrations-detail", args=[integration.public_primary_key]) + response = client.put(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") + assert response.status_code == status.HTTP_400_BAD_REQUEST + + # but updating team will fail if name exists + data["name"] = "a new name" + response = client.put(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") + assert response.status_code == status.HTTP_200_OK + + @pytest.mark.django_db def test_create_integrations_with_none_templates( make_organization_and_user_with_token,