oncall-engine/engine/apps/slack/tests/scenario_steps/test_resolution_note.py

466 lines
17 KiB
Python
Raw Permalink Normal View History

import json
from unittest.mock import patch
import pytest
from apps.slack.chatops_proxy_routing import make_value
from apps.slack.client import SlackClient
from apps.slack.constants import BLOCK_SECTION_TEXT_MAX_SIZE
from apps.slack.errors import SlackAPIViewNotFoundError
from apps.slack.scenarios.scenario_step import ScenarioStep
from apps.slack.tests.conftest import build_slack_response
2022-07-12 15:42:20 -06:00
from common.api_helpers.utils import create_engine_url
@pytest.mark.django_db
def test_get_resolution_notes_blocks_default_if_empty(
make_organization_and_user_with_slack_identities,
make_alert_receive_channel,
make_alert_group,
):
SlackResolutionNoteModalStep = ScenarioStep.get_step("resolution_note", "ResolutionNoteModalStep")
organization, _, slack_team_identity, _ = make_organization_and_user_with_slack_identities()
step = SlackResolutionNoteModalStep(slack_team_identity)
alert_receive_channel = make_alert_receive_channel(organization)
alert_group = make_alert_group(alert_receive_channel)
blocks = step.get_resolution_notes_blocks(alert_group, "", False)
expected_blocks = [
{
"type": "image",
"title": {
"type": "plain_text",
"text": SlackResolutionNoteModalStep.MESSAGE_SHORTCUT_INSTRUCTION,
},
"image_url": create_engine_url("static/images/resolution_note.gif"),
"alt_text": SlackResolutionNoteModalStep.MESSAGE_SHORTCUT_INSTRUCTION,
},
]
assert blocks == expected_blocks
@pytest.mark.django_db
def test_get_resolution_notes_blocks_non_empty(
make_organization_and_user_with_slack_identities,
make_alert_receive_channel,
make_alert_group,
make_resolution_note_slack_message,
):
SlackResolutionNoteModalStep = ScenarioStep.get_step("resolution_note", "ResolutionNoteModalStep")
organization, user, slack_team_identity, _ = make_organization_and_user_with_slack_identities()
step = SlackResolutionNoteModalStep(slack_team_identity)
alert_receive_channel = make_alert_receive_channel(organization)
alert_group = make_alert_group(alert_receive_channel)
resolution_note = make_resolution_note_slack_message(alert_group=alert_group, user=user, added_by_user=user, ts=1)
blocks = step.get_resolution_notes_blocks(alert_group, "", False)
expected_blocks = [
{
"type": "divider",
},
{
"type": "section",
"text": {
"type": "mrkdwn",
"text": "{} <!date^{:.0f}^{{date_num}} {{time_secs}}|message_created_at>\n{}".format(
resolution_note.user.get_username_with_slack_verbal(mention=True),
float(resolution_note.ts),
resolution_note.text,
),
},
"accessory": {
"type": "button",
"style": "primary",
"text": {
"type": "plain_text",
"text": "Add",
"emoji": True,
},
"action_id": "AddRemoveThreadMessageStep",
"value": make_value(
{
"resolution_note_window_action": "edit",
"msg_value": "add",
"message_pk": resolution_note.pk,
"resolution_note_pk": None,
"alert_group_pk": alert_group.pk,
},
organization,
),
},
},
]
assert blocks == expected_blocks
@pytest.mark.django_db
def test_get_resolution_note_blocks_truncate_text(
make_organization_and_user_with_slack_identities,
make_alert_receive_channel,
make_alert_group,
make_resolution_note,
):
UpdateResolutionNoteStep = ScenarioStep.get_step("resolution_note", "UpdateResolutionNoteStep")
organization, user, slack_team_identity, _ = make_organization_and_user_with_slack_identities()
step = UpdateResolutionNoteStep(slack_team_identity)
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, author=user, message_text="a" * 3000)
author_verbal = resolution_note.author_verbal(mention=False)
blocks = step.get_resolution_note_blocks(resolution_note)
expected_blocks = [
{
"type": "section",
"text": {
"type": "mrkdwn",
# text is truncated, ellipsis added
"text": resolution_note.text[: BLOCK_SECTION_TEXT_MAX_SIZE - 1] + "",
},
},
{
"type": "context",
"elements": [
{
"type": "mrkdwn",
"text": f"{author_verbal} resolution note from {resolution_note.get_source_display()}.",
}
],
},
]
assert blocks == expected_blocks
@pytest.mark.django_db
def test_post_or_update_resolution_note_in_thread_truncate_message_text(
make_organization_and_user_with_slack_identities,
make_alert_receive_channel,
make_alert_group,
make_slack_message,
make_slack_channel,
make_resolution_note,
):
organization, user, slack_team_identity, _ = make_organization_and_user_with_slack_identities()
alert_receive_channel = make_alert_receive_channel(organization)
alert_group = make_alert_group(alert_receive_channel)
slack_channel = make_slack_channel(slack_team_identity)
chore: drop usage of `SlackMessage.organization` + drop orphaned `SlackMessage`s (#5330) # What this PR does - Stops writing `SlackMessage.organization` + removes references to this field. [As we discussed](https://raintank-corp.slack.com/archives/C083TU81TCH/p1733315887463279?thread_ts=1733311105.095309&cid=C083TU81TCH), we do not need this field on this model/table, `SlackMessage._slack_team_identity` is sufficient (`organization` will be dropped in a separate PR) - Adds a data migration script which: - drops orphaned `SlackMessage` records; ie. ones which, even after the [`engine/apps/slack/migrations/0007_migrate_slackmessage_channel_id.py`](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/migrations/0007_migrate_slackmessage_channel_id.py) migration, still don't have a `SlackMessage.channel` id filled in (we discussed + agreed on dropping these records [here](https://raintank-corp.slack.com/archives/C083TU81TCH/p1733329914516859?thread_ts=1733311105.095309&cid=C083TU81TCH)) - fills in empty `SlackMessage.slack_team_identity` values (from `slack_message.channel.slack_team_identity`) ### Other notes On the `organization` topic. We store records in `SlackMessage` for two purposes (AFAICT), and in both cases, we have references back to the `organization`: - alert groups - `slack_message.alert_group.channel.organization` - shift swap requests - `shift_swap_request.schedule.organization` ## 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.
2024-12-06 11:43:40 -05:00
make_slack_message(slack_channel, alert_group=alert_group)
resolution_note = make_resolution_note(alert_group=alert_group, author=user, message_text="a" * 3000)
UpdateResolutionNoteStep = ScenarioStep.get_step("resolution_note", "UpdateResolutionNoteStep")
step = UpdateResolutionNoteStep(slack_team_identity)
with patch("apps.slack.client.SlackClient.api_call") as mock_slack_api_call:
mock_slack_api_call.return_value = {
"ts": "timestamp",
"message": {"ts": "timestamp"},
"permalink": "https://link.to.message",
}
step.post_or_update_resolution_note_in_thread(resolution_note)
assert mock_slack_api_call.called
post_message_call = mock_slack_api_call.mock_calls[0]
assert post_message_call.args[0] == "chat.postMessage"
assert post_message_call.kwargs["json"]["text"] == resolution_note.text[: BLOCK_SECTION_TEXT_MAX_SIZE - 1] + ""
@pytest.mark.django_db
def test_post_or_update_resolution_note_in_thread_update_truncate_message_text(
make_organization_and_user_with_slack_identities,
make_alert_receive_channel,
make_alert_group,
make_slack_message,
make_slack_channel,
make_resolution_note,
make_resolution_note_slack_message,
):
organization, user, slack_team_identity, _ = make_organization_and_user_with_slack_identities()
alert_receive_channel = make_alert_receive_channel(organization)
alert_group = make_alert_group(alert_receive_channel)
slack_channel = make_slack_channel(slack_team_identity)
chore: drop usage of `SlackMessage.organization` + drop orphaned `SlackMessage`s (#5330) # What this PR does - Stops writing `SlackMessage.organization` + removes references to this field. [As we discussed](https://raintank-corp.slack.com/archives/C083TU81TCH/p1733315887463279?thread_ts=1733311105.095309&cid=C083TU81TCH), we do not need this field on this model/table, `SlackMessage._slack_team_identity` is sufficient (`organization` will be dropped in a separate PR) - Adds a data migration script which: - drops orphaned `SlackMessage` records; ie. ones which, even after the [`engine/apps/slack/migrations/0007_migrate_slackmessage_channel_id.py`](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/migrations/0007_migrate_slackmessage_channel_id.py) migration, still don't have a `SlackMessage.channel` id filled in (we discussed + agreed on dropping these records [here](https://raintank-corp.slack.com/archives/C083TU81TCH/p1733329914516859?thread_ts=1733311105.095309&cid=C083TU81TCH)) - fills in empty `SlackMessage.slack_team_identity` values (from `slack_message.channel.slack_team_identity`) ### Other notes On the `organization` topic. We store records in `SlackMessage` for two purposes (AFAICT), and in both cases, we have references back to the `organization`: - alert groups - `slack_message.alert_group.channel.organization` - shift swap requests - `shift_swap_request.schedule.organization` ## 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.
2024-12-06 11:43:40 -05:00
make_slack_message(slack_channel, alert_group=alert_group)
resolution_note = make_resolution_note(alert_group=alert_group, author=user, message_text="a" * 3000)
make_resolution_note_slack_message(
alert_group=alert_group,
resolution_note=resolution_note,
user=user,
posted_by_bot=True,
added_by_user=user,
ts=1,
text=resolution_note.text,
)
UpdateResolutionNoteStep = ScenarioStep.get_step("resolution_note", "UpdateResolutionNoteStep")
step = UpdateResolutionNoteStep(slack_team_identity)
with patch("apps.slack.client.SlackClient.api_call") as mock_slack_api_call:
mock_slack_api_call.return_value = {
"ts": "timestamp",
"message": {"ts": "timestamp"},
"permalink": "https://link.to.message",
}
step.post_or_update_resolution_note_in_thread(resolution_note)
assert mock_slack_api_call.called
post_message_call = mock_slack_api_call.mock_calls[0]
assert post_message_call.args[0] == "chat.update"
assert post_message_call.kwargs["json"]["text"] == resolution_note.text[: BLOCK_SECTION_TEXT_MAX_SIZE - 1] + ""
@pytest.mark.django_db
def test_get_resolution_notes_blocks_latest_limit(
make_organization_and_user_with_slack_identities,
make_alert_receive_channel,
make_alert_group,
make_resolution_note_slack_message,
):
SlackResolutionNoteModalStep = ScenarioStep.get_step("resolution_note", "ResolutionNoteModalStep")
organization, user, slack_team_identity, _ = make_organization_and_user_with_slack_identities()
step = SlackResolutionNoteModalStep(slack_team_identity)
alert_receive_channel = make_alert_receive_channel(organization)
alert_group = make_alert_group(alert_receive_channel)
max_count = SlackResolutionNoteModalStep.RESOLUTION_NOTE_MESSAGES_MAX_COUNT
messages = [
make_resolution_note_slack_message(alert_group=alert_group, user=user, added_by_user=user, ts=i, text=i)
for i in range(max_count * 2)
]
blocks = step.get_resolution_notes_blocks(alert_group, "", False)
expected_blocks = [
{
"type": "divider",
},
{
"type": "section",
"text": {
"type": "mrkdwn",
"text": (
":warning: Listing up to last {} thread messages, "
"you can still add any other message using contextual menu actions."
).format(max_count),
},
},
]
for m in list(reversed(messages))[:max_count]:
expected_blocks += [
{
"type": "divider",
},
{
"type": "section",
"text": {
"type": "mrkdwn",
"text": "{} <!date^{:.0f}^{{date_num}} {{time_secs}}|message_created_at>\n{}".format(
m.user.get_username_with_slack_verbal(mention=True),
float(m.ts),
m.text,
),
},
"accessory": {
"type": "button",
"style": "primary",
"text": {
"type": "plain_text",
"text": "Add",
"emoji": True,
},
"action_id": "AddRemoveThreadMessageStep",
"value": make_value(
{
"resolution_note_window_action": "edit",
"msg_value": "add",
"message_pk": m.pk,
"resolution_note_pk": None,
"alert_group_pk": alert_group.pk,
},
organization,
),
},
},
]
assert blocks == expected_blocks
@pytest.mark.django_db
@patch.object(
SlackClient,
"api_call",
side_effect=SlackAPIViewNotFoundError(response=build_slack_response({"ok": False, "error": "not_found"})),
)
def test_resolution_notes_modal_closed_before_update(
Fix orphaned messages in Slack (#2023) # What this PR does Reworks Slack handlers for buttons and select menus for AG Slack messages. <img width="602" alt="Screenshot 2023-05-31 at 19 34 05" src="https://github.com/grafana/oncall/assets/20116910/857bf096-7bdd-427b-94b6-15aad873a8ac"> ## Current implementation - It's possible to end up with orphaned Slack messages that are posted to Slack but have no `SlackMessage` instance in the DB. For such messages, clicking buttons will result in an exception and HTTP 500. See private repo [issue](https://github.com/grafana/oncall-private/issues/1841) for more info. - Bug in authorization system, which effectively bypasses any permission checks. For example, it's possible to resolve an alert group while being a Viewer. - No tests covering most buttons. ## Changes in this PR - Make the system more robust, don't use `SlackMessage` model to figure out the alert group being interacted on, instead embed `alert_group_pk` to every button and use it when receiving interaction requests from Slack. - Existing orphaned Slack messages will be repaired. Clicking buttons under orphaned messages will work (and missing `SlackMessage` instance will be created on interaction). This is possible because some buttons already have `alert_group_pk` embedded, and it's possible to get this data on button clicks (even if the clicked button itself doesn't have `alert_group_pk` embedded). - Fix authorization. Show warning window when unauthorized: <img width="511" alt="Screenshot 2023-05-31 at 19 40 02" src="https://github.com/grafana/oncall/assets/20116910/5abeeaa7-1b61-4a47-b3af-0e21d5cd1907"> - Added tests for all the buttons under AG message. Add tests checking authorization, actual execution of scenario steps, orphan message repairing, backward compatibility, etc. Also add tests on `AlertGroupSlackRenderer` checking that correct data is embedded into buttons. - Cosmetic changes such as renaming `incident` to `Alert Group`. ## Which issue(s) this PR fixes Related to https://github.com/grafana/oncall-private/issues/1841 ## 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)
2023-06-01 11:21:30 +01:00
mock_slack_api_call,
make_organization_and_user_with_slack_identities,
make_alert_receive_channel,
make_alert_group,
refactor `SlackMessage.channel_id` (`CHAR` field) to `SlackMessage.channel` (foreign key relationship) (#5292) # What this PR does Related to https://github.com/grafana/oncall-private/issues/2947 **NOTE** This PR introduces steps 1 and 2 of the 3 part migration proposed [here](https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099). Step 3, swapping reads to be from the new-column and dropping dual-writes, will be done in a future PR/release. --- I’m tackling this work now because _ultimately_ I want to move `AlertReceiveChannel.rate_limited_in_slack_at` to `SlackChannel.rate_limited_at` , but first I sorta need to refactor `SlackMessage.channel_id` from a `CHAR` field to a foreign key relationship (because in the spots where we touch Slack rate limiting, like [here](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/alert_group_slack_service.py#L42-L50) for example, we only have `slack_message.channel_id`, which means I need to do extra queries to fetch the appropriate `SlackChannel` to then be able to get/set `SlackChannel.rate_limited_at` Other minor stuffs: - it also prepares us to drop `SlackMessage._slack_team_identity`. We already have a `@property` of `SlackMessage.slack_team_identity` (which [previously had some hacky logic](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/models/slack_message.py#L74-L84)). I've refactored `SlackMessage.slack_team_identity` to simply point to `self.organization.slack_team_identity` + updated our code to _stop_ setting `SlackMessage._slack_team_identity` (will drop this column in future release) ## 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.
2024-11-26 06:03:38 -05:00
make_slack_channel,
Fix orphaned messages in Slack (#2023) # What this PR does Reworks Slack handlers for buttons and select menus for AG Slack messages. <img width="602" alt="Screenshot 2023-05-31 at 19 34 05" src="https://github.com/grafana/oncall/assets/20116910/857bf096-7bdd-427b-94b6-15aad873a8ac"> ## Current implementation - It's possible to end up with orphaned Slack messages that are posted to Slack but have no `SlackMessage` instance in the DB. For such messages, clicking buttons will result in an exception and HTTP 500. See private repo [issue](https://github.com/grafana/oncall-private/issues/1841) for more info. - Bug in authorization system, which effectively bypasses any permission checks. For example, it's possible to resolve an alert group while being a Viewer. - No tests covering most buttons. ## Changes in this PR - Make the system more robust, don't use `SlackMessage` model to figure out the alert group being interacted on, instead embed `alert_group_pk` to every button and use it when receiving interaction requests from Slack. - Existing orphaned Slack messages will be repaired. Clicking buttons under orphaned messages will work (and missing `SlackMessage` instance will be created on interaction). This is possible because some buttons already have `alert_group_pk` embedded, and it's possible to get this data on button clicks (even if the clicked button itself doesn't have `alert_group_pk` embedded). - Fix authorization. Show warning window when unauthorized: <img width="511" alt="Screenshot 2023-05-31 at 19 40 02" src="https://github.com/grafana/oncall/assets/20116910/5abeeaa7-1b61-4a47-b3af-0e21d5cd1907"> - Added tests for all the buttons under AG message. Add tests checking authorization, actual execution of scenario steps, orphan message repairing, backward compatibility, etc. Also add tests on `AlertGroupSlackRenderer` checking that correct data is embedded into buttons. - Cosmetic changes such as renaming `incident` to `Alert Group`. ## Which issue(s) this PR fixes Related to https://github.com/grafana/oncall-private/issues/1841 ## 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)
2023-06-01 11:21:30 +01:00
make_slack_message,
):
ResolutionNoteModalStep = ScenarioStep.get_step("resolution_note", "ResolutionNoteModalStep")
Fix orphaned messages in Slack (#2023) # What this PR does Reworks Slack handlers for buttons and select menus for AG Slack messages. <img width="602" alt="Screenshot 2023-05-31 at 19 34 05" src="https://github.com/grafana/oncall/assets/20116910/857bf096-7bdd-427b-94b6-15aad873a8ac"> ## Current implementation - It's possible to end up with orphaned Slack messages that are posted to Slack but have no `SlackMessage` instance in the DB. For such messages, clicking buttons will result in an exception and HTTP 500. See private repo [issue](https://github.com/grafana/oncall-private/issues/1841) for more info. - Bug in authorization system, which effectively bypasses any permission checks. For example, it's possible to resolve an alert group while being a Viewer. - No tests covering most buttons. ## Changes in this PR - Make the system more robust, don't use `SlackMessage` model to figure out the alert group being interacted on, instead embed `alert_group_pk` to every button and use it when receiving interaction requests from Slack. - Existing orphaned Slack messages will be repaired. Clicking buttons under orphaned messages will work (and missing `SlackMessage` instance will be created on interaction). This is possible because some buttons already have `alert_group_pk` embedded, and it's possible to get this data on button clicks (even if the clicked button itself doesn't have `alert_group_pk` embedded). - Fix authorization. Show warning window when unauthorized: <img width="511" alt="Screenshot 2023-05-31 at 19 40 02" src="https://github.com/grafana/oncall/assets/20116910/5abeeaa7-1b61-4a47-b3af-0e21d5cd1907"> - Added tests for all the buttons under AG message. Add tests checking authorization, actual execution of scenario steps, orphan message repairing, backward compatibility, etc. Also add tests on `AlertGroupSlackRenderer` checking that correct data is embedded into buttons. - Cosmetic changes such as renaming `incident` to `Alert Group`. ## Which issue(s) this PR fixes Related to https://github.com/grafana/oncall-private/issues/1841 ## 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)
2023-06-01 11:21:30 +01:00
organization, user, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities()
alert_receive_channel = make_alert_receive_channel(organization)
alert_group = make_alert_group(alert_receive_channel)
refactor `SlackMessage.channel_id` (`CHAR` field) to `SlackMessage.channel` (foreign key relationship) (#5292) # What this PR does Related to https://github.com/grafana/oncall-private/issues/2947 **NOTE** This PR introduces steps 1 and 2 of the 3 part migration proposed [here](https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099). Step 3, swapping reads to be from the new-column and dropping dual-writes, will be done in a future PR/release. --- I’m tackling this work now because _ultimately_ I want to move `AlertReceiveChannel.rate_limited_in_slack_at` to `SlackChannel.rate_limited_at` , but first I sorta need to refactor `SlackMessage.channel_id` from a `CHAR` field to a foreign key relationship (because in the spots where we touch Slack rate limiting, like [here](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/alert_group_slack_service.py#L42-L50) for example, we only have `slack_message.channel_id`, which means I need to do extra queries to fetch the appropriate `SlackChannel` to then be able to get/set `SlackChannel.rate_limited_at` Other minor stuffs: - it also prepares us to drop `SlackMessage._slack_team_identity`. We already have a `@property` of `SlackMessage.slack_team_identity` (which [previously had some hacky logic](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/models/slack_message.py#L74-L84)). I've refactored `SlackMessage.slack_team_identity` to simply point to `self.organization.slack_team_identity` + updated our code to _stop_ setting `SlackMessage._slack_team_identity` (will drop this column in future release) ## 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.
2024-11-26 06:03:38 -05:00
slack_channel = make_slack_channel(slack_team_identity)
chore: drop usage of `SlackMessage.organization` + drop orphaned `SlackMessage`s (#5330) # What this PR does - Stops writing `SlackMessage.organization` + removes references to this field. [As we discussed](https://raintank-corp.slack.com/archives/C083TU81TCH/p1733315887463279?thread_ts=1733311105.095309&cid=C083TU81TCH), we do not need this field on this model/table, `SlackMessage._slack_team_identity` is sufficient (`organization` will be dropped in a separate PR) - Adds a data migration script which: - drops orphaned `SlackMessage` records; ie. ones which, even after the [`engine/apps/slack/migrations/0007_migrate_slackmessage_channel_id.py`](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/migrations/0007_migrate_slackmessage_channel_id.py) migration, still don't have a `SlackMessage.channel` id filled in (we discussed + agreed on dropping these records [here](https://raintank-corp.slack.com/archives/C083TU81TCH/p1733329914516859?thread_ts=1733311105.095309&cid=C083TU81TCH)) - fills in empty `SlackMessage.slack_team_identity` values (from `slack_message.channel.slack_team_identity`) ### Other notes On the `organization` topic. We store records in `SlackMessage` for two purposes (AFAICT), and in both cases, we have references back to the `organization`: - alert groups - `slack_message.alert_group.channel.organization` - shift swap requests - `shift_swap_request.schedule.organization` ## 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.
2024-12-06 11:43:40 -05:00
make_slack_message(slack_channel, alert_group=alert_group)
payload = {
"trigger_id": "TEST",
"view": {"id": "TEST"},
"actions": [
Fix orphaned messages in Slack (#2023) # What this PR does Reworks Slack handlers for buttons and select menus for AG Slack messages. <img width="602" alt="Screenshot 2023-05-31 at 19 34 05" src="https://github.com/grafana/oncall/assets/20116910/857bf096-7bdd-427b-94b6-15aad873a8ac"> ## Current implementation - It's possible to end up with orphaned Slack messages that are posted to Slack but have no `SlackMessage` instance in the DB. For such messages, clicking buttons will result in an exception and HTTP 500. See private repo [issue](https://github.com/grafana/oncall-private/issues/1841) for more info. - Bug in authorization system, which effectively bypasses any permission checks. For example, it's possible to resolve an alert group while being a Viewer. - No tests covering most buttons. ## Changes in this PR - Make the system more robust, don't use `SlackMessage` model to figure out the alert group being interacted on, instead embed `alert_group_pk` to every button and use it when receiving interaction requests from Slack. - Existing orphaned Slack messages will be repaired. Clicking buttons under orphaned messages will work (and missing `SlackMessage` instance will be created on interaction). This is possible because some buttons already have `alert_group_pk` embedded, and it's possible to get this data on button clicks (even if the clicked button itself doesn't have `alert_group_pk` embedded). - Fix authorization. Show warning window when unauthorized: <img width="511" alt="Screenshot 2023-05-31 at 19 40 02" src="https://github.com/grafana/oncall/assets/20116910/5abeeaa7-1b61-4a47-b3af-0e21d5cd1907"> - Added tests for all the buttons under AG message. Add tests checking authorization, actual execution of scenario steps, orphan message repairing, backward compatibility, etc. Also add tests on `AlertGroupSlackRenderer` checking that correct data is embedded into buttons. - Cosmetic changes such as renaming `incident` to `Alert Group`. ## Which issue(s) this PR fixes Related to https://github.com/grafana/oncall-private/issues/1841 ## 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)
2023-06-01 11:21:30 +01:00
{
"type": "button",
"value": json.dumps(
{
"organization_id": organization.pk,
"alert_group_pk": alert_group.pk,
"resolution_note_window_action": "update",
}
),
}
],
}
# Check that no error is raised even if the Slack API call fails
Fix orphaned messages in Slack (#2023) # What this PR does Reworks Slack handlers for buttons and select menus for AG Slack messages. <img width="602" alt="Screenshot 2023-05-31 at 19 34 05" src="https://github.com/grafana/oncall/assets/20116910/857bf096-7bdd-427b-94b6-15aad873a8ac"> ## Current implementation - It's possible to end up with orphaned Slack messages that are posted to Slack but have no `SlackMessage` instance in the DB. For such messages, clicking buttons will result in an exception and HTTP 500. See private repo [issue](https://github.com/grafana/oncall-private/issues/1841) for more info. - Bug in authorization system, which effectively bypasses any permission checks. For example, it's possible to resolve an alert group while being a Viewer. - No tests covering most buttons. ## Changes in this PR - Make the system more robust, don't use `SlackMessage` model to figure out the alert group being interacted on, instead embed `alert_group_pk` to every button and use it when receiving interaction requests from Slack. - Existing orphaned Slack messages will be repaired. Clicking buttons under orphaned messages will work (and missing `SlackMessage` instance will be created on interaction). This is possible because some buttons already have `alert_group_pk` embedded, and it's possible to get this data on button clicks (even if the clicked button itself doesn't have `alert_group_pk` embedded). - Fix authorization. Show warning window when unauthorized: <img width="511" alt="Screenshot 2023-05-31 at 19 40 02" src="https://github.com/grafana/oncall/assets/20116910/5abeeaa7-1b61-4a47-b3af-0e21d5cd1907"> - Added tests for all the buttons under AG message. Add tests checking authorization, actual execution of scenario steps, orphan message repairing, backward compatibility, etc. Also add tests on `AlertGroupSlackRenderer` checking that correct data is embedded into buttons. - Cosmetic changes such as renaming `incident` to `Alert Group`. ## Which issue(s) this PR fixes Related to https://github.com/grafana/oncall-private/issues/1841 ## 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)
2023-06-01 11:21:30 +01:00
step = ResolutionNoteModalStep(organization=organization, user=user, slack_team_identity=slack_team_identity)
step.process_scenario(slack_user_identity, slack_team_identity, payload)
# Check that "views.update" API call was made
call_args, _ = mock_slack_api_call.call_args
assert call_args[0] == "views.update"
@patch("apps.slack.models.SlackMessage.update_alert_groups_message")
@patch.object(SlackClient, "reactions_add")
@patch.object(SlackClient, "chat_getPermalink", return_value={"permalink": "https://example.com"})
@pytest.mark.django_db
def test_add_to_resolution_note(
_mock_chat_getPermalink,
mock_reactions_add,
mock_update_alert_groups_message,
make_organization_and_user_with_slack_identities,
make_alert_receive_channel,
make_alert_group,
make_alert,
make_slack_message,
make_slack_channel,
):
organization, user, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities()
alert_receive_channel = make_alert_receive_channel(organization)
alert_group = make_alert_group(alert_receive_channel)
make_alert(alert_group=alert_group, raw_request_data={})
slack_channel = make_slack_channel(slack_team_identity)
chore: drop usage of `SlackMessage.organization` + drop orphaned `SlackMessage`s (#5330) # What this PR does - Stops writing `SlackMessage.organization` + removes references to this field. [As we discussed](https://raintank-corp.slack.com/archives/C083TU81TCH/p1733315887463279?thread_ts=1733311105.095309&cid=C083TU81TCH), we do not need this field on this model/table, `SlackMessage._slack_team_identity` is sufficient (`organization` will be dropped in a separate PR) - Adds a data migration script which: - drops orphaned `SlackMessage` records; ie. ones which, even after the [`engine/apps/slack/migrations/0007_migrate_slackmessage_channel_id.py`](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/migrations/0007_migrate_slackmessage_channel_id.py) migration, still don't have a `SlackMessage.channel` id filled in (we discussed + agreed on dropping these records [here](https://raintank-corp.slack.com/archives/C083TU81TCH/p1733329914516859?thread_ts=1733311105.095309&cid=C083TU81TCH)) - fills in empty `SlackMessage.slack_team_identity` values (from `slack_message.channel.slack_team_identity`) ### Other notes On the `organization` topic. We store records in `SlackMessage` for two purposes (AFAICT), and in both cases, we have references back to the `organization`: - alert groups - `slack_message.alert_group.channel.organization` - shift swap requests - `shift_swap_request.schedule.organization` ## 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.
2024-12-06 11:43:40 -05:00
slack_message = make_slack_message(slack_channel, alert_group=alert_group)
payload = {
refactor `SlackMessage.channel_id` (`CHAR` field) to `SlackMessage.channel` (foreign key relationship) (#5292) # What this PR does Related to https://github.com/grafana/oncall-private/issues/2947 **NOTE** This PR introduces steps 1 and 2 of the 3 part migration proposed [here](https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099). Step 3, swapping reads to be from the new-column and dropping dual-writes, will be done in a future PR/release. --- I’m tackling this work now because _ultimately_ I want to move `AlertReceiveChannel.rate_limited_in_slack_at` to `SlackChannel.rate_limited_at` , but first I sorta need to refactor `SlackMessage.channel_id` from a `CHAR` field to a foreign key relationship (because in the spots where we touch Slack rate limiting, like [here](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/alert_group_slack_service.py#L42-L50) for example, we only have `slack_message.channel_id`, which means I need to do extra queries to fetch the appropriate `SlackChannel` to then be able to get/set `SlackChannel.rate_limited_at` Other minor stuffs: - it also prepares us to drop `SlackMessage._slack_team_identity`. We already have a `@property` of `SlackMessage.slack_team_identity` (which [previously had some hacky logic](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/models/slack_message.py#L74-L84)). I've refactored `SlackMessage.slack_team_identity` to simply point to `self.organization.slack_team_identity` + updated our code to _stop_ setting `SlackMessage._slack_team_identity` (will drop this column in future release) ## 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.
2024-11-26 06:03:38 -05:00
"channel": {"id": slack_channel.slack_id},
"message_ts": "random_ts",
"message": {
"type": "message",
"text": "Test resolution note",
"ts": "random_ts",
"thread_ts": slack_message.slack_id,
"user": slack_user_identity.slack_id,
},
"trigger_id": "random_trigger_id",
}
AddToResolutionNoteStep = ScenarioStep.get_step("resolution_note", "AddToResolutionNoteStep")
step = AddToResolutionNoteStep(organization=organization, user=user, slack_team_identity=slack_team_identity)
step.process_scenario(slack_user_identity, slack_team_identity, payload)
mock_reactions_add.assert_called_once()
mock_update_alert_groups_message.assert_called_once_with(debounce=False)
assert alert_group.resolution_notes.get().text == "Test resolution note"
@pytest.mark.django_db
def test_add_to_resolution_note_broadcast(make_organization_and_user_with_slack_identities, settings):
settings.UNIFIED_SLACK_APP_ENABLED = True
organization, user, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities()
payload = {
"channel": {"id": "TEST"},
"message_ts": "TEST",
"message": {"thread_ts": "TEST"},
"trigger_id": "TEST",
}
AddToResolutionNoteStep = ScenarioStep.get_step("resolution_note", "AddToResolutionNoteStep")
step = AddToResolutionNoteStep(organization=organization, user=user, slack_team_identity=slack_team_identity)
with patch.object(SlackClient, "api_call") as mock_api_call:
step.process_scenario(slack_user_identity, slack_team_identity, payload)
mock_api_call.assert_not_called() # no Slack API calls should be made
@patch.object(SlackClient, "chat_getPermalink", return_value={"permalink": "https://example.com"})
@pytest.mark.django_db
def test_add_to_resolution_note_deleted_org(
_,
make_organization_and_user_with_slack_identities,
make_alert_receive_channel,
make_alert_group,
make_alert,
refactor `SlackMessage.channel_id` (`CHAR` field) to `SlackMessage.channel` (foreign key relationship) (#5292) # What this PR does Related to https://github.com/grafana/oncall-private/issues/2947 **NOTE** This PR introduces steps 1 and 2 of the 3 part migration proposed [here](https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099). Step 3, swapping reads to be from the new-column and dropping dual-writes, will be done in a future PR/release. --- I’m tackling this work now because _ultimately_ I want to move `AlertReceiveChannel.rate_limited_in_slack_at` to `SlackChannel.rate_limited_at` , but first I sorta need to refactor `SlackMessage.channel_id` from a `CHAR` field to a foreign key relationship (because in the spots where we touch Slack rate limiting, like [here](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/alert_group_slack_service.py#L42-L50) for example, we only have `slack_message.channel_id`, which means I need to do extra queries to fetch the appropriate `SlackChannel` to then be able to get/set `SlackChannel.rate_limited_at` Other minor stuffs: - it also prepares us to drop `SlackMessage._slack_team_identity`. We already have a `@property` of `SlackMessage.slack_team_identity` (which [previously had some hacky logic](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/models/slack_message.py#L74-L84)). I've refactored `SlackMessage.slack_team_identity` to simply point to `self.organization.slack_team_identity` + updated our code to _stop_ setting `SlackMessage._slack_team_identity` (will drop this column in future release) ## 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.
2024-11-26 06:03:38 -05:00
make_slack_channel,
make_slack_message,
make_organization,
make_user_for_organization,
settings,
):
settings.UNIFIED_SLACK_APP_ENABLED = True
refactor `SlackMessage.channel_id` (`CHAR` field) to `SlackMessage.channel` (foreign key relationship) (#5292) # What this PR does Related to https://github.com/grafana/oncall-private/issues/2947 **NOTE** This PR introduces steps 1 and 2 of the 3 part migration proposed [here](https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099). Step 3, swapping reads to be from the new-column and dropping dual-writes, will be done in a future PR/release. --- I’m tackling this work now because _ultimately_ I want to move `AlertReceiveChannel.rate_limited_in_slack_at` to `SlackChannel.rate_limited_at` , but first I sorta need to refactor `SlackMessage.channel_id` from a `CHAR` field to a foreign key relationship (because in the spots where we touch Slack rate limiting, like [here](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/alert_group_slack_service.py#L42-L50) for example, we only have `slack_message.channel_id`, which means I need to do extra queries to fetch the appropriate `SlackChannel` to then be able to get/set `SlackChannel.rate_limited_at` Other minor stuffs: - it also prepares us to drop `SlackMessage._slack_team_identity`. We already have a `@property` of `SlackMessage.slack_team_identity` (which [previously had some hacky logic](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/models/slack_message.py#L74-L84)). I've refactored `SlackMessage.slack_team_identity` to simply point to `self.organization.slack_team_identity` + updated our code to _stop_ setting `SlackMessage._slack_team_identity` (will drop this column in future release) ## 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.
2024-11-26 06:03:38 -05:00
organization, _, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities()
alert_receive_channel = make_alert_receive_channel(organization)
alert_group = make_alert_group(alert_receive_channel)
make_alert(alert_group=alert_group, raw_request_data={})
refactor `SlackMessage.channel_id` (`CHAR` field) to `SlackMessage.channel` (foreign key relationship) (#5292) # What this PR does Related to https://github.com/grafana/oncall-private/issues/2947 **NOTE** This PR introduces steps 1 and 2 of the 3 part migration proposed [here](https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099). Step 3, swapping reads to be from the new-column and dropping dual-writes, will be done in a future PR/release. --- I’m tackling this work now because _ultimately_ I want to move `AlertReceiveChannel.rate_limited_in_slack_at` to `SlackChannel.rate_limited_at` , but first I sorta need to refactor `SlackMessage.channel_id` from a `CHAR` field to a foreign key relationship (because in the spots where we touch Slack rate limiting, like [here](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/alert_group_slack_service.py#L42-L50) for example, we only have `slack_message.channel_id`, which means I need to do extra queries to fetch the appropriate `SlackChannel` to then be able to get/set `SlackChannel.rate_limited_at` Other minor stuffs: - it also prepares us to drop `SlackMessage._slack_team_identity`. We already have a `@property` of `SlackMessage.slack_team_identity` (which [previously had some hacky logic](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/models/slack_message.py#L74-L84)). I've refactored `SlackMessage.slack_team_identity` to simply point to `self.organization.slack_team_identity` + updated our code to _stop_ setting `SlackMessage._slack_team_identity` (will drop this column in future release) ## 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.
2024-11-26 06:03:38 -05:00
slack_channel = make_slack_channel(slack_team_identity)
chore: drop usage of `SlackMessage.organization` + drop orphaned `SlackMessage`s (#5330) # What this PR does - Stops writing `SlackMessage.organization` + removes references to this field. [As we discussed](https://raintank-corp.slack.com/archives/C083TU81TCH/p1733315887463279?thread_ts=1733311105.095309&cid=C083TU81TCH), we do not need this field on this model/table, `SlackMessage._slack_team_identity` is sufficient (`organization` will be dropped in a separate PR) - Adds a data migration script which: - drops orphaned `SlackMessage` records; ie. ones which, even after the [`engine/apps/slack/migrations/0007_migrate_slackmessage_channel_id.py`](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/migrations/0007_migrate_slackmessage_channel_id.py) migration, still don't have a `SlackMessage.channel` id filled in (we discussed + agreed on dropping these records [here](https://raintank-corp.slack.com/archives/C083TU81TCH/p1733329914516859?thread_ts=1733311105.095309&cid=C083TU81TCH)) - fills in empty `SlackMessage.slack_team_identity` values (from `slack_message.channel.slack_team_identity`) ### Other notes On the `organization` topic. We store records in `SlackMessage` for two purposes (AFAICT), and in both cases, we have references back to the `organization`: - alert groups - `slack_message.alert_group.channel.organization` - shift swap requests - `shift_swap_request.schedule.organization` ## 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.
2024-12-06 11:43:40 -05:00
slack_message = make_slack_message(slack_channel, alert_group=alert_group)
organization.delete()
other_organization = make_organization(slack_team_identity=slack_team_identity)
other_user = make_user_for_organization(organization=other_organization, slack_user_identity=slack_user_identity)
payload = {
refactor `SlackMessage.channel_id` (`CHAR` field) to `SlackMessage.channel` (foreign key relationship) (#5292) # What this PR does Related to https://github.com/grafana/oncall-private/issues/2947 **NOTE** This PR introduces steps 1 and 2 of the 3 part migration proposed [here](https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099). Step 3, swapping reads to be from the new-column and dropping dual-writes, will be done in a future PR/release. --- I’m tackling this work now because _ultimately_ I want to move `AlertReceiveChannel.rate_limited_in_slack_at` to `SlackChannel.rate_limited_at` , but first I sorta need to refactor `SlackMessage.channel_id` from a `CHAR` field to a foreign key relationship (because in the spots where we touch Slack rate limiting, like [here](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/alert_group_slack_service.py#L42-L50) for example, we only have `slack_message.channel_id`, which means I need to do extra queries to fetch the appropriate `SlackChannel` to then be able to get/set `SlackChannel.rate_limited_at` Other minor stuffs: - it also prepares us to drop `SlackMessage._slack_team_identity`. We already have a `@property` of `SlackMessage.slack_team_identity` (which [previously had some hacky logic](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/models/slack_message.py#L74-L84)). I've refactored `SlackMessage.slack_team_identity` to simply point to `self.organization.slack_team_identity` + updated our code to _stop_ setting `SlackMessage._slack_team_identity` (will drop this column in future release) ## 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.
2024-11-26 06:03:38 -05:00
"channel": {"id": slack_message.channel.slack_id},
"message_ts": "random_ts",
"message": {
"type": "message",
"text": "Test resolution note",
"ts": "random_ts",
"thread_ts": slack_message.slack_id,
"user": slack_user_identity.slack_id,
},
"trigger_id": "random_trigger_id",
}
AddToResolutionNoteStep = ScenarioStep.get_step("resolution_note", "AddToResolutionNoteStep")
step = AddToResolutionNoteStep(
organization=other_organization, user=other_user, slack_team_identity=slack_team_identity
)
with patch.object(SlackClient, "api_call") as mock_api_call:
step.process_scenario(slack_user_identity, slack_team_identity, payload)
mock_api_call.assert_not_called() # no Slack API calls should be made