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

Conversation

vstpme
Copy link
Member

@vstpme vstpme commented Jul 20, 2023

What this PR does

  • Disallow creating direct paging integrations for a team that already has one + make sure this constraint is also enforced on update
  • Refactor some direct paging API parts to be more reusable & simple
  • Add tests

Checklist

  • Unit, integration, and e2e (if applicable) tests updated
  • Documentation added (or pr:no public docs PR label added if not required)
  • CHANGELOG.md updated (or pr:no changelog PR label added if not required)

Comment on lines +158 to +163
@staticmethod
def validate_integration(integration):
if integration is None or integration not in AlertReceiveChannel.WEB_INTEGRATION_CHOICES:
raise BadRequest(detail="invalid integration")
return integration

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

Comment on lines -110 to -120
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}
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.

Comment on lines -125 to -138
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
Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines 128 to 135
try:
instance = AlertReceiveChannel.create(
**validated_data,
author=self.context["request"].user,
organization=organization,
)
except AlertReceiveChannel.DuplicateDirectPaging:
raise DuplicateDirectPagingBadRequest
Copy link
Member Author

Choose a reason for hiding this comment

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

Making sure that direct paging integrations are unique per team in public API.

@vstpme vstpme added the pr:no public docs Added to a PR that does not require public documentation updates label Jul 20, 2023
@vstpme vstpme changed the title Improve APIs for direct paging Improve APIs for creating/updating direct paging integrations Jul 20, 2023
@vstpme vstpme marked this pull request as ready for review July 20, 2023 18:58
@vstpme vstpme requested a review from a team July 20, 2023 18:58
Copy link
Contributor

@matiasb matiasb left a comment

Choose a reason for hiding this comment

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

LGTM

engine/apps/alerts/models/alert_receive_channel.py Outdated Show resolved Hide resolved
engine/apps/api/serializers/alert_receive_channel.py Outdated Show resolved Hide resolved
@vstpme vstpme added this pull request to the merge queue Jul 21, 2023
Merged via the queue into dev with commit 29d532e Jul 21, 2023
@vstpme vstpme deleted the vadimkerr/direct-paging-apis branch July 21, 2023 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:no public docs Added to a PR that does not require public documentation updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants