Update Shift Swap Request Slack message formatting (#2918)

# What this PR does Updates Shift Swap Request Slack message formatting to be consistent with UI mockups: **New Request** ![Screenshot 2023-08-30 at 12 18 54](https://github.com/grafana/oncall/assets/9406895/712a13e2-b768-4be6-b066-c5daa98446eb) **Accepted** ![Screenshot 2023-08-30 at 12 19 41](https://github.com/grafana/oncall/assets/9406895/84d14adf-bb48-4ba8-bee2-eb7931ef2d55) **Deleted** ![Screenshot 2023-08-30 at 12 19 01](https://github.com/grafana/oncall/assets/9406895/9bda06c8-5ce7-405b-9303-b160eac4d635) ## 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)
2023-09-01 07:53:27 +02:00 · 2023-09-01 07:53:27 +02:00 · 73da83274a
commit 73da83274a
parent 15e02c7f30
4 changed files with 48 additions and 63 deletions
--- 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))
--- 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
 <img src="/static/img/oncall/swap-web-hover.png">
 <img src="/static/img/oncall/swap-web-request.png">

->**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.

-<img src="/static/img/oncall/swap-slack-notification.png">
+<img src="/static/img/oncall/swap-slack-notification-3.png">

 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

--- a/engine/apps/slack/scenarios/shift_swap_requests.py
+++ b/engine/apps/slack/scenarios/shift_swap_requests.py
@ -5,7 +5,6 @@ import typing
 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 @@ logger.setLevel(logging.DEBUG)
 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 @@ class BaseShiftSwapRequestStep(scenario_step.ScenarioStep):
                    },
                },
            ),
-            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 @@ class BaseShiftSwapRequestStep(scenario_step.ScenarioStep):
                        "type": "section",
                        "text": {
                            "type": "mrkdwn",
-                            "text": f"*📝 Description*: {description}",
+                            "text": f"*Description*\n{description}",
                        },
                    },
                )
@ -92,7 +104,7 @@ class BaseShiftSwapRequestStep(scenario_step.ScenarioStep):
                        "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 @@ class BaseShiftSwapRequestStep(scenario_step.ScenarioStep):
                        "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 @@ class BaseShiftSwapRequestStep(scenario_step.ScenarioStep):
                        "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 @@ class BaseShiftSwapRequestStep(scenario_step.ScenarioStep):
                )
            )

-        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 @@ class ShiftSwapRequestFollowUp(scenario_step.ScenarioStep):
                    "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!"
                        ),
                    },
                },
--- 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 @@ class TestBaseShiftSwapRequestStep:
        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 @@ class TestBaseShiftSwapRequestStep:
        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 @@ class TestBaseShiftSwapRequestStep:
        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 @@ class TestBaseShiftSwapRequestStep:
        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 @@ class TestBaseShiftSwapRequestStep:
        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")