From 69bafb61f155ac8a1ac2cb0536aeddb3f07df7e6 Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Mon, 17 Jul 2023 14:21:56 +0100 Subject: [PATCH] Direct paging improvements (#2537) # What this PR does - Deprecates `/oncall` Slack command in favour of `/esalate` (direct paging) + fixes a regression bug in both commands - Unifies direct paging UX across Slack & Web UI (or at least makes an attempt to make things more similar). Kudos to @iskhakov for all the great work on this recently! - A bunch of minor changes that hopefully make direct paging more usable - TODO: documentation updates will be added in a separate PR ## Screenshots ### No issues scenario Slack: Screenshot 2023-07-14 at 23 53 11 Web UI: Screenshot 2023-07-14 at 23 52 25 ### Not configured scenario Slack: Screenshot 2023-07-14 at 23 45 22 Web UI: Screenshot 2023-07-14 at 23 47 31 ### `/oncall` deprecation warning Screenshot 2023-07-17 at 10 31 56 ## Which issue(s) this PR fixes https://github.com/grafana/oncall/issues/2442 ## 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) --- CHANGELOG.md | 1 + engine/apps/alerts/paging.py | 5 +- engine/apps/alerts/tests/test_paging.py | 34 +- engine/apps/api/serializers/paging.py | 2 +- engine/apps/api/views/user.py | 2 +- .../apps/slack/scenarios/manual_incident.py | 15 +- engine/apps/slack/scenarios/paging.py | 411 ++++++++++-------- engine/apps/slack/scenarios/scenario_step.py | 1 + .../tests/test_interactive_api_endpoint.py | 33 ++ .../tests/test_scenario_steps/test_paging.py | 54 ++- engine/apps/slack/views.py | 3 +- .../alerts/directPaging.test.ts | 27 ++ .../integration-tests/utils/alertGroup.ts | 12 +- .../integration-tests/utils/navigation.ts | 2 +- .../ManualAlertGroup.config.ts | 4 +- .../ManualAlertGroup/ManualAlertGroup.tsx | 102 ++--- .../EscalationVariants/EscalationVariants.tsx | 6 +- .../GrafanaTeamSelect/GrafanaTeamSelect.tsx | 3 +- .../src/pages/incident/Incident.tsx | 3 +- .../src/pages/incidents/Incidents.tsx | 2 +- 20 files changed, 433 insertions(+), 289 deletions(-) create mode 100644 grafana-plugin/integration-tests/alerts/directPaging.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index b6c26db3..b8e4913c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Added `PHONE_PROVIDER` configuration check by @sreway ([#2523](https://github.com/grafana/oncall/pull/2523)) +- Deprecate `/oncall` Slack command, update direct paging functionality by @vadimkerr ([#2537](https://github.com/grafana/oncall/pull/2537)) ## v1.3.13 (2023-07-17) diff --git a/engine/apps/alerts/paging.py b/engine/apps/alerts/paging.py index d17f6f03..710ac486 100644 --- a/engine/apps/alerts/paging.py +++ b/engine/apps/alerts/paging.py @@ -42,7 +42,7 @@ def _trigger_alert( deleted_at=None, defaults={ "author": from_user, - "verbal_name": "Direct paging", + "verbal_name": f"Direct paging ({team.name if team else 'No'} team)", }, ) if alert_receive_channel.default_channel_filter is None: @@ -90,7 +90,7 @@ def _trigger_alert( return alert.group -def check_user_availability(user: User, team: Team) -> list[dict[str, Any]]: +def check_user_availability(user: User) -> list[dict[str, Any]]: """Check user availability to be paged. Return a warnings list indicating `error` and any additional related `data`. @@ -108,7 +108,6 @@ def check_user_availability(user: User, team: Team) -> list[dict[str, Any]]: schedules = OnCallSchedule.objects.filter( Q(cached_ical_file_primary__contains=user.username) | Q(cached_ical_file_primary__contains=user.email), organization=user.organization, - team=team, ) schedules_data = {} for s in schedules: diff --git a/engine/apps/alerts/tests/test_paging.py b/engine/apps/alerts/tests/test_paging.py index daa73a6a..901f655d 100644 --- a/engine/apps/alerts/tests/test_paging.py +++ b/engine/apps/alerts/tests/test_paging.py @@ -70,7 +70,7 @@ def test_check_user_availability_no_policies(make_organization, make_user_for_or organization = make_organization() user = make_user_for_organization(organization) - warnings = check_user_availability(user, None) + warnings = check_user_availability(user) assert warnings == [ {"data": {}, "error": USER_HAS_NO_NOTIFICATION_POLICY}, {"data": {"schedules": {}}, "error": USER_IS_NOT_ON_CALL}, @@ -95,40 +95,12 @@ def test_check_user_availability_not_on_call( make_schedule, make_on_call_shift, organization, None, other_user, extra_users=[user] ) - warnings = check_user_availability(user, None) + warnings = check_user_availability(user) assert warnings == [ {"data": {"schedules": {schedule.name: {other_user.public_primary_key}}}, "error": USER_IS_NOT_ON_CALL}, ] -@pytest.mark.django_db -def test_check_user_availability_on_call_different_team( - make_organization, - make_team, - make_user_for_organization, - make_user_notification_policy, - make_schedule, - make_on_call_shift, -): - organization = make_organization() - some_team = make_team(organization) - user = make_user_for_organization(organization) - make_user_notification_policy( - user=user, - step=UserNotificationPolicy.Step.NOTIFY, - notify_by=UserNotificationPolicy.NotificationChannel.SMS, - ) - - # setup on call schedule - # user is on call, but on a different team - setup_always_on_call_schedule(make_schedule, make_on_call_shift, organization, some_team, user) - - warnings = check_user_availability(user, None) - assert warnings == [ - {"data": {"schedules": {}}, "error": USER_IS_NOT_ON_CALL}, - ] - - @pytest.mark.django_db def test_check_user_availability_on_call( make_organization, @@ -150,7 +122,7 @@ def test_check_user_availability_on_call( # setup on call schedule setup_always_on_call_schedule(make_schedule, make_on_call_shift, organization, some_team, user) - warnings = check_user_availability(user, some_team) + warnings = check_user_availability(user) assert warnings == [] diff --git a/engine/apps/api/serializers/paging.py b/engine/apps/api/serializers/paging.py index b764f7ef..1f69b66a 100644 --- a/engine/apps/api/serializers/paging.py +++ b/engine/apps/api/serializers/paging.py @@ -49,7 +49,7 @@ class DirectPagingSerializer(serializers.Serializer): alert_group = serializers.HiddenField(default=None) # set in DirectPagingSerializer.validate title = serializers.CharField(required=False, default=None) - message = serializers.CharField(required=False, default=None) + message = serializers.CharField(required=False, default=None, allow_null=True) team = TeamPrimaryKeyRelatedField(allow_null=True, default=CurrentTeamDefault()) diff --git a/engine/apps/api/views/user.py b/engine/apps/api/views/user.py index 01f6fc37..629990a6 100644 --- a/engine/apps/api/views/user.py +++ b/engine/apps/api/views/user.py @@ -647,7 +647,7 @@ class UserView( @action(detail=True, methods=["get"]) def check_availability(self, request, pk) -> Response: user = self.get_object() - warnings = check_user_availability(user=user, team=request.user.current_team) + warnings = check_user_availability(user=user) return Response(data={"warnings": warnings}, status=status.HTTP_200_OK) diff --git a/engine/apps/slack/scenarios/manual_incident.py b/engine/apps/slack/scenarios/manual_incident.py index bca16d62..20a5c340 100644 --- a/engine/apps/slack/scenarios/manual_incident.py +++ b/engine/apps/slack/scenarios/manual_incident.py @@ -20,6 +20,7 @@ DEFAULT_TEAM_VALUE = "default_team" class StartCreateIncidentFromSlashCommand(scenario_step.ScenarioStep): """ StartCreateIncidentFromSlashCommand triggers creation of a manual incident from the slack message via slash command + THIS FEATURE IS DEPRECATED AND WILL BE REMOVED IN A FUTURE RELEASE """ command_name = [settings.SLACK_SLASH_COMMAND_NAME] @@ -232,6 +233,18 @@ class OnRouteChange(scenario_step.ScenarioStep): def _get_manual_incident_form_view(routing_uid, blocks, private_metatada): + deprecation_blocks = [ + { + "type": "header", + "text": { + "type": "plain_text", + "text": f":no_entry: This command is deprecated and will be removed soon. Please use {settings.SLACK_DIRECT_PAGING_SLASH_COMMAND} command instead :no_entry:", + "emoji": True, + }, + }, + {"type": "divider"}, + ] + view = { "type": "modal", "callback_id": routing_uid, @@ -248,7 +261,7 @@ def _get_manual_incident_form_view(routing_uid, blocks, private_metatada): "type": "plain_text", "text": "Submit", }, - "blocks": blocks, + "blocks": deprecation_blocks + blocks, "private_metadata": private_metatada, } diff --git a/engine/apps/slack/scenarios/paging.py b/engine/apps/slack/scenarios/paging.py index ce9891dc..ba495db6 100644 --- a/engine/apps/slack/scenarios/paging.py +++ b/engine/apps/slack/scenarios/paging.py @@ -4,6 +4,7 @@ from uuid import uuid4 from django.apps import apps from django.conf import settings +from apps.alerts.models import AlertReceiveChannel, EscalationChain from apps.alerts.paging import ( USER_HAS_NO_NOTIFICATION_POLICY, USER_IS_NOT_ON_CALL, @@ -11,19 +12,17 @@ from apps.alerts.paging import ( direct_paging, ) from apps.slack.constants import PRIVATE_METADATA_MAX_LENGTH -from apps.slack.models import SlackChannel from apps.slack.scenarios import scenario_step from apps.slack.slack_client.exceptions import SlackAPIException DIRECT_PAGING_TEAM_SELECT_ID = "paging_team_select" DIRECT_PAGING_ORG_SELECT_ID = "paging_org_select" -DIRECT_PAGING_ESCALATION_SELECT_ID = "paging_escalation_select" DIRECT_PAGING_USER_SELECT_ID = "paging_user_select" DIRECT_PAGING_SCHEDULE_SELECT_ID = "paging_schedule_select" DIRECT_PAGING_TITLE_INPUT_ID = "paging_title_input" DIRECT_PAGING_MESSAGE_INPUT_ID = "paging_message_input" +DIRECT_PAGING_ADDITIONAL_RESPONDERS_INPUT_ID = "paging_additional_responders_input" -DEFAULT_NO_ESCALATION_VALUE = "default_no_escalation" DEFAULT_TEAM_VALUE = "default_team" @@ -121,22 +120,27 @@ class FinishDirectPaging(scenario_step.ScenarioStep): private_metadata = json.loads(payload["view"]["private_metadata"]) channel_id = private_metadata["channel_id"] input_id_prefix = private_metadata["input_id_prefix"] - selected_organization = _get_selected_org_from_payload(payload, input_id_prefix) - selected_team = _get_selected_team_from_payload(payload, input_id_prefix) - selected_escalation = _get_selected_escalation_from_payload(payload, input_id_prefix) + selected_organization = _get_selected_org_from_payload( + payload, input_id_prefix, slack_team_identity, slack_user_identity + ) + _, selected_team = _get_selected_team_from_payload(payload, input_id_prefix) user = slack_user_identity.get_user(selected_organization) - selected_users = [ - (u, p == IMPORTANT_POLICY) - for u, p in get_current_items(payload, USERS_DATA_KEY, selected_organization.users) - ] - selected_schedules = [ - (s, p == IMPORTANT_POLICY) - for s, p in get_current_items(payload, SCHEDULES_DATA_KEY, selected_organization.oncall_schedules) - ] + # Only pass users/schedules if additional responders checkbox is checked + selected_users, selected_schedules = None, None + is_additional_responders_checked = _get_additional_responders_checked_from_payload(payload, input_id_prefix) + if is_additional_responders_checked: + selected_users = [ + (u, p == IMPORTANT_POLICY) + for u, p in get_current_items(payload, USERS_DATA_KEY, selected_organization.users) + ] + selected_schedules = [ + (s, p == IMPORTANT_POLICY) + for s, p in get_current_items(payload, SCHEDULES_DATA_KEY, selected_organization.oncall_schedules) + ] - # trigger direct paging to selected users/schedules/escalation - direct_paging( + # trigger direct paging to selected team + users/schedules + alert_group = direct_paging( selected_organization, selected_team, user, @@ -144,15 +148,16 @@ class FinishDirectPaging(scenario_step.ScenarioStep): message, selected_users, selected_schedules, - selected_escalation, ) + text = ":white_check_mark: Alert group *{}* created: {}".format(title, alert_group.web_link) + try: self._slack_client.api_call( "chat.postEphemeral", channel=channel_id, user=slack_user_identity.slack_id, - text=":white_check_mark: Alert *{}* successfully submitted".format(title), + text=text, ) except SlackAPIException as e: if e.response["error"] == "channel_not_found": @@ -160,7 +165,7 @@ class FinishDirectPaging(scenario_step.ScenarioStep): "chat.postEphemeral", channel=slack_user_identity.im_channel_id, user=slack_user_identity.slack_id, - text=":white_check_mark: Alert *{}* successfully submitted".format(title), + text=text, ) else: raise e @@ -183,12 +188,21 @@ class OnPagingOrgChange(scenario_step.ScenarioStep): ) -class OnPagingTeamChange(OnPagingOrgChange): - """Reload form with updated team.""" +class OnPagingTeamChange(scenario_step.ScenarioStep): + """Set team.""" + + def process_scenario(self, slack_user_identity, slack_team_identity, payload): + view = render_dialog(slack_user_identity, slack_team_identity, payload) + self._slack_client.api_call( + "views.update", + trigger_id=payload["trigger_id"], + view=view, + view_id=payload["view"]["id"], + ) -class OnPagingEscalationChange(scenario_step.ScenarioStep): - """Set escalation chain.""" +class OnPagingCheckAdditionalResponders(OnPagingOrgChange): + """Check/uncheck additional responders checkbox.""" class OnPagingUserChange(scenario_step.ScenarioStep): @@ -199,14 +213,15 @@ class OnPagingUserChange(scenario_step.ScenarioStep): def process_scenario(self, slack_user_identity, slack_team_identity, payload): private_metadata = json.loads(payload["view"]["private_metadata"]) - selected_organization = _get_selected_org_from_payload(payload, private_metadata["input_id_prefix"]) - selected_team = _get_selected_team_from_payload(payload, private_metadata["input_id_prefix"]) + selected_organization = _get_selected_org_from_payload( + payload, private_metadata["input_id_prefix"], slack_team_identity, slack_user_identity + ) selected_user = _get_selected_user_from_payload(payload, private_metadata["input_id_prefix"]) if selected_user is None: return # check availability - availability_warnings = check_user_availability(selected_user, selected_team) + availability_warnings = check_user_availability(selected_user) if availability_warnings: # display warnings and require additional confirmation view = _display_availability_warnings(payload, availability_warnings, selected_organization, selected_user) @@ -335,93 +350,62 @@ DIVIDER_BLOCK = {"type": "divider"} def render_dialog(slack_user_identity, slack_team_identity, payload, initial=False, error_msg=None): private_metadata = json.loads(payload["view"]["private_metadata"]) submit_routing_uid = private_metadata.get("submit_routing_uid") + + # Get organizations available to user + available_organizations = _get_available_organizations(slack_team_identity, slack_user_identity) + if initial: # setup initial form new_input_id_prefix = _generate_input_id_prefix() new_private_metadata = private_metadata new_private_metadata["input_id_prefix"] = new_input_id_prefix - selected_organization = ( - slack_team_identity.organizations.filter(users__slack_user_identity=slack_user_identity) - .order_by("pk") - .distinct() - .first() - ) - selected_team = None - selected_escalation = None + selected_organization = available_organizations.first() + is_team_selected, selected_team = False, None + is_additional_responders_checked = False else: # setup form using data/state old_input_id_prefix, new_input_id_prefix, new_private_metadata = _get_and_change_input_id_prefix_from_metadata( private_metadata ) - selected_organization = _get_selected_org_from_payload(payload, old_input_id_prefix) - selected_team = _get_selected_team_from_payload(payload, old_input_id_prefix) - selected_escalation = _get_selected_escalation_from_payload(payload, old_input_id_prefix) + selected_organization = _get_selected_org_from_payload( + payload, old_input_id_prefix, slack_team_identity, slack_user_identity + ) + is_team_selected, selected_team = _get_selected_team_from_payload(payload, old_input_id_prefix) + is_additional_responders_checked = _get_additional_responders_checked_from_payload(payload, old_input_id_prefix) # widgets - organization_select = _get_organization_select( - slack_team_identity, slack_user_identity, selected_organization, new_input_id_prefix + team_select_blocks = _get_team_select_blocks( + slack_user_identity, selected_organization, is_team_selected, selected_team, new_input_id_prefix ) - team_select = _get_team_select(slack_user_identity, selected_organization, selected_team, new_input_id_prefix) - escalation_select = _get_escalation_select( - selected_organization, selected_team, selected_escalation, new_input_id_prefix + additional_responders_blocks = _get_additional_responders_blocks( + payload, selected_organization, new_input_id_prefix, is_additional_responders_checked, error_msg ) - users_select = _get_users_select(selected_organization, selected_team, new_input_id_prefix) - schedules_select = _get_schedules_select(selected_organization, selected_team, new_input_id_prefix) - # blocks - blocks = [organization_select, team_select, escalation_select, users_select, schedules_select] + # Add title and message inputs + blocks = [_get_title_input(payload), _get_message_input(payload)] - if error_msg: - blocks += [ - { - "type": "section", - "block_id": "error_message", - "text": { - "type": "mrkdwn", - "text": f":warning: {error_msg}", - }, - } - ] + # Add organization select if more than one organization available for user + if len(available_organizations) > 1: + organization_select = _get_organization_select( + available_organizations, selected_organization, new_input_id_prefix + ) + blocks.append(organization_select) - # selected items - selected_users = get_current_items(payload, USERS_DATA_KEY, selected_organization.users) - selected_schedules = get_current_items(payload, SCHEDULES_DATA_KEY, selected_organization.oncall_schedules) + # Add team select and additional responders blocks + blocks += team_select_blocks + blocks += additional_responders_blocks - if selected_users or selected_schedules: - blocks += [DIVIDER_BLOCK] - blocks.extend(_get_selected_entries_list(new_input_id_prefix, USERS_DATA_KEY, selected_users)) - blocks.extend(_get_selected_entries_list(new_input_id_prefix, SCHEDULES_DATA_KEY, selected_schedules)) - blocks += [DIVIDER_BLOCK] - - blocks.extend([_get_title_input(payload), _get_message_input(payload)]) - - view = _get_form_view(submit_routing_uid, blocks, json.dumps(new_private_metadata), selected_organization) + view = _get_form_view(submit_routing_uid, blocks, json.dumps(new_private_metadata)) return view -def _get_form_view(routing_uid, blocks, private_metatada, organization): - try: - channel = organization.slack_team_identity.get_cached_channels().get( - slack_id=organization.general_log_channel_id - ) - additional_info = f":information_source: The alert group will be posted to the #{channel.name} Slack channel" - except SlackChannel.DoesNotExist: - additional_info = ( - ":information_source: The alert group will be posted to the default Slack channel if there is one setup" - ) - - blocks += [ - { - "type": "context", - "elements": [{"type": "mrkdwn", "text": additional_info}], - } - ] +def _get_form_view(routing_uid, blocks, private_metadata): view = { "type": "modal", "callback_id": routing_uid, "title": { "type": "plain_text", - "text": "Create alert group", + "text": "Create Alert Group", }, "close": { "type": "plain_text", @@ -430,19 +414,16 @@ def _get_form_view(routing_uid, blocks, private_metatada, organization): }, "submit": { "type": "plain_text", - "text": "Submit", + "text": "Create", }, "blocks": blocks, - "private_metadata": private_metatada, + "private_metadata": private_metadata, } return view -def _get_organization_select(slack_team_identity, slack_user_identity, value, input_id_prefix): - organizations = slack_team_identity.organizations.filter( - users__slack_user_identity=slack_user_identity, - ).distinct() +def _get_organization_select(organizations, value, input_id_prefix): organizations_options = [] initial_option_idx = 0 for idx, org in enumerate(organizations): @@ -452,7 +433,7 @@ def _get_organization_select(slack_team_identity, slack_user_identity, value, in { "text": { "type": "plain_text", - "text": f"{org.stack_slug}", + "text": f"{org.org_title}", "emoji": True, }, "value": f"{org.pk}", @@ -460,41 +441,50 @@ def _get_organization_select(slack_team_identity, slack_user_identity, value, in ) organization_select = { - "type": "section", - "text": {"type": "mrkdwn", "text": "Select an organization"}, + "type": "input", "block_id": input_id_prefix + DIRECT_PAGING_ORG_SELECT_ID, - "accessory": { + "label": { + "type": "plain_text", + "text": "Organization", + }, + "element": { "type": "static_select", - "placeholder": {"type": "plain_text", "text": "Select an organization", "emoji": True}, + "placeholder": {"type": "plain_text", "text": "Organization", "emoji": True}, "options": organizations_options, "action_id": OnPagingOrgChange.routing_uid(), "initial_option": organizations_options[initial_option_idx], }, + "dispatch_action": True, } return organization_select def _get_select_field_value(payload, prefix_id, routing_uid, field_id): - field = payload["view"]["state"]["values"][prefix_id + field_id][routing_uid]["selected_option"] + try: + field = payload["view"]["state"]["values"][prefix_id + field_id][routing_uid]["selected_option"] + except KeyError: + return None + if field: return field["value"] -def _get_selected_org_from_payload(payload, input_id_prefix): +def _get_selected_org_from_payload(payload, input_id_prefix, slack_team_identity, slack_user_identity): Organization = apps.get_model("user_management", "Organization") selected_org_id = _get_select_field_value( payload, input_id_prefix, OnPagingOrgChange.routing_uid(), DIRECT_PAGING_ORG_SELECT_ID ) - if selected_org_id is not None: + if selected_org_id is None: + return _get_available_organizations(slack_team_identity, slack_user_identity).first() + else: org = Organization.objects.filter(pk=selected_org_id).first() return org -def _get_team_select(slack_user_identity, organization, value, input_id_prefix): - teams = organization.teams.filter( - users__slack_user_identity=slack_user_identity, - ).distinct() +def _get_team_select_blocks(slack_user_identity, organization, is_selected, value, input_id_prefix): + user = slack_user_identity.get_user(organization) # TODO: handle None + teams = user.available_teams team_options = [] # Adding pseudo option for default team @@ -503,7 +493,7 @@ def _get_team_select(slack_user_identity, organization, value, input_id_prefix): { "text": { "type": "plain_text", - "text": f"General", + "text": f"No team", "emoji": True, }, "value": DEFAULT_TEAM_VALUE, @@ -524,73 +514,133 @@ def _get_team_select(slack_user_identity, organization, value, input_id_prefix): ) team_select = { - "type": "section", - "text": {"type": "mrkdwn", "text": "Select a team"}, + "type": "input", "block_id": input_id_prefix + DIRECT_PAGING_TEAM_SELECT_ID, - "accessory": { + "label": { + "type": "plain_text", + "text": "Team to notify", + }, + "element": { "type": "static_select", - "placeholder": {"type": "plain_text", "text": "Select a team", "emoji": True}, - "options": team_options, "action_id": OnPagingTeamChange.routing_uid(), - "initial_option": team_options[initial_option_idx], + "placeholder": {"type": "plain_text", "text": "Select team", "emoji": True}, + "options": team_options, + }, + "dispatch_action": True, + } + + # No context block if no team selected + if not is_selected: + return [team_select] + + team_select["element"]["initial_option"] = team_options[initial_option_idx] + return [team_select, _get_team_select_context(organization, value)] + + +def _get_team_select_context(organization, team): + team_name = team.name if team else "No team" + alert_receive_channel = AlertReceiveChannel.objects.filter( + organization=organization, + team=team, + integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING, + ).first() + + escalation_chains_exist = EscalationChain.objects.filter( + channel_filters__alert_receive_channel=alert_receive_channel + ).exists() + + if not alert_receive_channel: + context_text = ( + ":warning: *Direct paging integration missing*\n" + "The selected team doesn't have a direct paging integration configured and will not be notified. " + "If you proceed with the alert group, an empty direct paging integration will be created automatically for the team. " + "" + ) + elif not escalation_chains_exist: + context_text = ( + ":warning: *Direct paging integration not configured*\n" + "The direct paging integration for the selected team has no escalation chains configured. " + "If you proceed with the alert group, the team likely will not be notified. " + "" + ) + else: + context_text = f"Integration <{alert_receive_channel.web_link}|{alert_receive_channel.verbal_name} ({team_name})> will be used for notification." + + context = { + "type": "context", + "elements": [ + { + "type": "mrkdwn", + "text": context_text, + } + ], + } + return context + + +def _get_additional_responders_blocks( + payload, organization, input_id_prefix, is_additional_responders_checked, error_msg +): + checkbox_option = { + "text": { + "type": "plain_text", + "text": "Notify additional responders", }, } - return team_select - -def _get_escalation_select(organization, team, value, input_id_prefix): - escalations = organization.escalation_chains.filter(team=team) - # adding a default no-escalation option - initial_option_idx = 0 - options = [ + blocks = [ { - "text": { + "type": "input", + "block_id": input_id_prefix + DIRECT_PAGING_ADDITIONAL_RESPONDERS_INPUT_ID, + "label": { "type": "plain_text", - "text": f"None", - "emoji": True, + "text": "Additional responders", }, - "value": DEFAULT_NO_ESCALATION_VALUE, + "element": { + "type": "checkboxes", + "options": [checkbox_option], + "action_id": OnPagingCheckAdditionalResponders.routing_uid(), + }, + "optional": True, + "dispatch_action": True, } ] - for idx, escalation in enumerate(escalations, start=1): - if escalation == value: - initial_option_idx = idx - options.append( + + if is_additional_responders_checked: + blocks[0]["element"]["initial_options"] = [checkbox_option] + + if error_msg: + blocks += [ { + "type": "section", + "block_id": "error_message", "text": { - "type": "plain_text", - "text": f"{escalation.name}", - "emoji": True, + "type": "mrkdwn", + "text": f":warning: {error_msg}", }, - "value": f"{escalation.pk}", } - ) + ] - if not options: - escalations_select = { - "type": "context", - "elements": [{"type": "mrkdwn", "text": "No escalation chains available"}], - } - else: - escalations_select = { - "type": "section", - "text": {"type": "mrkdwn", "text": "Set escalation chain"}, - "block_id": input_id_prefix + DIRECT_PAGING_ESCALATION_SELECT_ID, - "accessory": { - "type": "static_select", - "placeholder": {"type": "plain_text", "text": "Select an escalation", "emoji": True}, - "options": options, - "action_id": OnPagingEscalationChange.routing_uid(), - "initial_option": options[initial_option_idx], - }, - } - return escalations_select + if is_additional_responders_checked: + users_select = _get_users_select(organization, input_id_prefix) + schedules_select = _get_schedules_select(organization, input_id_prefix) + + blocks += [users_select, schedules_select] + # selected items + selected_users = get_current_items(payload, USERS_DATA_KEY, organization.users) + selected_schedules = get_current_items(payload, SCHEDULES_DATA_KEY, organization.oncall_schedules) + + if selected_users or selected_schedules: + blocks += [DIVIDER_BLOCK] + blocks += _get_selected_entries_list(input_id_prefix, USERS_DATA_KEY, selected_users) + blocks += _get_selected_entries_list(input_id_prefix, SCHEDULES_DATA_KEY, selected_schedules) + blocks += [DIVIDER_BLOCK] + + return blocks -def _get_users_select(organization, team, input_id_prefix): +def _get_users_select(organization, input_id_prefix): users = organization.users.all() - if team is not None: - users = users.filter(teams=team) user_options = [ { @@ -609,7 +659,7 @@ def _get_users_select(organization, team, input_id_prefix): user_select = { "type": "section", - "text": {"type": "mrkdwn", "text": "Add responders"}, + "text": {"type": "mrkdwn", "text": "Add users"}, "block_id": input_id_prefix + DIRECT_PAGING_USER_SELECT_ID, "accessory": { "type": "static_select", @@ -637,8 +687,8 @@ def _get_users_select(organization, team, input_id_prefix): return user_select -def _get_schedules_select(organization, team, input_id_prefix): - schedules = organization.oncall_schedules.filter(team=team) +def _get_schedules_select(organization, input_id_prefix): + schedules = organization.oncall_schedules.all() schedule_options = [ { @@ -753,21 +803,26 @@ def _get_selected_team_from_payload(payload, input_id_prefix): selected_team_id = _get_select_field_value( payload, input_id_prefix, OnPagingTeamChange.routing_uid(), DIRECT_PAGING_TEAM_SELECT_ID ) - if selected_team_id is None or selected_team_id == DEFAULT_TEAM_VALUE: - return None + + if selected_team_id is None: + return None, None + + if selected_team_id == DEFAULT_TEAM_VALUE: + return selected_team_id, None + team = Team.objects.filter(pk=selected_team_id).first() - return team + return selected_team_id, team -def _get_selected_escalation_from_payload(payload, input_id_prefix): - EscalationChain = apps.get_model("alerts", "EscalationChain") - selected_escalation_id = _get_select_field_value( - payload, input_id_prefix, OnPagingEscalationChange.routing_uid(), DIRECT_PAGING_ESCALATION_SELECT_ID - ) - if selected_escalation_id is None or selected_escalation_id == DEFAULT_NO_ESCALATION_VALUE: - return None - escalation = EscalationChain.objects.filter(pk=selected_escalation_id).first() - return escalation +def _get_additional_responders_checked_from_payload(payload, input_id_prefix): + try: + selected_options = payload["view"]["state"]["values"][ + input_id_prefix + DIRECT_PAGING_ADDITIONAL_RESPONDERS_INPUT_ID + ][OnPagingCheckAdditionalResponders.routing_uid()]["selected_options"] + except KeyError: + return False + + return len(selected_options) > 0 def _get_selected_user_from_payload(payload, input_id_prefix): @@ -803,7 +858,7 @@ def _get_title_input(payload): "block_id": DIRECT_PAGING_TITLE_INPUT_ID, "label": { "type": "plain_text", - "text": "Title:", + "text": "Title", }, "element": { "type": "plain_text_input", @@ -830,7 +885,7 @@ def _get_message_input(payload): "block_id": DIRECT_PAGING_MESSAGE_INPUT_ID, "label": { "type": "plain_text", - "text": "Message:", + "text": "Message", }, "element": { "type": "plain_text_input", @@ -856,6 +911,14 @@ def _get_message_from_payload(payload): return message +def _get_available_organizations(slack_team_identity, slack_user_identity): + return ( + slack_team_identity.organizations.filter(users__slack_user_identity=slack_user_identity) + .order_by("pk") + .distinct() + ) + + # _generate_input_id_prefix returns uniq str to not to preserve input's values between view update # https://api.slack.com/methods/views.update#markdown def _generate_input_id_prefix(): @@ -877,9 +940,9 @@ STEPS_ROUTING = [ }, { "payload_type": scenario_step.PAYLOAD_TYPE_BLOCK_ACTIONS, - "block_action_type": scenario_step.BLOCK_ACTION_TYPE_STATIC_SELECT, - "block_action_id": OnPagingEscalationChange.routing_uid(), - "step": OnPagingEscalationChange, + "block_action_type": scenario_step.BLOCK_ACTION_TYPE_CHECKBOXES, + "block_action_id": OnPagingCheckAdditionalResponders.routing_uid(), + "step": OnPagingCheckAdditionalResponders, }, { "payload_type": scenario_step.PAYLOAD_TYPE_BLOCK_ACTIONS, diff --git a/engine/apps/slack/scenarios/scenario_step.py b/engine/apps/slack/scenarios/scenario_step.py index 38de8d0c..cdeac0e7 100644 --- a/engine/apps/slack/scenarios/scenario_step.py +++ b/engine/apps/slack/scenarios/scenario_step.py @@ -46,6 +46,7 @@ BLOCK_ACTION_TYPE_CONVERSATIONS_SELECT = "conversations_select" BLOCK_ACTION_TYPE_CHANNELS_SELECT = "channels_select" BLOCK_ACTION_TYPE_OVERFLOW = "overflow" BLOCK_ACTION_TYPE_DATEPICKER = "datepicker" +BLOCK_ACTION_TYPE_CHECKBOXES = "checkboxes" PAYLOAD_TYPE_DIALOG_SUBMISSION = "dialog_submission" PAYLOAD_TYPE_VIEW_SUBMISSION = "view_submission" diff --git a/engine/apps/slack/tests/test_interactive_api_endpoint.py b/engine/apps/slack/tests/test_interactive_api_endpoint.py index f3e21a80..23a4437b 100644 --- a/engine/apps/slack/tests/test_interactive_api_endpoint.py +++ b/engine/apps/slack/tests/test_interactive_api_endpoint.py @@ -6,6 +6,7 @@ from django.conf import settings from rest_framework import status from rest_framework.test import APIClient +from apps.slack.scenarios.paging import OnPagingTeamChange from apps.slack.scenarios.scenario_step import PAYLOAD_TYPE_BLOCK_ACTIONS EVENT_TRIGGER_ID = "5333959822612.4122782784722.4734ff484b2ac4d36a185bb242ee9932" @@ -131,3 +132,35 @@ def test_organization_not_found_scenario_doesnt_break_slash_commands( assert response.status_code == status.HTTP_200_OK mock_open_warning_window_if_needed.assert_not_called() + + +@patch("apps.slack.views.SlackEventApiEndpointView.verify_signature", return_value=True) +@patch.object(OnPagingTeamChange, "process_scenario") +@pytest.mark.django_db +def test_organization_not_found_scenario_doesnt_break_direct_paging( + mock_on_paging_team_change, + _, + make_organization, + make_slack_user_identity, + make_user, + slack_team_identity, +): + """ + Check OnPagingTeamChange.process_scenario gets called when a user changes the team in direct paging dialog. + """ + organization = make_organization(slack_team_identity=slack_team_identity) + slack_user_identity = make_slack_user_identity(slack_team_identity=slack_team_identity, slack_id=SLACK_USER_ID) + make_user(organization=organization, slack_user_identity=slack_user_identity) + + response = _make_request( + { + "team_id": SLACK_TEAM_ID, + "user_id": SLACK_USER_ID, + "type": "block_actions", + "actions": [{"action_id": OnPagingTeamChange.routing_uid(), "type": "static_select"}], + "view": {"type": "modal"}, + } + ) + + assert response.status_code == status.HTTP_200_OK + mock_on_paging_team_change.assert_called_once() 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 bb7b7cac..6aa0d2fe 100644 --- a/engine/apps/slack/tests/test_scenario_steps/test_paging.py +++ b/engine/apps/slack/tests/test_scenario_steps/test_paging.py @@ -8,7 +8,7 @@ from apps.base.models import UserNotificationPolicy from apps.schedules.models import CustomOnCallShift, OnCallScheduleWeb from apps.slack.scenarios.paging import ( DEFAULT_POLICY, - DIRECT_PAGING_ESCALATION_SELECT_ID, + DIRECT_PAGING_ADDITIONAL_RESPONDERS_INPUT_ID, DIRECT_PAGING_MESSAGE_INPUT_ID, DIRECT_PAGING_ORG_SELECT_ID, DIRECT_PAGING_SCHEDULE_SELECT_ID, @@ -20,7 +20,7 @@ from apps.slack.scenarios.paging import ( SCHEDULES_DATA_KEY, USERS_DATA_KEY, FinishDirectPaging, - OnPagingEscalationChange, + OnPagingCheckAdditionalResponders, OnPagingItemActionChange, OnPagingOrgChange, OnPagingScheduleChange, @@ -31,7 +31,14 @@ from apps.slack.scenarios.paging import ( def make_slack_payload( - organization, user=None, schedule=None, escalation=None, current_users=None, current_schedules=None, actions=None + organization, + team=None, + user=None, + schedule=None, + additional_responders=False, + current_users=None, + current_schedules=None, + actions=None, ): payload = { "channel_id": "123", @@ -52,10 +59,12 @@ def make_slack_payload( DIRECT_PAGING_ORG_SELECT_ID: { OnPagingOrgChange.routing_uid(): {"selected_option": {"value": organization.pk}} }, - DIRECT_PAGING_TEAM_SELECT_ID: {OnPagingTeamChange.routing_uid(): {"selected_option": {"value": 0}}}, - DIRECT_PAGING_ESCALATION_SELECT_ID: { - OnPagingEscalationChange.routing_uid(): { - "selected_option": {"value": escalation.pk} if escalation else None + DIRECT_PAGING_TEAM_SELECT_ID: { + OnPagingTeamChange.routing_uid(): {"selected_option": {"value": team.pk if team else None}} + }, + DIRECT_PAGING_ADDITIONAL_RESPONDERS_INPUT_ID: { + OnPagingCheckAdditionalResponders.routing_uid(): { + "selected_options": ["something"] if additional_responders else [] } }, DIRECT_PAGING_USER_SELECT_ID: { @@ -263,13 +272,34 @@ def test_trigger_paging_no_responders(make_organization_and_user_with_slack_iden @pytest.mark.django_db -def test_trigger_paging(make_organization_and_user_with_slack_identities, make_escalation_chain, make_schedule): +def test_trigger_paging(make_organization_and_user_with_slack_identities, make_team, make_schedule): organization, user, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities() - schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb, team=None) - escalation = make_escalation_chain(organization) + team = make_team(organization) payload = make_slack_payload( organization=organization, - escalation=escalation, + team=team, + additional_responders=False, + ) + + step = FinishDirectPaging(slack_team_identity) + with patch("apps.slack.scenarios.paging.direct_paging") as mock_direct_paging: + with patch.object(step._slack_client, "api_call"): + step.process_scenario(slack_user_identity, slack_team_identity, payload) + + assert mock_direct_paging.called_with(organization, team, user, "The Title", "The Message", [], [], None) + + +@pytest.mark.django_db +def test_trigger_paging_additional_responders( + make_organization_and_user_with_slack_identities, make_team, make_schedule +): + organization, user, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities() + team = make_team(organization) + schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb, team=None) + payload = make_slack_payload( + organization=organization, + team=team, + additional_responders=True, current_users={str(user.pk): IMPORTANT_POLICY}, current_schedules={str(schedule.pk): DEFAULT_POLICY}, ) @@ -280,7 +310,7 @@ def test_trigger_paging(make_organization_and_user_with_slack_identities, make_e step.process_scenario(slack_user_identity, slack_team_identity, payload) assert mock_direct_paging.called_with( - organization, None, user, "The Title", "The Message", [(user, True)], [(schedule, False)], escalation + organization, team, user, "The Title", "The Message", [(user, True)], [(schedule, False)], None ) diff --git a/engine/apps/slack/views.py b/engine/apps/slack/views.py index da43077c..f90efea7 100644 --- a/engine/apps/slack/views.py +++ b/engine/apps/slack/views.py @@ -300,7 +300,8 @@ class SlackEventApiEndpointView(APIView): # Open pop-up to inform user why OnCall bot doesn't work if any action was triggered self._open_warning_window_if_needed(payload, slack_team_identity, warning_text) return Response(status=200) - elif organization is None and payload_type_is_block_actions: + # direct paging / manual incident dialogs don't require organization to be set + elif organization is None and payload_type_is_block_actions and not payload.get("view"): # see this GitHub issue for more context on how this situation can arise # https://github.com/grafana/oncall-private/issues/1836 warning_text = ( diff --git a/grafana-plugin/integration-tests/alerts/directPaging.test.ts b/grafana-plugin/integration-tests/alerts/directPaging.test.ts new file mode 100644 index 00000000..122e5be9 --- /dev/null +++ b/grafana-plugin/integration-tests/alerts/directPaging.test.ts @@ -0,0 +1,27 @@ +import { test } from '../fixtures'; +import { clickButton, fillInInput, selectDropdownValue } from '../utils/forms'; +import { goToOnCallPage } from "../utils/navigation"; +import { verifyAlertGroupTitleAndMessageContainText } from "../utils/alertGroup"; + +test('we can create an alert group for default team', async ({ adminRolePage }) => { + const { page } = adminRolePage; + + await goToOnCallPage(page, 'alert-groups'); + await clickButton({ page, buttonText: 'New alert group' }); + + await fillInInput(page, 'input[name="title"]', "Help me!"); + await fillInInput(page, 'textarea[name="message"]', "Help me please!"); + + await selectDropdownValue({ + page, + selectType: 'grafanaSelect', + placeholderText: "Select team", + value: "No team", + }); + + await clickButton({ page, buttonText: 'Create' }); + + // Check we are redirected to the alert group page + await page.waitForURL('**/alert-groups/I*'); // Alert group IDs always start with "I" + await verifyAlertGroupTitleAndMessageContainText(page, "Help me!", "Help me please!") +}); diff --git a/grafana-plugin/integration-tests/utils/alertGroup.ts b/grafana-plugin/integration-tests/utils/alertGroup.ts index 1a72d941..2e415172 100644 --- a/grafana-plugin/integration-tests/utils/alertGroup.ts +++ b/grafana-plugin/integration-tests/utils/alertGroup.ts @@ -42,7 +42,7 @@ export const filterAlertGroupsTableByIntegrationAndGoToDetailPage = async ( throw new Error('we were not able to properly filter the alert groups table by integration'); } - await goToOnCallPage(page, 'incidents'); + await goToOnCallPage(page, 'alert-groups'); // filter by integration const selectElement = await selectDropdownValue({ @@ -100,3 +100,13 @@ export const verifyThatAlertGroupIsTriggered = async ( expect(await incidentTimelineContainsStep(page, triggeredStepText)).toBe(true); }; + + +export const verifyAlertGroupTitleAndMessageContainText = async ( + page: Page, + title: string, + message: string +): Promise => { + await expect(page.getByTestId('incident-title')).toContainText(title); + await expect(page.getByTestId('incident-message')).toContainText(message); +}; diff --git a/grafana-plugin/integration-tests/utils/navigation.ts b/grafana-plugin/integration-tests/utils/navigation.ts index 662f7a34..f75eb046 100644 --- a/grafana-plugin/integration-tests/utils/navigation.ts +++ b/grafana-plugin/integration-tests/utils/navigation.ts @@ -2,7 +2,7 @@ import type { Page, Response } from '@playwright/test'; import { BASE_URL } from './constants'; type GrafanaPage = '/plugins/grafana-oncall-app'; -type OnCallPage = 'incidents' | 'integrations' | 'escalations' | 'schedules' | 'users'; +type OnCallPage = 'alert-groups' | 'integrations' | 'escalations' | 'schedules' | 'users'; const _goToPage = (page: Page, url = ''): Promise => page.goto(`${BASE_URL}${url}`); diff --git a/grafana-plugin/src/components/ManualAlertGroup/ManualAlertGroup.config.ts b/grafana-plugin/src/components/ManualAlertGroup/ManualAlertGroup.config.ts index e4a5f0cd..35459f32 100644 --- a/grafana-plugin/src/components/ManualAlertGroup/ManualAlertGroup.config.ts +++ b/grafana-plugin/src/components/ManualAlertGroup/ManualAlertGroup.config.ts @@ -12,8 +12,8 @@ export const manualAlertFormConfig: { name: string; fields: FormItem[] } = { { name: 'message', type: FormItemType.TextArea, - label: 'Description', - validation: { required: true }, + label: 'Message (optional)', + validation: { required: false }, }, ], }; diff --git a/grafana-plugin/src/components/ManualAlertGroup/ManualAlertGroup.tsx b/grafana-plugin/src/components/ManualAlertGroup/ManualAlertGroup.tsx index 0c4f2342..2c8ef9bb 100644 --- a/grafana-plugin/src/components/ManualAlertGroup/ManualAlertGroup.tsx +++ b/grafana-plugin/src/components/ManualAlertGroup/ManualAlertGroup.tsx @@ -113,7 +113,7 @@ const ManualAlertGroup: FC = (props) => { ); const DirectPagingIntegrationVariants = ({ selectedTeamId, selectedTeamDirectPaging, chatOpsAvailableChannels }) => { - const escalationChainsExist = selectedTeamDirectPaging?.connected_escalations_chains_count === 0; + const escalationChainsExist = selectedTeamDirectPaging?.connected_escalations_chains_count !== 0; return ( @@ -122,41 +122,32 @@ const ManualAlertGroup: FC = (props) => { ) : selectedTeamDirectPaging ? ( - +
  • - {escalationChainsExist && ( - - - - )} {selectedTeamDirectPaging.verbal_name} Team: - - {chatOpsAvailableChannels && ( - <> - {chatOpsAvailableChannels.map( - (chatOpsChannel: { name: string; icon: IconName }, chatOpsIndex) => ( -
    - {chatOpsChannel.icon && } - {chatOpsChannel.name || ''} -
    - ) - )} - {chatOpsAvailableChannels && ( - - - - )} - - )} -     + {chatOpsAvailableChannels.length && ( + + {chatOpsAvailableChannels.map( + (chatOpsChannel: { name: string; icon: IconName }, chatOpsIndex) => ( +
    + {chatOpsChannel.icon && } + {chatOpsChannel.name || ''} +
    + ) + )} + + + +     + )} = (props) => {
- - {(escalationChainsExist || !chatOpsAvailableChannels) && ( - + {!escalationChainsExist && ( + - {escalationChainsExist && ( - - Integration doesn't have connected escalation policies. Consider adding responders manually by - user or by email - - )} - {!chatOpsAvailableChannels && ( - Integration doesn't have connected ChatOps channels in messengers. - )} + + The direct paging integration for the selected team has no escalation chains configured. +
+ If you proceed with the alert group, the team likely will not be notified.
+ + Learn more. + + )} ) : ( - + - Empty integration for this team will be created automatically. Consider selecting responders by - schedule or user below + The selected team doesn't have a direct paging integration configured and will not be notified.
+ If you proceed with the alert group, an empty direct paging integration will be created automatically + for the team.
+ + Learn more. + @@ -200,22 +203,11 @@ const ManualAlertGroup: FC = (props) => { ); }; - const submitButtonDisabled = !( - selectedTeamId && - (selectedTeamDirectPaging || userResponders.length || scheduleResponders.length) - ); - return ( - + - + = (props) => { - diff --git a/grafana-plugin/src/containers/EscalationVariants/EscalationVariants.tsx b/grafana-plugin/src/containers/EscalationVariants/EscalationVariants.tsx index c03b19b8..6aa0f9a1 100644 --- a/grafana-plugin/src/containers/EscalationVariants/EscalationVariants.tsx +++ b/grafana-plugin/src/containers/EscalationVariants/EscalationVariants.tsx @@ -105,7 +105,7 @@ const EscalationVariants = observer(
{!hideSelected && Boolean(value.userResponders.length || value.scheduleResponders.length) && ( <> - +
    {value.userResponders.map((responder, index) => ( )}
    - {withLabels && } + {withLabels && }
    diff --git a/grafana-plugin/src/containers/GrafanaTeamSelect/GrafanaTeamSelect.tsx b/grafana-plugin/src/containers/GrafanaTeamSelect/GrafanaTeamSelect.tsx index 4f7bebe0..2e1db921 100644 --- a/grafana-plugin/src/containers/GrafanaTeamSelect/GrafanaTeamSelect.tsx +++ b/grafana-plugin/src/containers/GrafanaTeamSelect/GrafanaTeamSelect.tsx @@ -52,10 +52,11 @@ const GrafanaTeamSelect = observer(({ onSelect, onHide, withoutModal, defaultVal const select = ( {/* @ts-ignore*/} - + #{incident.inside_organization_number} {incident.render_for_web.title} {incident.root_alert_group && ( @@ -647,6 +647,7 @@ function Incident({ incident }: { incident: Alert; datetimeReference: string }) dangerouslySetInnerHTML={{ __html: sanitize(incident.render_for_web.message), }} + data-testid="incident-message" /> {incident.render_for_web.image_url && }
diff --git a/grafana-plugin/src/pages/incidents/Incidents.tsx b/grafana-plugin/src/pages/incidents/Incidents.tsx index f26f8bb4..4c8e5895 100644 --- a/grafana-plugin/src/pages/incidents/Incidents.tsx +++ b/grafana-plugin/src/pages/incidents/Incidents.tsx @@ -126,7 +126,7 @@ class Incidents extends React.Component Alert Groups