From 9657533b5b5f05c28cede2e17ae12684e694a2aa Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 22 Dec 2023 07:36:54 -0500 Subject: [PATCH] fix duplicate teams showing up in teams dropdown for `/escalate` slack command (#3590) # Which issue(s) this PR fixes - Closes https://github.com/grafana/support-escalations/issues/8763 - Closes https://github.com/grafana/oncall/issues/3388 ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [ ] 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) --- CHANGELOG.md | 7 ++--- .../tests/test_scenario_steps/test_paging.py | 30 +++++++++++++++---- .../user_management/models/organization.py | 24 ++++++++------- .../tests/test_organization.py | 9 ++++++ 4 files changed, 50 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 186e4e79d9..e2b3eef550 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix alert group table columns validation @Ferril ([#3577](https://github.com/grafana/oncall/pull/3577)) - Fix posting message about rate limit to Slack @Ferril ([#3582](https://github.com/grafana/oncall/pull/3582)) - Fix issue with parsing sender email address from email message for inbound email integration endpoint @Ferril ([#3586](https://github.com/grafana/oncall/pull/3586)) +- Fix PUT /api/v1/escalation_policies/id issue when updating `from_time` and `to_time` by @joeyorlando ([#3581](https://github.com/grafana/oncall/pull/3581)) +- Fix issue where duplicate team options would show up in the teams dropdown for the `/escalate` Slack command + by @joeyorlando ([#3590](https://github.com/grafana/oncall/pull/3590)) ## v1.3.80 (2023-12-14) @@ -35,10 +38,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add backend for multi-stack support for mobile-app @Ferril ([#3500](https://github.com/grafana/oncall/pull/3500)) -### Fixed - -- Fix PUT /api/v1/escalation_policies/id issue when updating `from_time` and `to_time` by @joeyorlando ([#3581](https://github.com/grafana/oncall/pull/3581)) - ## v1.3.78 (2023-12-12) ### Changed diff --git a/engine/apps/slack/tests/test_scenario_steps/test_paging.py b/engine/apps/slack/tests/test_scenario_steps/test_paging.py index 64c0c29e84..d665328af1 100644 --- a/engine/apps/slack/tests/test_scenario_steps/test_paging.py +++ b/engine/apps/slack/tests/test_scenario_steps/test_paging.py @@ -284,6 +284,9 @@ def test_get_team_select_blocks( input_id_prefix = "nmxcnvmnxv" + def _contstruct_team_option(team): + return {"text": {"emoji": True, "text": team.name, "type": "plain_text"}, "value": str(team.pk)} + # no team selected - no team direct paging integrations available organization, _, _, slack_user_identity = make_organization_and_user_with_slack_identities() blocks = _get_team_select_blocks(slack_user_identity, organization, False, None, input_id_prefix) @@ -309,11 +312,9 @@ def test_get_team_select_blocks( assert len(blocks) == 2 input_block, context_block = blocks - team_option = {"text": {"emoji": True, "text": team.name, "type": "plain_text"}, "value": str(team.pk)} - assert input_block["type"] == "input" assert len(input_block["element"]["options"]) == 1 - assert input_block["element"]["options"] == [team_option] + assert input_block["element"]["options"] == [_contstruct_team_option(team)] assert context_block["elements"][0]["text"] == info_msg # team selected @@ -337,9 +338,6 @@ def _setup_direct_paging_integration(team): assert len(blocks) == 2 input_block, context_block = blocks - def _contstruct_team_option(team): - return {"text": {"emoji": True, "text": team.name, "type": "plain_text"}, "value": str(team.pk)} - team1_option = _contstruct_team_option(team1) team2_option = _contstruct_team_option(team2) @@ -355,3 +353,23 @@ def _sort_team_options(options): context_block["elements"][0]["text"] == f"Integration <{team2_direct_paging_arc.web_link}|{team2_direct_paging_arc.verbal_name}> will be used for notification." ) + + # team's direct paging integration has two routes associated with it + # the team should only be displayed once + organization, _, _, slack_user_identity = make_organization_and_user_with_slack_identities() + team = make_team(organization) + + arc = make_alert_receive_channel(organization, team=team, integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING) + escalation_chain = make_escalation_chain(organization) + make_channel_filter(arc, is_default=True, escalation_chain=escalation_chain) + make_channel_filter(arc, escalation_chain=escalation_chain) + + blocks = _get_team_select_blocks(slack_user_identity, organization, False, None, input_id_prefix) + + assert len(blocks) == 2 + input_block, context_block = blocks + + assert input_block["type"] == "input" + assert len(input_block["element"]["options"]) == 1 + assert input_block["element"]["options"] == [_contstruct_team_option(team)] + assert context_block["elements"][0]["text"] == info_msg diff --git a/engine/apps/user_management/models/organization.py b/engine/apps/user_management/models/organization.py index 9d550ab23d..ef4bb8d990 100644 --- a/engine/apps/user_management/models/organization.py +++ b/engine/apps/user_management/models/organization.py @@ -323,16 +323,20 @@ def get_notifiable_direct_paging_integrations(self) -> "RelatedManager['AlertRec """ from apps.alerts.models import AlertReceiveChannel - return self.alert_receive_channels.annotate( - num_channel_filters=Count("channel_filters"), - # used to determine if the organization has telegram configured - num_org_telegram_channels=Count("organization__telegram_channel"), - ).filter( - Q(num_channel_filters__gt=1) - | (Q(organization__slack_team_identity__isnull=False) | Q(num_org_telegram_channels__gt=0)) - | Q(channel_filters__is_default=True, channel_filters__escalation_chain__isnull=False) - | Q(channel_filters__is_default=True, channel_filters__notification_backends__isnull=False), - integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING, + return ( + self.alert_receive_channels.annotate( + num_channel_filters=Count("channel_filters"), + # used to determine if the organization has telegram configured + num_org_telegram_channels=Count("organization__telegram_channel"), + ) + .filter( + Q(num_channel_filters__gt=1) + | (Q(organization__slack_team_identity__isnull=False) | Q(num_org_telegram_channels__gt=0)) + | Q(channel_filters__is_default=True, channel_filters__escalation_chain__isnull=False) + | Q(channel_filters__is_default=True, channel_filters__notification_backends__isnull=False), + integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING, + ) + .distinct() ) @property diff --git a/engine/apps/user_management/tests/test_organization.py b/engine/apps/user_management/tests/test_organization.py index 91915500b5..58636f880a 100644 --- a/engine/apps/user_management/tests/test_organization.py +++ b/engine/apps/user_management/tests/test_organization.py @@ -217,6 +217,7 @@ def _assert(org, arc, should_be_returned=True): assert arc in notifiable_direct_paging_integrations else: assert arc not in notifiable_direct_paging_integrations + return notifiable_direct_paging_integrations # integration has no default channel filter org, arc = _make_org_and_arc() @@ -269,3 +270,11 @@ def _assert(org, arc, should_be_returned=True): escalation_chain = make_escalation_chain(org) make_channel_filter(arc, is_default=True, notify_in_slack=False, escalation_chain=escalation_chain) _assert(org, arc) + + # integration has more than one channel filter associated with it, nevertheless the integration should only + # be returned once + org, arc = _make_org_and_arc() + make_channel_filter(arc, is_default=True) + make_channel_filter(arc, is_default=False) + notifiable_direct_paging_integrations = _assert(org, arc) + assert notifiable_direct_paging_integrations.count() == 1