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

fix duplicate teams showing up in teams dropdown for /escalate slack command #3590

Merged
merged 3 commits into from
Dec 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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))

Comment on lines -38 to -41
Copy link
Contributor Author

@joeyorlando joeyorlando Dec 19, 2023

Choose a reason for hiding this comment

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

I added this to the wrong section, this hasn't been released yet

## v1.3.78 (2023-12-12)

### Changed
Expand Down
30 changes: 24 additions & 6 deletions engine/apps/slack/tests/test_scenario_steps/test_paging.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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)

Expand All @@ -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
24 changes: 14 additions & 10 deletions engine/apps/user_management/models/organization.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

only change is the addition of .distinct() here (rest is formatting)

)

@property
Expand Down
9 changes: 9 additions & 0 deletions engine/apps/user_management/tests/test_organization.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Loading