From b8d78fd6bb8e02768e9905e5c72d09b26846d64e Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Wed, 18 Jan 2023 15:52:25 +0000 Subject: [PATCH 1/4] Allow messaging backends to be enabled/disabled per organization (#1151) # What this PR does Allows messaging backends to be enabled/disabled per organization when getting a list of available personal notification channels. ## Checklist - [x] Tests updated - [ ] Documentation added (N/A) - [x] `CHANGELOG.md` updated --- CHANGELOG.md | 6 ++++++ .../tests/test_user_notification_policy.py | 21 +++++++++++++++++++ .../api/views/user_notification_policy.py | 4 +++- engine/apps/base/messaging.py | 4 ++++ engine/apps/mobile_app/backend.py | 9 ++++++++ 5 files changed, 43 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9e1ff0d..11ef9797 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Added + +- Allow messaging backends to be enabled/disabled per organization ([#1151](https://github.com/grafana/oncall/pull/1151)) + ## v1.1.17 (2023-01-18) ### Changed diff --git a/engine/apps/api/tests/test_user_notification_policy.py b/engine/apps/api/tests/test_user_notification_policy.py index 4bc9c306..3cda1f0d 100644 --- a/engine/apps/api/tests/test_user_notification_policy.py +++ b/engine/apps/api/tests/test_user_notification_policy.py @@ -1,4 +1,5 @@ import json +from unittest.mock import patch import pytest from django.urls import reverse @@ -8,6 +9,7 @@ from rest_framework.test import APIClient from apps.api.permissions import LegacyAccessControlRole from apps.base.models import UserNotificationPolicy +from apps.base.tests.messaging_backend import TestOnlyBackend DEFAULT_NOTIFICATION_CHANNEL = UserNotificationPolicy.NotificationChannel.SLACK @@ -463,3 +465,22 @@ def test_notification_policy_backends_enabled( assert response.status_code == status.HTTP_200_OK options = [opt["display_name"] for opt in response.json()] assert "Test Only Backend" in options + + +@pytest.mark.django_db +def test_notification_policy_backends_disabled_for_organization( + user_notification_policy_internal_api_setup, settings, make_user_auth_headers +): + token, _, users = user_notification_policy_internal_api_setup + admin, _ = users + + client = APIClient() + url = reverse("api-internal:notification_policy-notify-by-options") + + with patch.object(TestOnlyBackend, "is_enabled_for_organization", return_value=False): + response = client.get(url, **make_user_auth_headers(admin, token)) + + assert response.status_code == status.HTTP_200_OK + + options = [opt["display_name"] for opt in response.json()] + assert "Test Only Backend" not in options diff --git a/engine/apps/api/views/user_notification_policy.py b/engine/apps/api/views/user_notification_policy.py index e43622a0..08b45c01 100644 --- a/engine/apps/api/views/user_notification_policy.py +++ b/engine/apps/api/views/user_notification_policy.py @@ -176,7 +176,9 @@ class UserNotificationPolicyView(UpdateSerializerMixin, ModelViewSet): built_in_backend_names = {b[0] for b in BUILT_IN_BACKENDS} if notification_channel.name not in built_in_backend_names: extra_messaging_backend = get_messaging_backend_from_id(notification_channel.name) - if extra_messaging_backend is None: + if extra_messaging_backend is None or not extra_messaging_backend.is_enabled_for_organization( + request.auth.organization + ): continue choices.append( diff --git a/engine/apps/base/messaging.py b/engine/apps/base/messaging.py index 7d2b2e88..6c66277d 100644 --- a/engine/apps/base/messaging.py +++ b/engine/apps/base/messaging.py @@ -38,6 +38,10 @@ class BaseMessagingBackend: """Remove backend link to user account.""" return + @staticmethod + def is_enabled_for_organization(organization): + return True + def serialize_user(self, user): """Return a serialized backend user representation.""" raise NotImplementedError("serialize_user method missing implementation") diff --git a/engine/apps/mobile_app/backend.py b/engine/apps/mobile_app/backend.py index 43f257c3..381782f6 100644 --- a/engine/apps/mobile_app/backend.py +++ b/engine/apps/mobile_app/backend.py @@ -4,6 +4,7 @@ from django.conf import settings from fcm_django.models import FCMDevice from apps.base.messaging import BaseMessagingBackend +from apps.base.models import DynamicSetting from apps.mobile_app.tasks import notify_user_async @@ -50,6 +51,14 @@ class MobileAppBackend(BaseMessagingBackend): critical=critical, ) + @staticmethod + def is_enabled_for_organization(organization): + mobile_app_settings, _ = DynamicSetting.objects.get_or_create( + name="mobile_app_settings", defaults={"json_value": {"org_ids": []}} + ) + + return organization.pk in mobile_app_settings.json_value["org_ids"] + class MobileAppCriticalBackend(MobileAppBackend): """ From 90def887521dc1c01e5d59b2ec88e1b065e7e4fd Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Wed, 18 Jan 2023 12:58:26 -0300 Subject: [PATCH 2/4] Add escalation chain option when creating a direct page alert group (#1143) Also changes the default integration used when creating an alert group for a direct page to a custom manual integration to avoid conflicts/unexpected behaviors with existing manual alerts. --- engine/apps/alerts/models/alert.py | 8 ++- engine/apps/alerts/paging.py | 50 +++++++++++++++-- engine/apps/alerts/tests/test_alert.py | 43 ++++++++++++++ engine/apps/alerts/tests/test_paging.py | 35 ++++++++++++ .../html/integration_direct_paging.html | 2 + engine/config_integrations/direct_paging.py | 56 +++++++++++++++++++ engine/settings/base.py | 1 + 7 files changed, 186 insertions(+), 9 deletions(-) create mode 100644 engine/apps/alerts/tests/test_alert.py create mode 100644 engine/apps/integrations/templates/html/integration_direct_paging.html create mode 100644 engine/config_integrations/direct_paging.py diff --git a/engine/apps/alerts/models/alert.py b/engine/apps/alerts/models/alert.py index 3a1c7fe5..f8b012f5 100644 --- a/engine/apps/alerts/models/alert.py +++ b/engine/apps/alerts/models/alert.py @@ -79,6 +79,7 @@ class Alert(models.Model): raw_request_data, enable_autoresolve=True, is_demo=False, + channel_filter=None, force_route_id=None, ): ChannelFilter = apps.get_model("alerts", "ChannelFilter") @@ -87,9 +88,10 @@ class Alert(models.Model): AlertGroupLogRecord = apps.get_model("alerts", "AlertGroupLogRecord") group_data = Alert.render_group_data(alert_receive_channel, raw_request_data, is_demo) - channel_filter = ChannelFilter.select_filter( - alert_receive_channel, raw_request_data, title, message, force_route_id - ) + if channel_filter is None: + channel_filter = ChannelFilter.select_filter( + alert_receive_channel, raw_request_data, title, message, force_route_id + ) group, group_created = AlertGroup.all_objects.get_or_create_grouping( channel=alert_receive_channel, diff --git a/engine/apps/alerts/paging.py b/engine/apps/alerts/paging.py index d7bd715c..c82bb7dd 100644 --- a/engine/apps/alerts/paging.py +++ b/engine/apps/alerts/paging.py @@ -3,7 +3,15 @@ from typing import Any from django.db import transaction from django.db.models import Q -from apps.alerts.models import Alert, AlertGroup, AlertGroupLogRecord, AlertReceiveChannel, UserHasNotification +from apps.alerts.models import ( + Alert, + AlertGroup, + AlertGroupLogRecord, + AlertReceiveChannel, + ChannelFilter, + EscalationChain, + UserHasNotification, +) from apps.alerts.tasks.notify_user import notify_user_task from apps.schedules.ical_utils import list_users_to_notify_from_ical from apps.schedules.models import OnCallSchedule @@ -17,18 +25,43 @@ UserNotifications = list[tuple[User, bool]] ScheduleNotifications = list[tuple[OnCallSchedule, bool]] -def _trigger_alert(organization: Organization, team: Team, title: str, message: str, from_user: User) -> AlertGroup: +def _trigger_alert( + organization: Organization, + team: Team, + title: str, + message: str, + from_user: User, + escalation_chain: EscalationChain = None, +) -> AlertGroup: """Trigger manual integration alert from params.""" alert_receive_channel = AlertReceiveChannel.get_or_create_manual_integration( organization=organization, team=team, - integration=AlertReceiveChannel.INTEGRATION_MANUAL, + integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING, deleted_at=None, defaults={ "author": from_user, - "verbal_name": f"Manual alert groups ({team.name if team else 'General'} team)", + "verbal_name": f"Direct paging ({team.name if team else 'General'} team)", }, ) + if alert_receive_channel.default_channel_filter is None: + ChannelFilter.objects.create( + alert_receive_channel=alert_receive_channel, + notify_in_slack=True, + is_default=True, + ) + + channel_filter = None + if escalation_chain is not None: + channel_filter, _ = ChannelFilter.objects.get_or_create( + alert_receive_channel=alert_receive_channel, + escalation_chain=escalation_chain, + is_default=False, + defaults={ + "filtering_term": f"escalate to {escalation_chain.name}", + "notify_in_slack": True, + }, + ) permalink = None if not title: @@ -49,6 +82,7 @@ def _trigger_alert(organization: Organization, team: Team, title: str, message: integration_unique_data={"created_by": from_user.username}, image_url=None, link_to_upstream_details=None, + channel_filter=channel_filter, ) return alert.group @@ -103,6 +137,7 @@ def direct_paging( message: str = None, users: UserNotifications = None, schedules: ScheduleNotifications = None, + escalation_chain: EscalationChain = None, alert_group: AlertGroup = None, ) -> None: """Trigger escalation targeting given users/schedules. @@ -111,7 +146,7 @@ def direct_paging( Otherwise, create a new alert using given title and message. """ - if not users and not schedules: + if not users and not schedules and not escalation_chain: return if users is None: @@ -120,9 +155,12 @@ def direct_paging( if schedules is None: schedules = [] + if escalation_chain is not None and alert_group is not None: + raise ValueError("Cannot change an existing alert group escalation chain") + # create alert group if needed if alert_group is None: - alert_group = _trigger_alert(organization, team, title, message, from_user) + alert_group = _trigger_alert(organization, team, title, message, from_user, escalation_chain=escalation_chain) # get on call users, add log entry for each schedule for (s, important) in schedules: diff --git a/engine/apps/alerts/tests/test_alert.py b/engine/apps/alerts/tests/test_alert.py new file mode 100644 index 00000000..34901075 --- /dev/null +++ b/engine/apps/alerts/tests/test_alert.py @@ -0,0 +1,43 @@ +import pytest + +from apps.alerts.models import Alert + + +@pytest.mark.django_db +def test_alert_create_default_channel_filter(make_organization, make_alert_receive_channel, make_channel_filter): + organization = make_organization() + alert_receive_channel = make_alert_receive_channel(organization) + channel_filter = make_channel_filter(alert_receive_channel, is_default=True) + + alert = Alert.create( + title="the title", + message="the message", + alert_receive_channel=alert_receive_channel, + raw_request_data={}, + integration_unique_data={}, + image_url=None, + link_to_upstream_details=None, + ) + + assert alert.group.channel_filter == channel_filter + + +@pytest.mark.django_db +def test_alert_create_custom_channel_filter(make_organization, make_alert_receive_channel, make_channel_filter): + organization = make_organization() + alert_receive_channel = make_alert_receive_channel(organization) + make_channel_filter(alert_receive_channel, is_default=True) + other_channel_filter = make_channel_filter(alert_receive_channel) + + alert = Alert.create( + title="the title", + message="the message", + alert_receive_channel=alert_receive_channel, + raw_request_data={}, + integration_unique_data={}, + image_url=None, + link_to_upstream_details=None, + channel_filter=other_channel_filter, + ) + + assert alert.group.channel_filter == other_channel_filter diff --git a/engine/apps/alerts/tests/test_paging.py b/engine/apps/alerts/tests/test_paging.py index 3cbc5039..59715a42 100644 --- a/engine/apps/alerts/tests/test_paging.py +++ b/engine/apps/alerts/tests/test_paging.py @@ -244,6 +244,41 @@ def test_direct_paging_reusing_alert_group( assert notify_task.apply_async.called_with((user.pk, ag.pk), {"important": False}) +@pytest.mark.django_db +def test_direct_paging_reusing_alert_group_custom_chain_raises( + make_organization, make_user_for_organization, make_alert_receive_channel, make_alert_group, make_escalation_chain +): + organization = make_organization() + from_user = make_user_for_organization(organization) + alert_receive_channel = make_alert_receive_channel(organization=organization) + alert_group = make_alert_group(alert_receive_channel=alert_receive_channel) + custom_chain = make_escalation_chain(organization) + + with pytest.raises(ValueError): + direct_paging(organization, None, from_user, alert_group=alert_group, escalation_chain=custom_chain) + + +@pytest.mark.django_db +def test_direct_paging_custom_chain( + make_organization, make_user_for_organization, make_alert_receive_channel, make_alert_group, make_escalation_chain +): + organization = make_organization() + from_user = make_user_for_organization(organization) + custom_chain = make_escalation_chain(organization) + + direct_paging(organization, None, from_user, escalation_chain=custom_chain) + + # alert group created + alert_groups = AlertGroup.all_objects.all() + assert alert_groups.count() == 1 + ag = alert_groups.get() + channel_filter = ag.channel_filter_with_respect_to_escalation_snapshot + assert channel_filter is not None + assert not channel_filter.is_default + assert channel_filter.notify_in_slack + assert ag.escalation_chain_with_respect_to_escalation_snapshot == custom_chain + + @pytest.mark.django_db def test_unpage_user_not_exists( make_organization, make_user_for_organization, make_alert_receive_channel, make_alert_group diff --git a/engine/apps/integrations/templates/html/integration_direct_paging.html b/engine/apps/integrations/templates/html/integration_direct_paging.html new file mode 100644 index 00000000..0d87618b --- /dev/null +++ b/engine/apps/integrations/templates/html/integration_direct_paging.html @@ -0,0 +1,2 @@ + +

You can create a direct page alert group from the web UI

diff --git a/engine/config_integrations/direct_paging.py b/engine/config_integrations/direct_paging.py new file mode 100644 index 00000000..8b400c83 --- /dev/null +++ b/engine/config_integrations/direct_paging.py @@ -0,0 +1,56 @@ +# Main +enabled = True +title = "Direct paging" +slug = "direct_paging" +short_description = None +description = None +is_displayed_on_web = False +is_featured = False +is_able_to_autoresolve = False +is_demo_alert_enabled = False + +description = None + +# Default templates +slack_title = """\ +*<{{ grafana_oncall_link }}|#{{ grafana_oncall_incident_id }} {{ payload.oncall.title }}>* via {{ integration_name }} +{% if source_link %} + (*<{{ source_link }}|source>*) +{% endif %} +""" + +slack_message = """{{ payload.oncall.message }} + +created by {{ payload.oncall.author_username }} +""" + +slack_image_url = None + +web_title = "{{ payload.oncall.title }}" + +web_message = """{{ payload.oncall.message }} +{% if source_link %} +<{{ source_link }} | Link to the original message > +{% endif %} +created by {{ payload.oncall.author_username }} +""" + +web_image_url = slack_image_url + +sms_title = web_title + +phone_call_title = sms_title + +telegram_title = sms_title + +telegram_message = slack_message + +telegram_image_url = slack_image_url + +source_link = "{{ payload.oncall.permalink }}" + +grouping_id = """{{ payload }}""" + +resolve_condition = None + +acknowledge_condition = None diff --git a/engine/settings/base.py b/engine/settings/base.py index b408b1a3..d28f73e3 100644 --- a/engine/settings/base.py +++ b/engine/settings/base.py @@ -617,6 +617,7 @@ INSTALLED_ONCALL_INTEGRATIONS = [ "config_integrations.manual", "config_integrations.slack_channel", "config_integrations.zabbix", + "config_integrations.direct_paging", ] if OSS_INSTALLATION: From c93ee5c5543ae26766e05e820c55012bd1322f4d Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Wed, 18 Jan 2023 16:08:15 +0000 Subject: [PATCH 3/4] Send a Slack DM when user is not in channel (#1144) # What this PR does Currently, when a user gets mentioned in an alert group thread and the user is not in the Slack channel, the Slack bot sends the following to the channel: > :warning: Tried to ask USER to look at incident. Unfortunately USER is not in this channel. Please, invite. This PR changes this behaviour to instead send a direct message to the user. The message contains a link to the main alert group message in Slack. Screenshot 2023-01-17 at 19 25 36 ## Checklist - [ ] Tests updated (N/A) - [ ] Documentation added (N/A) - [x] `CHANGELOG.md` updated --- CHANGELOG.md | 4 ++ engine/apps/slack/models/slack_message.py | 24 +---------- .../apps/slack/models/slack_user_identity.py | 43 +++++++++++++++++++ 3 files changed, 48 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11ef9797..c59551f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Allow messaging backends to be enabled/disabled per organization ([#1151](https://github.com/grafana/oncall/pull/1151)) +### Changed + +- Send a Slack DM when user is not in channel ([#1144](https://github.com/grafana/oncall/pull/1144)) + ## v1.1.17 (2023-01-18) ### Changed diff --git a/engine/apps/slack/models/slack_message.py b/engine/apps/slack/models/slack_message.py index 96b3aae8..614ef5d2 100644 --- a/engine/apps/slack/models/slack_message.py +++ b/engine/apps/slack/models/slack_message.py @@ -212,29 +212,7 @@ class SlackMessage(models.Model): if slack_user_identity.slack_id not in channel_members: time.sleep(5) # 2 messages in the same moment are ratelimited by Slack. Dirty hack. - result = sc.api_call( - "chat.postMessage", - channel=channel_id, - text=f":warning: Tried to ask {user_verbal} to look at incident. " - f"Unfortunately {user_verbal} is not in this channel. Please, invite.", - ) - SlackMessage( - slack_id=result["ts"], - organization=self.organization, - _slack_team_identity=self.slack_team_identity, - channel_id=channel_id, - alert_group=alert_group, - ).save() - UserNotificationPolicyLogRecord( - author=user, - type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED, - notification_policy=notification_policy, - alert_group=alert_group, - reason="User is not in Slack channel", - notification_step=notification_policy.step, - notification_channel=notification_policy.notify_by, - notification_error_code=UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_IN_SLACK_USER_NOT_IN_CHANNEL, - ).save() + slack_user_identity.send_link_to_slack_message(slack_message) except SlackAPITokenException as e: print(e) except SlackAPIException as e: diff --git a/engine/apps/slack/models/slack_user_identity.py b/engine/apps/slack/models/slack_user_identity.py index 4c0047fa..c9288baf 100644 --- a/engine/apps/slack/models/slack_user_identity.py +++ b/engine/apps/slack/models/slack_user_identity.py @@ -92,6 +92,49 @@ class SlackUserIdentity(models.Model): def __str__(self): return self.slack_login + def send_link_to_slack_message(self, slack_message): + blocks = [ + { + "type": "section", + "text": { + "type": "plain_text", + "text": "You are invited to look at an alert group!", + "emoji": True, + }, + }, + { + "type": "actions", + "elements": [ + { + "type": "button", + "text": {"type": "plain_text", "text": "➡️ Go to the alert group"}, + "url": slack_message.permalink, + "style": "primary", + } + ], + }, + { + "type": "context", + "elements": [ + { + "type": "mrkdwn", + "text": ( + f"You received this message because you're not a member of <#{slack_message.channel_id}>.\n" + "Please join the channel to get notified right in the alert group thread." + ), + } + ], + }, + ] + + sc = SlackClientWithErrorHandling(self.slack_team_identity.bot_access_token) + return sc.api_call( + "chat.postMessage", + channel=self.im_channel_id, + text="You are invited to look at an alert group!", + blocks=blocks, + ) + @property def slack_verbal(self): return ( From 8e0438ddc82c2668faf462bb24ce7ce0464bf6be Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Wed, 18 Jan 2023 17:37:33 +0000 Subject: [PATCH 4/4] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c59551f9..a7d08a1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## Unreleased +## v1.1.18 (2023-01-18) ### Added