From f4cabe736057457fd3922629f841753540b32a61 Mon Sep 17 00:00:00 2001 From: Alfredo <109958902+alfredo-d@users.noreply.github.com> Date: Mon, 17 Jul 2023 19:21:17 +0800 Subject: [PATCH 1/8] Docs: Update oncall rbac detail (#2494) # What this PR does - Update docs to callout permission requirement in mobile app doc for responding to alert group. - Update link to RBAC fixed role doc for mapping between OnCall RBAC fixed role and Grafana basic role. --- docs/sources/mobile-app/alert-groups-feed/index.md | 11 +++++++++-- docs/sources/user-and-team-management/_index.md | 1 + 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/docs/sources/mobile-app/alert-groups-feed/index.md b/docs/sources/mobile-app/alert-groups-feed/index.md index 4901caca..93fb682a 100644 --- a/docs/sources/mobile-app/alert-groups-feed/index.md +++ b/docs/sources/mobile-app/alert-groups-feed/index.md @@ -17,9 +17,16 @@ On the **Feed** page, you can view Alert groups, under two tabs: - **Mine** shows Alert Groups that involve you in one way or another. E.g. because a notification went to you about it, or you resolved it. - **All** shows all Alert Groups, including ones that may not be relevant to you. -You can filter by status via the **filter** button on the top right. We are working on an expansion for this filter, to filter by team, integration name, and more. +You can filter by status via the **filter** button on the top right. We are working on an expansion for this filter, to filter by team, integration name, and more. -Tap on any Alert Group to go to the detailed view. This page also provides links for opening in slack and sharing the link to this alert group. +Tap on any Alert Group to go to the detailed view. +From this page, you have various options available to you. +You can open the alert group in Slack for further discussion and collaboration, as well as share the link to this specific alert group with others. +Additionally, you can take action on the alert group directly from this page. You have the ability to acknowledge, resolve, and silence the alert group. + +> **Note:** You need to have sufficient permission to take action on the alert group. +> To learn more about Grafana OnCall user roles and permission, +> refer to [this documentation]({{< relref "../../user-and-team-management#user-roles-and-permissions" >}}). diff --git a/docs/sources/user-and-team-management/_index.md b/docs/sources/user-and-team-management/_index.md index 8dd416a5..40ee6923 100644 --- a/docs/sources/user-and-team-management/_index.md +++ b/docs/sources/user-and-team-management/_index.md @@ -53,6 +53,7 @@ To learn more about RBAC for Grafana OnCall, refer to the following documentatio - [Manage RBAC roles](https://grafana.com/docs/grafana/latest/administration/roles-and-permissions/access-control/manage-rbac-roles/#update-basic-role-permissions) - [RBAC permissions, actions, and scopes](https://grafana.com/docs/grafana/latest/administration/roles-and-permissions/access-control/custom-role-actions-scopes/) +- [RBAC role definitions](https://grafana.com/docs/grafana/latest/administration/roles-and-permissions/access-control/rbac-fixed-basic-role-definitions/#grafana-oncall-roles-beta) #### Available Grafana OnCall RBAC roles + granted actions From cae8387f0b690dd39778eab271f7ad519b0acf66 Mon Sep 17 00:00:00 2001 From: Zach Day Date: Mon, 17 Jul 2023 06:22:43 -0500 Subject: [PATCH 2/8] Update Users.tsx (#2539) Update "Grafana user management" url from `/org/users` to `/admin/users` on the OnCall Users page. # What this PR does Currently in my Grafana Cloud instance when I click the link for "Grafana user management" on the Users page of the OnCall app it takes me to the `/org/users` page, which shows a 404 error (but also kinda loads). 2023-07-14_13-00-06 ## Which issue(s) this PR fixes Discussed in Slack here: https://raintank-corp.slack.com/archives/C0229FD3CE9/p1689006468472659 In my cloud instance the users page is found on `/admin/users`. 2023-07-14_13-01-33 ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- grafana-plugin/src/pages/users/Users.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grafana-plugin/src/pages/users/Users.tsx b/grafana-plugin/src/pages/users/Users.tsx index 2284e9a5..31df49ca 100644 --- a/grafana-plugin/src/pages/users/Users.tsx +++ b/grafana-plugin/src/pages/users/Users.tsx @@ -192,7 +192,7 @@ class Users extends React.Component { All Grafana users listed below to set notification preferences. To manage permissions or add new users, please visit{' '} - + Grafana user management From 8644883883c6b8809d50be6e9c674e763fb62c1c Mon Sep 17 00:00:00 2001 From: Andrey Oleynik Date: Mon, 17 Jul 2023 16:04:55 +0300 Subject: [PATCH 3/8] set default phone provider (#2523) # What this PR does Sets a default value for the PHONE_PROVIDER setting and replaces the value of PHONE_PROVIDER with this default value if it is not valid. ## Which issue(s) this PR fixes - [#2520](https://github.com/grafana/oncall/issues/2520) - [#2323](https://github.com/grafana/oncall/issues/2323) ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --------- Co-authored-by: Innokentii Konstantinov Co-authored-by: Joey Orlando Co-authored-by: Joey Orlando --- CHANGELOG.md | 4 ++++ engine/apps/phone_notifications/phone_provider.py | 4 ++++ engine/settings/base.py | 3 ++- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6754052..b6c26db3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Changed + +- Added `PHONE_PROVIDER` configuration check by @sreway ([#2523](https://github.com/grafana/oncall/pull/2523)) + ## v1.3.13 (2023-07-17) ### Changed diff --git a/engine/apps/phone_notifications/phone_provider.py b/engine/apps/phone_notifications/phone_provider.py index ad72924a..1bff1397 100644 --- a/engine/apps/phone_notifications/phone_provider.py +++ b/engine/apps/phone_notifications/phone_provider.py @@ -181,4 +181,8 @@ def get_phone_provider() -> PhoneProvider: if len(_providers) == 0: for provider_alias, importpath in settings.PHONE_PROVIDERS.items(): _providers[provider_alias] = import_string(importpath)() + + if live_settings.PHONE_PROVIDER not in settings.PHONE_PROVIDERS.keys(): + return _providers[settings.DEFAULT_PHONE_PROVIDER] + return _providers[live_settings.PHONE_PROVIDER] diff --git a/engine/settings/base.py b/engine/settings/base.py index 7a7b6228..4a4cc73f 100644 --- a/engine/settings/base.py +++ b/engine/settings/base.py @@ -725,6 +725,7 @@ PYROSCOPE_AUTH_TOKEN = os.getenv("PYROSCOPE_AUTH_TOKEN", "") # map of phone provider alias to importpath. # Used in get_phone_provider function to dynamically load current provider. +DEFAULT_PHONE_PROVIDER = "twilio" PHONE_PROVIDERS = { "twilio": "apps.twilioapp.phone_provider.TwilioPhoneProvider", # "simple": "apps.phone_notifications.simple_phone_provider.SimplePhoneProvider", @@ -733,7 +734,7 @@ PHONE_PROVIDERS = { if IS_OPEN_SOURCE: PHONE_PROVIDERS["zvonok"] = "apps.zvonok.phone_provider.ZvonokPhoneProvider" -PHONE_PROVIDER = os.environ.get("PHONE_PROVIDER", default="twilio") +PHONE_PROVIDER = os.environ.get("PHONE_PROVIDER", default=DEFAULT_PHONE_PROVIDER) ZVONOK_API_KEY = os.getenv("ZVONOK_API_KEY", None) ZVONOK_CAMPAIGN_ID = os.getenv("ZVONOK_CAMPAIGN_ID", None) From 78df72bef9bd799385ce5e09f3fd99b3433579ae Mon Sep 17 00:00:00 2001 From: Jack Baldry Date: Mon, 17 Jul 2023 14:15:34 +0100 Subject: [PATCH 4/8] Update all make docs CI to match (#2538) My previous PR only updated the CI that ran on PRs, I forgot about the CD for releases. Fixes https://github.com/grafana/oncall/actions/runs/5547818896. You can see that this will resolve the problem because it is what allows the PR CI to pass. I just forgot to include it in the release CD. Signed-off-by: Jack Baldry --- .github/workflows/linting-and-tests.yml | 3 +-- .../workflows/publish-technical-documentation-next.yml | 2 +- .../workflows/publish-technical-documentation-release.yml | 8 ++++---- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/.github/workflows/linting-and-tests.yml b/.github/workflows/linting-and-tests.yml index ac5db291..75197ca3 100644 --- a/.github/workflows/linting-and-tests.yml +++ b/.github/workflows/linting-and-tests.yml @@ -83,9 +83,8 @@ jobs: # -e HUGO_REFLINKSERRORLEVEL=ERROR prevents merging broken refs with the downside # that no refs to external content can be used as these refs will not resolve in the # docs-base image. - # Use alternative image (dbd975af06) until make-docs 3.0.0 is rolled out everywhere. run: | - docker run -v ${PWD}/docs/sources:/hugo/content/docs/oncall/latest -e HUGO_REFLINKSERRORLEVEL=ERROR --rm grafana/docs-base:dbd975af06 /bin/bash -c 'echo -e "---\\nredirectURL: /hugo/content/docs/oncall/latest/\\ntype: redirect\\nversioned: true\\n---\\n" > /hugo/content/docs/oncall/_index.md; make hugo' + docker run -v ${PWD}/docs/sources:/hugo/content/docs/oncall/latest -e HUGO_REFLINKSERRORLEVEL=ERROR --rm grafana/docs-base:latest /bin/bash -c 'echo -e "---\\nredirectURL: /hugo/content/docs/oncall/latest/\\ntype: redirect\\nversioned: true\\n---\\n" > /hugo/content/docs/oncall/_index.md; make hugo' lint-migrations-backend-mysql-rabbitmq: name: "Lint database migrations" diff --git a/.github/workflows/publish-technical-documentation-next.yml b/.github/workflows/publish-technical-documentation-next.yml index c242bb78..3de57668 100644 --- a/.github/workflows/publish-technical-documentation-next.yml +++ b/.github/workflows/publish-technical-documentation-next.yml @@ -18,7 +18,7 @@ jobs: # that no refs to external content can be used as these refs will not resolve in the # docs-base image. run: | - docker run -v ${PWD}/docs/sources:/hugo/content/docs/oncall/latest -e HUGO_REFLINKSERRORLEVEL=ERROR --rm grafana/docs-base:latest /bin/bash -c 'make hugo' + docker run -v ${PWD}/docs/sources:/hugo/content/docs/oncall/latest -e HUGO_REFLINKSERRORLEVEL=ERROR --rm grafana/docs-base:latest /bin/bash -c 'echo -e "---\\nredirectURL: /hugo/content/docs/oncall/latest/\\ntype: redirect\\nversioned: true\\n---\\n" > /hugo/content/docs/oncall/_index.md; make hugo' sync: runs-on: "ubuntu-latest" diff --git a/.github/workflows/publish-technical-documentation-release.yml b/.github/workflows/publish-technical-documentation-release.yml index 1078d0e0..7a5f7c89 100644 --- a/.github/workflows/publish-technical-documentation-release.yml +++ b/.github/workflows/publish-technical-documentation-release.yml @@ -17,11 +17,11 @@ jobs: uses: "actions/checkout@v3" - name: "Build website" - # -e HUGO_REFLINKSERRORLEVEL=ERROR prevents merging broken refs with the downside - # that no refs to external content can be used as these refs will not resolve in the - # docs-base image. + # -e HUGO_REFLINKSERRORLEVEL=ERROR prevents merging broken refs with the downside + # that no refs to external content can be used as these refs will not resolve in the + # docs-base image. run: | - docker run -v ${PWD}/docs/sources:/hugo/content/docs/oncall/latest -e HUGO_REFLINKSERRORLEVEL=ERROR --rm grafana/docs-base:latest /bin/bash -c 'make hugo' + docker run -v ${PWD}/docs/sources:/hugo/content/docs/oncall/latest -e HUGO_REFLINKSERRORLEVEL=ERROR --rm grafana/docs-base:latest /bin/bash -c 'echo -e "---\\nredirectURL: /hugo/content/docs/oncall/latest/\\ntype: redirect\\nversioned: true\\n---\\n" > /hugo/content/docs/oncall/_index.md; make hugo' sync: runs-on: "ubuntu-latest" From 69bafb61f155ac8a1ac2cb0536aeddb3f07df7e6 Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Mon, 17 Jul 2023 14:21:56 +0100 Subject: [PATCH 5/8] 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 From 2aab2a3e044d7ff9e585265e5fa17953b7aa25c2 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Mon, 17 Jul 2023 10:59:16 -0300 Subject: [PATCH 6/8] Remove v prefix from plugin version on build (#2540) Related to https://github.com/grafana/oncall/issues/458 Change version naming for plugin and docker releases to drop the `v` prefix (ie. `1.2.3` instead of `v1.2.3`). Update both to keep consistency between plugin and engine versions, which are also checked to match. --- .drone.yml | 8 ++++---- CHANGELOG.md | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.drone.yml b/.drone.yml index 6a656d5c..5d655cc9 100644 --- a/.drone.yml +++ b/.drone.yml @@ -10,7 +10,7 @@ steps: - apt-get update - apt-get --assume-yes install jq - cd grafana-plugin/ - - if [ -z "$DRONE_TAG" ]; then echo "No tag, not modifying version"; else jq '.version="${DRONE_TAG}"' package.json > package.new && mv package.new package.json && jq '.version' package.json; fi + - if [ -z "$DRONE_TAG" ]; then echo "No tag, not modifying version"; else jq ".version=\"${DRONE_TAG#v}\"" package.json > package.new && mv package.new package.json && jq '.version' package.json; fi - yarn --network-timeout 500000 - yarn build - ls ./ @@ -179,7 +179,7 @@ steps: - apt-get update - apt-get --assume-yes install jq - cd grafana-plugin/ - - if [ -z "$DRONE_TAG" ]; then echo "No tag, not modifying version"; else jq '.version="${DRONE_TAG}"' package.json > package.new && mv package.new package.json && jq '.version' package.json; fi + - if [ -z "$DRONE_TAG" ]; then echo "No tag, not modifying version"; else jq ".version=\"${DRONE_TAG#v}\"" package.json > package.new && mv package.new package.json && jq '.version' package.json; fi - yarn --network-timeout 500000 - yarn build - ls ./ @@ -231,7 +231,7 @@ steps: image: alpine commands: - apk add --no-cache bash sed - - if [ -z "$DRONE_TAG" ]; then echo "No tag, not modifying version"; else sed "0,/VERSION.*/ s/VERSION.*/VERSION = \"${DRONE_TAG}\"/g" engine/settings/base.py > engine/settings/base.temp && mv engine/settings/base.temp engine/settings/base.py; fi + - if [ -z "$DRONE_TAG" ]; then echo "No tag, not modifying version"; else sed "0,/VERSION.*/ s/VERSION.*/VERSION = \"${DRONE_TAG#v}\"/g" engine/settings/base.py > engine/settings/base.temp && mv engine/settings/base.temp engine/settings/base.py; fi - cat engine/settings/base.py | grep VERSION | head -1 - name: build and push docker image @@ -334,6 +334,6 @@ kind: secret name: drone_token --- kind: signature -hmac: be10373849d65e1f90bce64c7468d5cf5bac546285226ff8931a8b953163a752 +hmac: 1ca79af7b8ada54237e4fb78fffb9314babc00b3a8a4866ec454d32356bc067b ... diff --git a/CHANGELOG.md b/CHANGELOG.md index b8e4913c..cce28012 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - 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)) +- Change plugin version to drop the `v` prefix. ([#2540](https://github.com/grafana/oncall/pull/2540)) ## v1.3.13 (2023-07-17) From d5b43b043979a1d7d8bf1c9429ea1353d2db3365 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 17 Jul 2023 16:04:53 +0200 Subject: [PATCH 7/8] minor improvements for check_escalation_finished celery task (#2554) # What this PR does This PR adds some enhancements to the `check_escalation_finished` celery task. It short-circuits auditing of an alert group if it does not have an escalation chain associated with it. In `EscalationSnapshotMixin.start_escalation_if_needed` we will not set `raw_escalation_snapshot` ([here](https://github.com/grafana/oncall/blob/dev/engine/apps/alerts/escalation_snapshot/escalation_snapshot_mixin.py#L262)) in this case: ```python3 def start_escalation_if_needed(self, countdown=START_ESCALATION_DELAY, eta=None): """ :type self:AlertGroup """ AlertGroup = apps.get_model("alerts", "AlertGroup") is_on_maintenace_or_debug_mode = self.channel.maintenance_mode is not None if ( self.is_restricted or is_on_maintenace_or_debug_mode or self.pause_escalation or not self.escalation_chain_exists <-- here ): logger.debug( f"Not escalating alert group w/ pk: {self.pk}\n" f"is_restricted: {self.is_restricted}\n" f"is_on_maintenace_or_debug_mode: {is_on_maintenace_or_debug_mode}\n" f"pause_escalation: {self.pause_escalation}\n" f"escalation_chain_exists: {self.escalation_chain_exists}" ) return logger.debug(f"Start escalation for alert group with pk: {self.pk}") # take raw escalation snapshot from db if escalation is paused raw_escalation_snapshot = ( self.build_raw_escalation_snapshot() if not self.pause_escalation else self.raw_escalation_snapshot ) task_id = celery_uuid() AlertGroup.all_objects.filter(pk=self.pk,).update( active_escalation_id=task_id, is_escalation_finished=False, raw_escalation_snapshot=raw_escalation_snapshot, ) ``` `EscalationSnapshotMixin.escalation_chain_exists` is as such: ```python3 @property def escalation_chain_exists(self) -> bool: if self.pause_escalation: return False elif not self.channel_filter: return False return self.channel_filter.escalation_chain is not None ``` ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) (N/A) - [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) (N/A) --- .../alerts/tasks/check_escalation_finished.py | 30 ++++++++++++++----- .../test_check_escalation_finished_task.py | 21 +++++++++++++ 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/engine/apps/alerts/tasks/check_escalation_finished.py b/engine/apps/alerts/tasks/check_escalation_finished.py index 343696c9..b0cfee9e 100644 --- a/engine/apps/alerts/tasks/check_escalation_finished.py +++ b/engine/apps/alerts/tasks/check_escalation_finished.py @@ -34,10 +34,19 @@ def audit_alert_group_escalation(alert_group: "AlertGroup") -> None: alert_group_id = alert_group.id base_msg = f"Alert group {alert_group_id}" - if not escalation_snapshot: - raise AlertGroupEscalationPolicyExecutionAuditException( - f"{base_msg} does not have an escalation snapshot associated with it, this should never occur" + if not alert_group.escalation_chain_exists: + task_logger.info( + f"{base_msg} does not have an escalation chain associated with it, and therefore it is expected " + "that it will not have an escalation snapshot, skipping further validation" ) + return + + if not escalation_snapshot: + msg = f"{base_msg} does not have an escalation snapshot associated with it, this should never occur" + + task_logger.warning(msg) + raise AlertGroupEscalationPolicyExecutionAuditException(msg) + task_logger.info(f"{base_msg} has an escalation snapshot associated with it, auditing if it executed properly") escalation_policies_snapshots = escalation_snapshot.escalation_policies_snapshots @@ -118,8 +127,11 @@ def check_escalation_finished_task() -> None: started_at__range=(two_days_ago, now), ) - if not alert_groups.exists(): - task_logger.info("There are no alert groups to audit, everything is good :)") + task_logger.info( + f"There are {len(alert_groups)} alert group(s) to audit" + if alert_groups.exists() + else "There are no alert groups to audit, everything is good :)" + ) alert_group_ids_that_failed_audit: typing.List[str] = [] @@ -130,8 +142,10 @@ def check_escalation_finished_task() -> None: alert_group_ids_that_failed_audit.append(str(alert_group.id)) if alert_group_ids_that_failed_audit: - raise AlertGroupEscalationPolicyExecutionAuditException( - f"The following alert group id(s) failed auditing: {', '.join(alert_group_ids_that_failed_audit)}" - ) + msg = f"The following alert group id(s) failed auditing: {', '.join(alert_group_ids_that_failed_audit)}" + task_logger.warning(msg) + raise AlertGroupEscalationPolicyExecutionAuditException(msg) + + task_logger.info("There were no alert groups that failed auditing") send_alert_group_escalation_auditor_task_heartbeat() diff --git a/engine/apps/alerts/tests/test_check_escalation_finished_task.py b/engine/apps/alerts/tests/test_check_escalation_finished_task.py index ab0cfaaf..8e947210 100644 --- a/engine/apps/alerts/tests/test_check_escalation_finished_task.py +++ b/engine/apps/alerts/tests/test_check_escalation_finished_task.py @@ -80,6 +80,27 @@ def test_send_alert_group_escalation_auditor_task_heartbeat_raises_an_exception_ mock_requests.get.return_value.raise_for_status.assert_called_once_with() +@pytest.mark.django_db +def test_audit_alert_group_escalation_skips_validation_if_the_alert_group_does_not_have_an_escalation_chain( + make_organization_and_user, + make_alert_receive_channel, + make_alert_group, +): + organization, _ = make_organization_and_user() + alert_receive_channel = make_alert_receive_channel(organization) + alert_group = make_alert_group(alert_receive_channel) + + alert_group.escalation_snapshot = None + alert_group.save() + + assert alert_group.escalation_chain_exists is False + + try: + audit_alert_group_escalation(alert_group) + except AlertGroupEscalationPolicyExecutionAuditException: + pytest.fail() + + @pytest.mark.django_db def test_audit_alert_group_escalation_raises_exception_if_the_alert_group_does_not_have_an_escalation_snapshot( escalation_snapshot_test_setup, From 75ee2bc0c0ebb2b15aa90c1921054afe1bc60085 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 17 Jul 2023 10:41:19 -0400 Subject: [PATCH 8/8] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cce28012..f7dd4d57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +## v1.3.14 (2023-07-17) + ### Changed - Added `PHONE_PROVIDER` configuration check by @sreway ([#2523](https://github.com/grafana/oncall/pull/2523))