diff --git a/CHANGELOG.md b/CHANGELOG.md index fb5b83dcc7..fb1e39cb86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Update Shift Swap Request Slack message formatting by @joeyorlando ([#2918](https://github.com/grafana/oncall/pull/2918)) - Performance and UX tweaks to integrations page ([#2869](https://github.com/grafana/oncall/pull/2869)) - Expand users details in filter swaps internal endpoint ([#2921](https://github.com/grafana/oncall/pull/2921)) - Truncate exported final shifts to match the requested period ([#2924](https://github.com/grafana/oncall/pull/2924)) diff --git a/docs/sources/on-call-schedules/shift-swaps/_index.md b/docs/sources/on-call-schedules/shift-swaps/_index.md index 97858cbf87..03340a09bc 100644 --- a/docs/sources/on-call-schedules/shift-swaps/_index.md +++ b/docs/sources/on-call-schedules/shift-swaps/_index.md @@ -36,14 +36,14 @@ of a schedule, or clicking the button shown when hovering on a particular shift ->**Note**: no recurrence rules support is available when requesting a shift swap. If you need to recurrently change a shift, -consider creating a higher level layer rotation with the desired updates. +> **Note**: no recurrence rules support is available when requesting a shift swap. If you need to recurrently change a shift, +> consider creating a higher level layer rotation with the desired updates. Upon submitting the request, a Slack notification will be sent to the channel associated to the correspondent schedule, if there is one. A [mobile push notification][shift-swap-notifications] will be sent to team members who participate in the schedule and have the notifications enabled. - + Push notifications are sent 4 weeks ahead of the requested shift swap, or shortly after creation in case the shift swap start time is less than 4 weeks away, but always during users' working hours (by default 9am-5pm on @@ -64,8 +64,8 @@ The follow-up notifications will be sent at the following intervals before the s You can delete the swap request at any time. If the swap has been taken, it will automatically be undone upon removal. ->**Note**: if [RBAC][rbac] is enabled, a user is required to have the `SCHEDULES_WRITE` permission to create, -update, take or delete a swap request. `SCHEDULES_READ` will be enough to get details about existing requests. +> **Note**: if [RBAC][rbac] is enabled, a user is required to have the `SCHEDULES_WRITE` permission to create, +> update, take or delete a swap request. `SCHEDULES_READ` will be enough to get details about existing requests. ## Check existing swap requests diff --git a/engine/apps/slack/scenarios/shift_swap_requests.py b/engine/apps/slack/scenarios/shift_swap_requests.py index 525ab7303b..18711b55f9 100644 --- a/engine/apps/slack/scenarios/shift_swap_requests.py +++ b/engine/apps/slack/scenarios/shift_swap_requests.py @@ -5,7 +5,6 @@ import humanize from django.utils import timezone -from apps.slack.constants import DIVIDER from apps.slack.models import SlackMessage from apps.slack.scenarios import scenario_step from apps.slack.types import Block, BlockActionType, EventPayload, PayloadType, ScenarioRoute @@ -21,17 +20,26 @@ SHIFT_SWAP_PK_ACTION_KEY = "shift_swap_request_pk" +def _schedule_slack_url(shift_swap_request) -> str: + schedule = shift_swap_request.schedule + return f"<{schedule.web_detail_page_link}|{schedule.name}>" + + class BaseShiftSwapRequestStep(scenario_step.ScenarioStep): def _generate_blocks(self, shift_swap_request: "ShiftSwapRequest") -> Block.AnyBlocks: pk = shift_swap_request.pk - main_message_text = f"Your teammate {shift_swap_request.beneficiary.get_username_with_slack_verbal()} has submitted a shift swap request." + main_message_text = ( + f"*New shift swap request for {_schedule_slack_url(shift_swap_request)}*\n" + f"Your teammate {shift_swap_request.beneficiary.get_username_with_slack_verbal()} has submitted a shift swap request." + ) datetime_format = SlackDateFormat.DATE_LONG_PRETTY time_format = SlackDateFormat.TIME shift_details = "" - for shift in shift_swap_request.shifts(): + shifts = shift_swap_request.shifts() + for shift in shifts: shift_start = shift["start"] shift_start_posix = shift_start.timestamp() shift_end = shift["end"] @@ -58,18 +66,22 @@ def _generate_blocks(self, shift_swap_request: "ShiftSwapRequest") -> Block.AnyB }, }, ), - typing.cast( - Block.Section, - { - "type": "section", - "text": { - "type": "mrkdwn", - "text": f"*📅 Shift Details*:\n\n{shift_details}", - }, - }, - ), ] + if shifts: + blocks.append( + typing.cast( + Block.Section, + { + "type": "section", + "text": { + "type": "mrkdwn", + "text": f"*Shift detail{'s' if len(shifts) > 1 else ''}*\n{shift_details}", + }, + }, + ), + ) + if description := shift_swap_request.description: blocks.append( typing.cast( @@ -78,7 +90,7 @@ def _generate_blocks(self, shift_swap_request: "ShiftSwapRequest") -> Block.AnyB "type": "section", "text": { "type": "mrkdwn", - "text": f"*📝 Description*: {description}", + "text": f"*Description*\n{description}", }, }, ) @@ -92,7 +104,7 @@ def _generate_blocks(self, shift_swap_request: "ShiftSwapRequest") -> Block.AnyB "type": "section", "text": { "type": "mrkdwn", - "text": "*Update*: this shift swap request has been deleted.", + "text": "❌ this shift swap request has been deleted", }, }, ), @@ -105,7 +117,7 @@ def _generate_blocks(self, shift_swap_request: "ShiftSwapRequest") -> Block.AnyB "type": "section", "text": { "type": "mrkdwn", - "text": f"*Update*: {shift_swap_request.benefactor.get_username_with_slack_verbal()} has taken the shift swap.", + "text": f"✅ {shift_swap_request.benefactor.get_username_with_slack_verbal()} has accepted the shift swap request", }, }, ), @@ -124,10 +136,9 @@ def _generate_blocks(self, shift_swap_request: "ShiftSwapRequest") -> Block.AnyB "elements": [ { "type": "button", - "style": "primary", "text": { "type": "plain_text", - "text": "✔️ Accept Shift Swap Request", + "text": "Accept", "emoji": True, }, "value": json.dumps(value), @@ -138,24 +149,6 @@ def _generate_blocks(self, shift_swap_request: "ShiftSwapRequest") -> Block.AnyB ) ) - blocks.extend( - [ - DIVIDER, - typing.cast( - Block.Context, - { - "type": "context", - "elements": [ - { - "type": "mrkdwn", - "text": f"👀 View the shift swap within Grafana OnCall by clicking <{shift_swap_request.web_link}|here>.", - }, - ], - }, - ), - ] - ) - return blocks def create_message(self, shift_swap_request: "ShiftSwapRequest") -> SlackMessage: @@ -226,8 +219,9 @@ def _generate_blocks(shift_swap_request: "ShiftSwapRequest") -> Block.AnyBlocks: "text": { "type": "mrkdwn", "text": ( - f":exclamation: This shift swap request is still open and will start in {delta}.\n" - "Jump back into the thread and accept it if you're available!" + f"⚠️ This shift swap request for {_schedule_slack_url(shift_swap_request)} is " + f"still open and will start in {delta}. Jump back into the thread and accept it if " + "you're available!" ), }, }, diff --git a/engine/apps/slack/tests/test_scenario_steps/test_shift_swap_requests.py b/engine/apps/slack/tests/test_scenario_steps/test_shift_swap_requests.py index a2089fbbc9..f480b6b4e9 100644 --- a/engine/apps/slack/tests/test_scenario_steps/test_shift_swap_requests.py +++ b/engine/apps/slack/tests/test_scenario_steps/test_shift_swap_requests.py @@ -40,26 +40,16 @@ def test_generate_blocks(self, setup) -> None: step = scenarios.BaseShiftSwapRequestStep(ssr.organization.slack_team_identity, ssr.organization) blocks = step._generate_blocks(ssr) - assert ( - blocks[0]["text"]["text"] - == f"Your teammate {beneficiary.get_username_with_slack_verbal()} has submitted a shift swap request." + assert blocks[0]["text"]["text"] == ( + f"*New shift swap request for <{ssr.schedule.web_detail_page_link}|{ssr.schedule.name}>*\n" + f"Your teammate {beneficiary.get_username_with_slack_verbal()} has submitted a shift swap request." ) - accept_button = blocks[2] + accept_button = blocks[1] - assert accept_button["elements"][0]["text"]["text"] == "✔️ Accept Shift Swap Request" + assert accept_button["elements"][0]["text"]["text"] == "Accept" assert accept_button["type"] == "actions" - assert blocks[3]["type"] == "divider" - - context_section = blocks[4] - - assert context_section["type"] == "context" - assert ( - context_section["elements"][0]["text"] - == f"👀 View the shift swap within Grafana OnCall by clicking <{ssr.web_link}|here>." - ) - @patch("apps.schedules.models.ShiftSwapRequest.shifts") @pytest.mark.parametrize( "shifts,expected_text", @@ -108,7 +98,7 @@ def test_generate_blocks_shift_details(self, mock_shifts, setup, shifts, expecte step = scenarios.BaseShiftSwapRequestStep(ssr.organization.slack_team_identity, ssr.organization) blocks = step._generate_blocks(ssr) - assert blocks[1]["text"]["text"] == f"*📅 Shift Details*:\n\n{expected_text}" + assert blocks[1]["text"]["text"] == f"*Shift details*\n{expected_text}" @pytest.mark.django_db def test_generate_blocks_ssr_has_description(self, setup) -> None: @@ -118,7 +108,7 @@ def test_generate_blocks_ssr_has_description(self, setup) -> None: step = scenarios.BaseShiftSwapRequestStep(ssr.organization.slack_team_identity, ssr.organization) blocks = step._generate_blocks(ssr) - assert blocks[2]["text"]["text"] == f"*📝 Description*: {description}" + assert blocks[1]["text"]["text"] == f"*Description*\n{description}" @pytest.mark.django_db def test_generate_blocks_ssr_is_deleted(self, setup) -> None: @@ -128,7 +118,7 @@ def test_generate_blocks_ssr_is_deleted(self, setup) -> None: step = scenarios.BaseShiftSwapRequestStep(ssr.organization.slack_team_identity, ssr.organization) blocks = step._generate_blocks(ssr) - assert blocks[2]["text"]["text"] == "*Update*: this shift swap request has been deleted." + assert blocks[1]["text"]["text"] == "❌ this shift swap request has been deleted" @pytest.mark.django_db def test_generate_blocks_ssr_is_taken(self, setup) -> None: @@ -140,8 +130,8 @@ def test_generate_blocks_ssr_is_taken(self, setup) -> None: blocks = step._generate_blocks(ssr) assert ( - blocks[2]["text"]["text"] - == f"*Update*: {benefactor.get_username_with_slack_verbal()} has taken the shift swap." + blocks[1]["text"]["text"] + == f"✅ {benefactor.get_username_with_slack_verbal()} has accepted the shift swap request" ) @patch("apps.slack.scenarios.shift_swap_requests.BaseShiftSwapRequestStep._generate_blocks")