Skip to content

Commit

Permalink
fix: don't update Slack messages if resolution note text not changed (#…
Browse files Browse the repository at this point in the history
…5126)

# What this PR does

Reduces the number of `chat.update` Slack API calls when a resolution
note is updated with the same text.

<!--
*Note*: If you want the issue to be auto-closed once the PR is merged,
change "Related to" to "Closes" in the line above.
If you have more than one GitHub issue that this PR closes, be sure to
preface
each issue link with a [closing
keyword](https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/using-keywords-in-issues-and-pull-requests#linking-a-pull-request-to-an-issue).
This ensures that the issue(s) are auto-closed once the PR has been
merged.
-->

## Checklist

- [x] Unit, integration, and e2e (if applicable) tests updated
- [x] Documentation added (or `pr:no public docs` PR label added if not
required)
- [x] Added the relevant release notes label (see labels prefixed w/
`release:`). These labels dictate how your PR will
    show up in the autogenerated release notes.
  • Loading branch information
vadimkerr authored Oct 4, 2024
1 parent c65a3c9 commit ac7dc97
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 17 deletions.
48 changes: 47 additions & 1 deletion engine/apps/public_api/tests/test_resolution_notes.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,14 @@ def test_get_resolution_note(
assert response.data == result


@patch("apps.alerts.tasks.send_update_resolution_note_signal.send_update_resolution_note_signal.apply_async")
@pytest.mark.django_db
def test_create_resolution_note(make_organization_and_user_with_token, make_alert_receive_channel, make_alert_group):
def test_create_resolution_note(
mock_send_update_resolution_note_signal,
make_organization_and_user_with_token,
make_alert_receive_channel,
make_alert_group,
):
organization, user, token = make_organization_and_user_with_token()
client = APIClient()

Expand Down Expand Up @@ -137,6 +143,8 @@ def test_create_resolution_note(make_organization_and_user_with_token, make_aler
assert response.status_code == status.HTTP_201_CREATED
assert response.data == result

mock_send_update_resolution_note_signal.assert_called_once()


@pytest.mark.django_db
def test_create_resolution_note_invalid_text(
Expand All @@ -163,8 +171,10 @@ def test_create_resolution_note_invalid_text(
assert response.data["text"][0] == "This field may not be blank."


@patch("apps.alerts.tasks.send_update_resolution_note_signal.send_update_resolution_note_signal.apply_async")
@pytest.mark.django_db
def test_update_resolution_note(
mock_send_update_resolution_note_signal,
make_organization_and_user_with_token,
make_alert_receive_channel,
make_alert_group,
Expand Down Expand Up @@ -206,6 +216,39 @@ def test_update_resolution_note(
assert resolution_note.text == result["text"]
assert response.data == result

mock_send_update_resolution_note_signal.assert_called_once()


@patch("apps.alerts.tasks.send_update_resolution_note_signal.send_update_resolution_note_signal.apply_async")
@pytest.mark.django_db
def test_update_resolution_note_same_text(
mock_send_update_resolution_note_signal,
make_organization_and_user_with_token,
make_alert_receive_channel,
make_alert_group,
make_resolution_note,
):
organization, user, token = make_organization_and_user_with_token()
client = APIClient()

alert_receive_channel = make_alert_receive_channel(organization)
alert_group = make_alert_group(alert_receive_channel)

resolution_note = make_resolution_note(
alert_group=alert_group,
source=ResolutionNote.Source.WEB,
author=user,
)

url = reverse("api-public:resolution_notes-detail", kwargs={"pk": resolution_note.public_primary_key})
response = client.put(
url, data={"text": resolution_note.message_text}, format="json", HTTP_AUTHORIZATION=f"{token}"
)
assert response.status_code == status.HTTP_200_OK

# update signal shouldn't be sent when text doesn't change
mock_send_update_resolution_note_signal.assert_not_called()


@pytest.mark.django_db
def test_update_resolution_note_invalid_source(
Expand Down Expand Up @@ -242,8 +285,10 @@ def test_update_resolution_note_invalid_source(
assert response.data["detail"] == "Cannot update message with this source type"


@patch("apps.alerts.tasks.send_update_resolution_note_signal.send_update_resolution_note_signal.apply_async")
@pytest.mark.django_db
def test_delete_resolution_note(
mock_send_update_resolution_note_signal,
make_organization_and_user_with_token,
make_alert_receive_channel,
make_alert_group,
Expand Down Expand Up @@ -272,6 +317,7 @@ def test_delete_resolution_note(
resolution_note.refresh_from_db()

assert resolution_note.deleted_at is not None
mock_send_update_resolution_note_signal.assert_called_once()

response = client.get(url, format="json", HTTP_AUTHORIZATION=f"{token}")

Expand Down
28 changes: 12 additions & 16 deletions engine/apps/public_api/views/resolution_notes.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,20 +56,16 @@ def get_object(self):
except ResolutionNote.DoesNotExist:
raise NotFound

def dispatch(self, request, *args, **kwargs):
result = super().dispatch(request, *args, **kwargs)
def perform_create(self, serializer):
super().perform_create(serializer)
send_update_resolution_note_signal.apply_async((serializer.instance.alert_group.pk, serializer.instance.pk))

# send signal to update alert group and resolution_note
method = request.method.lower()
if method in ["post", "put", "patch", "delete"]:
instance_id = self.kwargs.get("pk") or result.data.get("id")
if instance_id:
instance = ResolutionNote.objects_with_deleted.filter(public_primary_key=instance_id).first()
if instance is not None:
send_update_resolution_note_signal.apply_async(
kwargs={
"alert_group_pk": instance.alert_group.pk,
"resolution_note_pk": instance.pk,
}
)
return result
def perform_update(self, serializer):
is_text_updated = serializer.instance.message_text != serializer.validated_data["message_text"]
super().perform_update(serializer)
if is_text_updated:
send_update_resolution_note_signal.apply_async((serializer.instance.alert_group.pk, serializer.instance.pk))

def perform_destroy(self, instance):
super().perform_destroy(instance)
send_update_resolution_note_signal.apply_async((instance.alert_group.pk, instance.pk))

0 comments on commit ac7dc97

Please sign in to comment.