Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve APIs for creating/updating direct paging integrations #2603

Merged
merged 9 commits into from
Jul 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed

- Update direct paging docs by @vadimkerr ([#2600](https://github.com/grafana/oncall/pull/2600))
- Improve APIs for creating/updating direct paging integrations by @vadimkerr ([#2603](https://github.com/grafana/oncall/pull/2603))

### Fixed

Expand Down
19 changes: 19 additions & 0 deletions engine/apps/alerts/models/alert_receive_channel.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,25 @@ def delete(self):
def hard_delete(self):
super(AlertReceiveChannel, self).delete()

class DuplicateDirectPagingError(Exception):
"""Only one Direct Paging integration is allowed per team."""

DETAIL = "Direct paging integration already exists for this team" # Returned in BadRequest responses

def save(self, *args, **kwargs):
# Don't allow multiple Direct Paging integrations per team
if (
self.integration == AlertReceiveChannel.INTEGRATION_DIRECT_PAGING
and AlertReceiveChannel.objects.filter(
organization=self.organization, team=self.team, integration=self.integration
)
.exclude(pk=self.pk)
.exists()
):
raise self.DuplicateDirectPagingError

return super().save(*args, **kwargs)

def change_team(self, team_id, user):
if team_id == self.team_id:
raise TeamCanNotBeChangedError("Integration is already in this team")
Expand Down
27 changes: 21 additions & 6 deletions engine/apps/api/serializers/alert_receive_channel.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +121,24 @@ def create(self, validated_data):
if _integration.slug == integration:
is_able_to_autoresolve = _integration.is_able_to_autoresolve

instance = AlertReceiveChannel.create(
**validated_data,
organization=organization,
author=self.context["request"].user,
allow_source_based_resolving=is_able_to_autoresolve,
)
try:
instance = AlertReceiveChannel.create(
**validated_data,
organization=organization,
author=self.context["request"].user,
allow_source_based_resolving=is_able_to_autoresolve,
)
except AlertReceiveChannel.DuplicateDirectPagingError:
raise BadRequest(detail=AlertReceiveChannel.DuplicateDirectPagingError.DETAIL)

return instance

def update(self, *args, **kwargs):
try:
return super().update(*args, **kwargs)
except AlertReceiveChannel.DuplicateDirectPagingError:
raise BadRequest(detail=AlertReceiveChannel.DuplicateDirectPagingError.DETAIL)

def get_instructions(self, obj):
if obj.integration in [AlertReceiveChannel.INTEGRATION_MAINTENANCE]:
return ""
Expand All @@ -146,6 +155,12 @@ def get_default_channel_filter(self, obj):
if filter.is_default:
return filter.public_primary_key

@staticmethod
def validate_integration(integration):
if integration is None or integration not in AlertReceiveChannel.WEB_INTEGRATION_CHOICES:
raise BadRequest(detail="invalid integration")
return integration

Comment on lines +158 to +163
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplified this bit, added a test

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):
Expand Down
89 changes: 89 additions & 0 deletions engine/apps/api/tests/test_alert_receive_channel.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,27 @@ def test_create_invalid_alert_receive_channel(alert_receive_channel_internal_api
assert response.status_code == status.HTTP_400_BAD_REQUEST


@pytest.mark.django_db
def test_create_invalid_alert_receive_channel_type(alert_receive_channel_internal_api_setup, make_user_auth_headers):
user, token, _ = alert_receive_channel_internal_api_setup

client = APIClient()
url = reverse("api-internal:alert_receive_channel-list")

response_1 = client.post(
url,
data={"integration": "random123", "verbal_name": "Random 123"},
format="json",
**make_user_auth_headers(user, token),
)
response_2 = client.post(
url, data={"verbal_name": "Random 123"}, format="json", **make_user_auth_headers(user, token)
)

assert response_1.status_code == status.HTTP_400_BAD_REQUEST
assert response_2.status_code == status.HTTP_400_BAD_REQUEST


@pytest.mark.django_db
def test_update_alert_receive_channel(alert_receive_channel_internal_api_setup, make_user_auth_headers):
user, token, alert_receive_channel = alert_receive_channel_internal_api_setup
Expand Down Expand Up @@ -693,6 +714,74 @@ def test_get_alert_receive_channels_direct_paging_present_for_filters(
assert response.json()["results"][0]["value"] == alert_receive_channel.public_primary_key


@pytest.mark.django_db
def test_create_alert_receive_channels_direct_paging(
make_organization_and_user_with_plugin_token, make_team, make_alert_receive_channel, make_user_auth_headers
):
organization, user, token = make_organization_and_user_with_plugin_token()
team = make_team(organization)

client = APIClient()
url = reverse("api-internal:alert_receive_channel-list")

response_1 = client.post(
url, data={"integration": "direct_paging"}, format="json", **make_user_auth_headers(user, token)
)
response_2 = client.post(
url, data={"integration": "direct_paging"}, format="json", **make_user_auth_headers(user, token)
)

response_3 = client.post(
url,
data={"integration": "direct_paging", "team": team.public_primary_key},
format="json",
**make_user_auth_headers(user, token),
)
response_4 = client.post(
url,
data={"integration": "direct_paging", "team": team.public_primary_key},
format="json",
**make_user_auth_headers(user, token),
)

# Check direct paging integration for "No team" is created
assert response_1.status_code == status.HTTP_201_CREATED
# Check direct paging integration is not created, as it already exists for "No team"
assert response_2.status_code == status.HTTP_400_BAD_REQUEST

# Check direct paging integration for team is created
assert response_3.status_code == status.HTTP_201_CREATED
# Check direct paging integration is not created, as it already exists for team
assert response_4.status_code == status.HTTP_400_BAD_REQUEST
assert response_4.json()["detail"] == AlertReceiveChannel.DuplicateDirectPagingError.DETAIL


@pytest.mark.django_db
def test_update_alert_receive_channels_direct_paging(
make_organization_and_user_with_plugin_token, make_team, make_alert_receive_channel, make_user_auth_headers
):
organization, user, token = make_organization_and_user_with_plugin_token()
team = make_team(organization)
integration = make_alert_receive_channel(
organization, integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING, team=None
)
make_alert_receive_channel(organization, integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING, team=team)

client = APIClient()
url = reverse("api-internal:alert_receive_channel-detail", kwargs={"pk": integration.public_primary_key})

# Move direct paging integration from "No team" to team
response = client.put(
url,
data={"integration": "direct_paging", "team": team.public_primary_key},
format="json",
**make_user_auth_headers(user, token),
)

assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.json()["detail"] == AlertReceiveChannel.DuplicateDirectPagingError.DETAIL


@pytest.mark.django_db
def test_start_maintenance_integration(
make_user_auth_headers,
Expand Down
37 changes: 0 additions & 37 deletions engine/apps/api/views/alert_receive_channel.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
)
from apps.api.throttlers import DemoAlertThrottler
from apps.auth_token.auth import PluginAuthentication
from apps.user_management.models.team import Team
from common.api_helpers.exceptions import BadRequest
from common.api_helpers.filters import ByTeamModelFieldFilterMixin, TeamModelMultipleChoiceFilter
from common.api_helpers.mixins import (
Expand Down Expand Up @@ -104,42 +103,6 @@ class AlertReceiveChannelView(
"stop_maintenance": [RBACPermission.Permissions.INTEGRATIONS_WRITE],
}

def create(self, request, *args, **kwargs):
organization = request.auth.organization
user = request.user
team_lookup = {}
if "team" in request.data:
team_public_pk = request.data.get("team", None)
if team_public_pk is not None:
try:
team = user.available_teams.get(public_primary_key=team_public_pk)
team_lookup = {"team": team}
except Team.DoesNotExist:
return Response(data="invalid team", status=status.HTTP_400_BAD_REQUEST)
else:
team_lookup = {"team__isnull": True}
Comment on lines -110 to -120
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed this team check completely, as it seems to be excessive – only admins can create integrations, and every team is available for admins.


if request.data["integration"] is not None:
if request.data["integration"] in AlertReceiveChannel.WEB_INTEGRATION_CHOICES:
# Don't allow multiple Direct Paging integrations
if request.data["integration"] == AlertReceiveChannel.INTEGRATION_DIRECT_PAGING:
try:
AlertReceiveChannel.objects.get(
organization=organization,
integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING,
deleted_at=None,
**team_lookup,
)
return Response(
data="Direct paging integration already exists for this team",
status=status.HTTP_400_BAD_REQUEST,
)
except AlertReceiveChannel.DoesNotExist:
pass
Comment on lines -125 to -138
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return super().create(request, *args, **kwargs)

return Response(data="invalid integration", status=status.HTTP_400_BAD_REQUEST)

def perform_update(self, serializer):
prev_state = serializer.instance.insight_logs_serialized
serializer.save()
Expand Down
19 changes: 14 additions & 5 deletions engine/apps/public_api/serializers/integrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,14 @@ def create(self, validated_data):
if connection_error:
raise serializers.ValidationError(connection_error)
with transaction.atomic():
instance = AlertReceiveChannel.create(
**validated_data,
author=self.context["request"].user,
organization=organization,
)
try:
instance = AlertReceiveChannel.create(
**validated_data,
author=self.context["request"].user,
organization=organization,
)
except AlertReceiveChannel.DuplicateDirectPagingError:
raise BadRequest(detail=AlertReceiveChannel.DuplicateDirectPagingError.DETAIL)
if default_route_data:
serializer = DefaultChannelFilterSerializer(
instance.default_channel_filter, default_route_data, context=self.context
Expand All @@ -138,6 +141,12 @@ def create(self, validated_data):
serializer.save()
return instance

def update(self, *args, **kwargs):
try:
return super().update(*args, **kwargs)
except AlertReceiveChannel.DuplicateDirectPagingError:
raise BadRequest(detail=AlertReceiveChannel.DuplicateDirectPagingError.DETAIL)

def validate(self, attrs):
organization = self.context["request"].auth.organization
verbal_name = attrs.get("verbal_name", None)
Expand Down
54 changes: 54 additions & 0 deletions engine/apps/public_api/tests/test_integrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -817,3 +817,57 @@ def test_update_integration_default_route(

assert response.status_code == status.HTTP_200_OK
assert response.data["default_route"]["escalation_chain_id"] == escalation_chain.public_primary_key


@pytest.mark.django_db
def test_create_integrations_direct_paging(
make_organization_and_user_with_token, make_team, make_alert_receive_channel, make_user_auth_headers
):
organization, _, token = make_organization_and_user_with_token()
team = make_team(organization)

client = APIClient()
url = reverse("api-public:integrations-list")

response_1 = client.post(url, data={"type": "direct_paging"}, format="json", HTTP_AUTHORIZATION=token)
response_2 = client.post(url, data={"type": "direct_paging"}, format="json", HTTP_AUTHORIZATION=token)

response_3 = client.post(
url, data={"type": "direct_paging", "team_id": team.public_primary_key}, format="json", HTTP_AUTHORIZATION=token
)
response_4 = client.post(
url, data={"type": "direct_paging", "team_id": team.public_primary_key}, format="json", HTTP_AUTHORIZATION=token
)

# Check direct paging integration for "No team" is created
assert response_1.status_code == status.HTTP_201_CREATED
# Check direct paging integration is not created, as it already exists for "No team"
assert response_2.status_code == status.HTTP_400_BAD_REQUEST

# Check direct paging integration for team is created
assert response_3.status_code == status.HTTP_201_CREATED
# Check direct paging integration is not created, as it already exists for team
assert response_4.status_code == status.HTTP_400_BAD_REQUEST
assert response_4.data["detail"] == AlertReceiveChannel.DuplicateDirectPagingError.DETAIL


@pytest.mark.django_db
def test_update_integrations_direct_paging(
make_organization_and_user_with_token, make_team, make_alert_receive_channel, make_user_auth_headers
):
organization, _, token = make_organization_and_user_with_token()
team = make_team(organization)

integration = make_alert_receive_channel(
organization, integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING, team=None
)
make_alert_receive_channel(organization, integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING, team=team)

client = APIClient()
url = reverse("api-public:integrations-detail", args=[integration.public_primary_key])

# Move direct paging integration from "No team" to team
response = client.put(url, data={"team_id": team.public_primary_key}, format="json", HTTP_AUTHORIZATION=token)

assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.data["detail"] == AlertReceiveChannel.DuplicateDirectPagingError.DETAIL