Address bug when taking a Shift Swap Request and the Slack message is not updated (#2886)

# What this PR does Address bug when a Shift Swap Request is accepted either via the web or mobile UI, and the Slack message is not updated to reflect the latest state. ## 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-08-28 11:55:17 +02:00 · 2023-08-28 11:55:17 +02:00 · c811a2ad15
commit c811a2ad15
parent d5e1e9ade2
5 changed files with 48 additions and 9 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

 ## Unreleased

+### Fixed
+
+- Address bug when a Shift Swap Request is accepted either via the web or mobile UI, and the Slack message is not
+  updated to reflect the latest state by @joeyorlando ([#2886](https://github.com/grafana/oncall/pull/2886))
+
 ## v1.3.27 (2023-08-25)

 ### Added
--- a/engine/apps/api/tests/test_shift_swaps.py
+++ b/engine/apps/api/tests/test_shift_swaps.py
@ -669,8 +669,9 @@ def test_delete_others_ssr_permissions(ssr_setup, make_user_auth_headers):
    assert response.status_code == status.HTTP_403_FORBIDDEN


+@patch("apps.api.views.shift_swap.update_shift_swap_request_message")
@pytest.mark.django_db
-def test_take(ssr_setup, make_user_auth_headers):
+def test_take(mock_update_shift_swap_request_message, ssr_setup, make_user_auth_headers):
    ssr, _, token, benefactor = ssr_setup()
    client = APIClient()
    url = reverse("api-internal:shift_swap-take", kwargs={"pk": ssr.public_primary_key})
@ -696,9 +697,14 @@ def test_take(ssr_setup, make_user_auth_headers):
    assert response.status_code == status.HTTP_200_OK
    assert response_json == expected_response

+    mock_update_shift_swap_request_message.apply_async.assert_called_once_with((ssr.pk,))

+
+@patch("apps.api.views.shift_swap.update_shift_swap_request_message")
@pytest.mark.django_db
-def test_benficiary_tries_to_take_their_own_ssr(ssr_setup, make_user_auth_headers):
+def test_benficiary_tries_to_take_their_own_ssr(
+    mock_update_shift_swap_request_message, ssr_setup, make_user_auth_headers
+):
    ssr, beneficiary, token, _ = ssr_setup()
    client = APIClient()
    url = reverse("api-internal:shift_swap-take", kwargs={"pk": ssr.public_primary_key})
@ -706,6 +712,8 @@ def test_benficiary_tries_to_take_their_own_ssr(ssr_setup, make_user_auth_header
    response = client.post(url, format="json", **make_user_auth_headers(beneficiary, token))
    assert response.status_code == status.HTTP_400_BAD_REQUEST

+    mock_update_shift_swap_request_message.apply_async.assert_not_called()
+

@pytest.mark.django_db
 def test_take_already_taken_ssr(ssr_setup, make_user_auth_headers):
@ -713,15 +721,22 @@ def test_take_already_taken_ssr(ssr_setup, make_user_auth_headers):
    client = APIClient()
    url = reverse("api-internal:shift_swap-take", kwargs={"pk": ssr.public_primary_key})

-    response = client.post(url, format="json", **make_user_auth_headers(benefactor, token))
-    assert response.status_code == status.HTTP_200_OK
+    with patch("apps.api.views.shift_swap.update_shift_swap_request_message") as mock_update_shift_swap_request_message:
+        response = client.post(url, format="json", **make_user_auth_headers(benefactor, token))
+        assert response.status_code == status.HTTP_200_OK

-    response = client.post(url, format="json", **make_user_auth_headers(benefactor, token))
-    assert response.status_code == status.HTTP_400_BAD_REQUEST
+        mock_update_shift_swap_request_message.apply_async.assert_called_once_with((ssr.pk,))
+
+    with patch("apps.api.views.shift_swap.update_shift_swap_request_message") as mock_update_shift_swap_request_message:
+        response = client.post(url, format="json", **make_user_auth_headers(benefactor, token))
+        assert response.status_code == status.HTTP_400_BAD_REQUEST
+
+        mock_update_shift_swap_request_message.apply_async.assert_not_called()


+@patch("apps.api.views.shift_swap.update_shift_swap_request_message")
@pytest.mark.django_db
-def test_take_past_due_ssr(ssr_setup, make_user_auth_headers):
+def test_take_past_due_ssr(mock_update_shift_swap_request_message, ssr_setup, make_user_auth_headers):
    ssr, _, token, benefactor = ssr_setup()
    client = APIClient()
    url = reverse("api-internal:shift_swap-take", kwargs={"pk": ssr.public_primary_key})
@ -732,9 +747,12 @@ def test_take_past_due_ssr(ssr_setup, make_user_auth_headers):
    response = client.post(url, format="json", **make_user_auth_headers(benefactor, token))
    assert response.status_code == status.HTTP_400_BAD_REQUEST

+    mock_update_shift_swap_request_message.apply_async.assert_not_called()

+
+@patch("apps.api.views.shift_swap.update_shift_swap_request_message")
@pytest.mark.django_db
-def test_take_deleted_ssr(ssr_setup, make_user_auth_headers):
+def test_take_deleted_ssr(mock_update_shift_swap_request_message, ssr_setup, make_user_auth_headers):
    ssr, _, token, benefactor = ssr_setup()
    client = APIClient()
    url = reverse("api-internal:shift_swap-take", kwargs={"pk": ssr.public_primary_key})
@ -744,6 +762,8 @@ def test_take_deleted_ssr(ssr_setup, make_user_auth_headers):
    response = client.post(url, format="json", **make_user_auth_headers(benefactor, token))
    assert response.status_code == status.HTTP_404_NOT_FOUND

+    mock_update_shift_swap_request_message.apply_async.assert_not_called()
+

@patch("apps.api.views.shift_swap.ShiftSwapViewSet.take", return_value=mock_success_response)
@pytest.mark.django_db
--- a/engine/apps/api/views/shift_swap.py
+++ b/engine/apps/api/views/shift_swap.py
@ -45,6 +45,8 @@ class BaseShiftSwapViewSet(ModelViewSet):
        except exceptions.BeneficiaryCannotTakeOwnShiftSwapRequest:
            raise BadRequest(detail="A shift swap request cannot be created and taken by the same user")

+        update_shift_swap_request_message.apply_async((shift_swap.pk,))
+
        return ShiftSwapRequestSerializer(shift_swap).data

    def get_serializer_class(self):
--- a/engine/apps/public_api/tests/test_shift_swap.py
+++ b/engine/apps/public_api/tests/test_shift_swap.py
@ -330,8 +330,10 @@ def test_delete(
    mock_update_shift_swap_request_message.apply_async.assert_called_once_with((swap.pk,))


+@patch("apps.api.views.shift_swap.update_shift_swap_request_message")
@pytest.mark.django_db
 def test_take(
+    mock_update_shift_swap_request_message,
    make_organization_and_user_with_token,
    make_user_for_organization,
    setup_swap,
@ -352,9 +354,13 @@ def test_take(
    assert swap.status == ShiftSwapRequest.Statuses.TAKEN
    assert swap.benefactor == another_user

+    mock_update_shift_swap_request_message.apply_async.assert_called_once_with((swap.pk,))

+
+@patch("apps.api.views.shift_swap.update_shift_swap_request_message")
@pytest.mark.django_db
 def test_take_requires_benefactor(
+    mock_update_shift_swap_request_message,
    make_organization_and_user_with_token,
    setup_swap,
 ):
@ -372,9 +378,13 @@ def test_take_requires_benefactor(
    assert swap.status == ShiftSwapRequest.Statuses.OPEN
    assert swap.benefactor is None

+    mock_update_shift_swap_request_message.apply_async.assert_not_called()

+
+@patch("apps.api.views.shift_swap.update_shift_swap_request_message")
@pytest.mark.django_db
 def test_take_errors(
+    mock_update_shift_swap_request_message,
    make_organization_and_user_with_token,
    make_user_for_organization,
    setup_swap,
@ -411,3 +421,5 @@ def test_take_errors(
    data = {"benefactor": another_user.public_primary_key}
    response = client.post(url, data, format="json", HTTP_AUTHORIZATION=f"{token}")
    assert response.status_code == status.HTTP_400_BAD_REQUEST
+
+    mock_update_shift_swap_request_message.apply_async.assert_not_called()
--- a/engine/apps/slack/tests/test_interactive_api_endpoint.py
+++ b/engine/apps/slack/tests/test_interactive_api_endpoint.py
@ -228,7 +228,7 @@ def test_accept_shift_swap_request(
                "action_id": "AcceptShiftSwapRequestStep",
                "block_id": "G0ec",
                "text": {"type": "plain_text", "text": ":heavy_check_mark: Accept Shift Swap Request", "emoji": True},
-                "value": '{"shift_swap_request_pk": 5, "organization_id": 1}',
+                "value": f'{{"shift_swap_request_pk": 5, "organization_id": {organization.pk}}}',
                "style": "primary",
                "type": "button",
                "action_ts": "1693208812.474860",