centralcloud/oncall-engine
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix IntegrityErrors occuring when creating ResolutionNoteSlackMessage objects (#2933)

Browse source 
# What this PR does

## Which issue(s) this PR fixes

Closes https://github.com/grafana/oncall-private/issues/1822

## 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] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required)
This commit is contained in:
Joey Orlando 2023-09-01 10:14:59 +02:00 • committed by GitHub
parent 73da83274a
commit 1d089aa7a3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 491 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
CHANGELOG.md
View file

@ -23,10 +23,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
- Fix issue with helm chart when specifying `broker.type=rabbitmq` where Redis environment variables
were not longer being injected @joeyorlando ([#2927](https://github.com/grafana/oncall/pull/2927))
were not longer being injected by @joeyorlando ([#2927](https://github.com/grafana/oncall/pull/2927))
- Fix silence for alert groups with empty escalation chain @Ferril ([#2929](https://github.com/grafana/oncall/pull/2929))
- Fixed NPE when migrating legacy Grafana Alerting integrations
([#2908](https://github.com/grafana/oncall/issues/2908))
- Fix `IntegrityError` exceptions that occasionally would occur when trying to create `ResolutionNoteSlackMessage`
objects by @joeyorlando ([#2933](https://github.com/grafana/oncall/pull/2933))
## v1.3.29 (2023-08-29)

21
engine/apps/alerts/migrations/0031_auto_20230831_1445.py Normal file
View file

@ -0,0 +1,21 @@
# Generated by Django 3.2.20 on 2023-08-31 14:45
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('alerts', '0030_auto_20230731_0341'),
]
operations = [
migrations.AddIndex(
model_name='resolutionnoteslackmessage',
index=models.Index(fields=['ts', 'thread_ts', 'alert_group_id'], name='alerts_reso_ts_08f72c_idx'),
),
migrations.AddIndex(
model_name='resolutionnoteslackmessage',
index=models.Index(fields=['ts', 'thread_ts', 'slack_channel_id'], name='alerts_reso_ts_a9bdf7_idx'),
),
]

5
engine/apps/alerts/models/resolution_note.py
View file

@ -83,6 +83,11 @@ class ResolutionNoteSlackMessage(models.Model):
class Meta:
unique_together = ("thread_ts", "ts")
indexes = [
models.Index(fields=["ts", "thread_ts", "alert_group_id"]),
models.Index(fields=["ts", "thread_ts", "slack_channel_id"]),
]
def get_resolution_note(self) -> typing.Optional["ResolutionNote"]:
try:
return self.resolution_note

61
engine/apps/slack/scenarios/slack_channel_integration.py
View file

@ -82,48 +82,33 @@ class SlackChannelMessageEventStep(scenario_step.ScenarioStep):
if result["permalink"] is not None:
permalink = result["permalink"]
try:
slack_thread_message = ResolutionNoteSlackMessage.objects.get(
ts=message_ts,
thread_ts=thread_ts,
alert_group=alert_group,
if len(text) > 2900:
self._slack_client.api_call(
"chat.postEphemeral",
channel=channel,
user=slack_user_identity.slack_id,
text=":warning: Unable to show the <{}|message> in Resolution Note: the message is too long ({}). "
"Max length - 2900 symbols.".format(permalink, len(text)),
)
if len(text) > 2900:
if slack_thread_message.added_to_resolution_note:
self._slack_client.api_call(
"chat.postEphemeral",
channel=channel,
user=slack_user_identity.slack_id,
text=":warning: Unable to update the <{}|message> in Resolution Note: the message is too long ({}). "
"Max length - 2900 symbols.".format(permalink, len(text)),
)
return
return
slack_thread_message, created = ResolutionNoteSlackMessage.objects.get_or_create(
ts=message_ts,
thread_ts=thread_ts,
alert_group=alert_group,
defaults={
"user": self.user,
"added_by_user": self.user,
"text": text,
"slack_channel_id": channel,
"permalink": permalink,
},
)
if not created:
slack_thread_message.text = text
slack_thread_message.save()
except ResolutionNoteSlackMessage.DoesNotExist:
if len(text) > 2900:
self._slack_client.api_call(
"chat.postEphemeral",
channel=channel,
user=slack_user_identity.slack_id,
text=":warning: The <{}|message> will not be displayed in Resolution Note: "
"the message is too long ({}). Max length - 2900 symbols.".format(permalink, len(text)),
)
return
slack_thread_message = ResolutionNoteSlackMessage(
alert_group=alert_group,
user=self.user,
added_by_user=self.user,
text=text,
slack_channel_id=channel,
thread_ts=thread_ts,
ts=message_ts,
permalink=permalink,
)
slack_thread_message.save()
def delete_thread_message_from_resolution_note(
self, slack_user_identity: "SlackUserIdentity", payload: EventPayload
) -> None:

439
engine/apps/slack/tests/test_scenario_steps/test_slack_channel_integration.py Normal file
View file

@ -0,0 +1,439 @@
from unittest.mock import Mock, call, patch
import pytest
from apps.alerts.models import ResolutionNoteSlackMessage
from apps.slack.scenarios.slack_channel_integration import SlackChannelMessageEventStep
@pytest.mark.django_db
class TestSlackChannelMessageEventStep:
@patch.object(SlackChannelMessageEventStep, "save_thread_message_for_resolution_note")
@patch.object(SlackChannelMessageEventStep, "delete_thread_message_from_resolution_note")
@pytest.mark.parametrize(
"payload,save_called,delete_called",
[
(
{
# does not have thread_ts attribute or subtype
"event": {},
},
False,
False,
),
(
{
# has thread_ts attribute but has subtype attribute that is not MESSAGE_CHANGED
"event": {
"thread_ts": "foo",
"subtype": "bar",
},
},
False,
False,
),
(
{
# has thread_ts attribute but not subtype attribute
"event": {
"thread_ts": "foo",
},
},
True,
False,
),
# MESSAGE_CHANGED event.subtype scenarios
(
{
"event": {
"subtype": "message_changed",
"message": {
"subtype": "bar",
"thread_ts": "hello",
},
},
},
False,
False,
),
(
{
"event": {
"subtype": "message_changed",
"message": {
"subtype": "bar",
},
},
},
False,
False,
),
(
{
"event": {
"subtype": "potato",
"message": {
"thread_ts": "bar",
},
},
},
False,
False,
),
(
{
"event": {
"subtype": "message_changed",
"message": {
"thread_ts": "bar",
},
},
},
True,
False,
),
(
{
"event": {
"subtype": "message_deleted",
"previous_message": {},
},
},
False,
False,
),
(
{
"event": {
"subtype": "message_deleted",
"previous_message": {},
},
},
False,
False,
),
(
{
"event": {
"subtype": "potato",
"previous_message": {
"thread_ts": "bar",
},
},
},
False,
False,
),
(
{
"event": {
"subtype": "message_deleted",
"previous_message": {
"thread_ts": "bar",
},
},
},
False,
True,
),
],
)
def test_process_scenario(
self,
mock_delete_thread_message_from_resolution_note,
mock_save_thread_message_for_resolution_note,
make_organization_and_user_with_slack_identities,
payload,
save_called,
delete_called,
) -> None:
(
organization,
user,
slack_team_identity,
slack_user_identity,
) = make_organization_and_user_with_slack_identities()
step = SlackChannelMessageEventStep(slack_team_identity, organization, user)
step.process_scenario(slack_user_identity, slack_team_identity, payload)
if save_called:
mock_save_thread_message_for_resolution_note.assert_called_once_with(slack_user_identity, payload)
else:
mock_save_thread_message_for_resolution_note.assert_not_called()
if delete_called:
mock_delete_thread_message_from_resolution_note.assert_called_once_with(slack_user_identity, payload)
else:
mock_delete_thread_message_from_resolution_note.assert_not_called()
@patch("apps.alerts.models.ResolutionNoteSlackMessage")
def test_save_thread_message_for_resolution_note_no_slack_user_identity(
self, MockResolutionNoteSlackMessage, make_organization_and_user_with_slack_identities
) -> None:
organization, user, slack_team_identity, _ = make_organization_and_user_with_slack_identities()
step = SlackChannelMessageEventStep(slack_team_identity, organization, user)
step._slack_client = Mock()
step.save_thread_message_for_resolution_note(None, {})
step._slack_client.api_call.assert_not_called()
MockResolutionNoteSlackMessage.objects.get_or_create.assert_not_called()
@patch("apps.alerts.models.ResolutionNoteSlackMessage")
def test_save_thread_message_for_resolution_note_no_slack_message(
self, MockResolutionNoteSlackMessage, make_organization_and_user_with_slack_identities
) -> None:
(
organization,
user,
slack_team_identity,
slack_user_identity,
) = make_organization_and_user_with_slack_identities()
step = SlackChannelMessageEventStep(slack_team_identity, organization, user)
step._slack_client = Mock()
payload = {
"event": {
"channel": "potato",
"ts": 88945.4849,
"thread_ts": 16789.123,
"text": "hello",
},
}
step.save_thread_message_for_resolution_note(slack_user_identity, payload)
step._slack_client.api_call.assert_not_called()
MockResolutionNoteSlackMessage.objects.get_or_create.assert_not_called()
@patch("apps.alerts.models.ResolutionNoteSlackMessage")
def test_save_thread_message_for_resolution_note_really_long_text(
self,
MockResolutionNoteSlackMessage,
make_organization_and_user_with_slack_identities,
make_alert_receive_channel,
make_alert_group,
make_slack_message,
) -> None:
(
organization,
user,
slack_team_identity,
slack_user_identity,
) = make_organization_and_user_with_slack_identities()
integration = make_alert_receive_channel(organization)
alert_group = make_alert_group(integration)
channel = "potato"
ts = 88945.4849
thread_ts = 16789.123
make_slack_message(alert_group, slack_id=thread_ts, channel_id=channel)
mock_permalink = "http://example.com"
step = SlackChannelMessageEventStep(slack_team_identity, organization, user)
step._slack_client = Mock()
step._slack_client.api_call.side_effect = [{"permalink": mock_permalink}, None]
payload = {
"event": {
"channel": channel,
"ts": ts,
"thread_ts": thread_ts,
"text": "h" * 2901,
},
}
step.save_thread_message_for_resolution_note(slack_user_identity, payload)
step._slack_client.api_call.assert_has_calls(
[
call(
"chat.getPermalink",
channel=payload["event"]["channel"],
message_ts=payload["event"]["ts"],
),
call(
"chat.postEphemeral",
channel=payload["event"]["channel"],
user=slack_user_identity.slack_id,
text=":warning: Unable to show the <{}|message> in Resolution Note: the message is too long ({}). "
"Max length - 2900 symbols.".format(mock_permalink, len(payload["event"]["text"])),
),
]
)
MockResolutionNoteSlackMessage.objects.get_or_create.assert_not_called()
@pytest.mark.parametrize("resolution_note_slack_message_already_exists", [True, False])
def test_save_thread_message_for_resolution_note(
self,
make_organization_and_user_with_slack_identities,
make_alert_receive_channel,
make_alert_group,
make_slack_message,
make_resolution_note_slack_message,
resolution_note_slack_message_already_exists,
) -> None:
(
organization,
user,
slack_team_identity,
slack_user_identity,
) = make_organization_and_user_with_slack_identities()
integration = make_alert_receive_channel(organization)
alert_group = make_alert_group(integration)
original_text = "original text"
new_text = "new text"
channel = "potato"
ts = 88945.4849
thread_ts = 16789.123
make_slack_message(alert_group, slack_id=thread_ts, channel_id=channel)
resolution_note_slack_message = None
if resolution_note_slack_message_already_exists:
resolution_note_slack_message = make_resolution_note_slack_message(
alert_group, user, user, ts=ts, thread_ts=thread_ts, text=original_text
)
mock_permalink = "http://example.com"
step = SlackChannelMessageEventStep(slack_team_identity, organization, user)
step._slack_client = Mock()
step._slack_client.api_call.side_effect = [{"permalink": mock_permalink}, None]
payload = {
"event": {
"channel": channel,
"ts": ts,
"thread_ts": thread_ts,
"text": new_text,
},
}
step.save_thread_message_for_resolution_note(slack_user_identity, payload)
step._slack_client.api_call.assert_has_calls(
[
call(
"chat.getPermalink",
channel=payload["event"]["channel"],
message_ts=payload["event"]["ts"],
),
]
)
if resolution_note_slack_message_already_exists:
resolution_note_slack_message.refresh_from_db()
resolution_note_slack_message.text = new_text
else:
assert (
ResolutionNoteSlackMessage.objects.filter(
ts=ts,
thread_ts=thread_ts,
alert_group=alert_group,
).count()
== 1
)
@patch("apps.alerts.models.ResolutionNoteSlackMessage")
def test_delete_thread_message_from_resolution_note_no_slack_user_identity(
self, MockResolutionNoteSlackMessage, make_organization_and_user_with_slack_identities
) -> None:
(
organization,
user,
slack_team_identity,
slack_user_identity,
) = make_organization_and_user_with_slack_identities()
step = SlackChannelMessageEventStep(slack_team_identity, organization, user)
step.delete_thread_message_from_resolution_note(None, {})
MockResolutionNoteSlackMessage.objects.get.assert_not_called()
def test_delete_thread_message_from_resolution_note_no_message_found(
self, make_organization_and_user_with_slack_identities
) -> None:
(
organization,
user,
slack_team_identity,
slack_user_identity,
) = make_organization_and_user_with_slack_identities()
channel = "potato"
ts = 88945.4849
thread_ts = 16789.123
payload = {
"event": {
"channel": channel,
"previous_message": {
"ts": ts,
"thread_ts": thread_ts,
},
},
}
step = SlackChannelMessageEventStep(slack_team_identity, organization, user)
step.alert_group_slack_service = Mock()
step.delete_thread_message_from_resolution_note(slack_user_identity, payload)
step.alert_group_slack_service.assert_not_called()
def test_delete_thread_message_from_resolution_note(
self,
make_organization_and_user_with_slack_identities,
make_alert_receive_channel,
make_alert_group,
make_resolution_note_slack_message,
) -> None:
(
organization,
user,
slack_team_identity,
slack_user_identity,
) = make_organization_and_user_with_slack_identities()
integration = make_alert_receive_channel(organization)
alert_group = make_alert_group(integration)
channel = "potato"
ts = 88945.4849
thread_ts = 16789.123
payload = {
"event": {
"channel": channel,
"previous_message": {
"ts": ts,
"thread_ts": thread_ts,
},
},
}
make_resolution_note_slack_message(
alert_group, user, user, ts=ts, thread_ts=thread_ts, slack_channel_id=channel
)
step = SlackChannelMessageEventStep(slack_team_identity, organization, user)
step.alert_group_slack_service = Mock()
step.delete_thread_message_from_resolution_note(slack_user_identity, payload)
step.alert_group_slack_service.update_alert_group_slack_message.assert_called_once_with(alert_group)
assert (
ResolutionNoteSlackMessage.objects.filter(
ts=ts,
thread_ts=thread_ts,
slack_channel_id=channel,
).count()
== 0
)