diff --git a/CHANGELOG.md b/CHANGELOG.md index fdceacc7..fbc80b9c 100644 --- 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 diff --git a/engine/apps/api/tests/test_shift_swaps.py b/engine/apps/api/tests/test_shift_swaps.py index 145c79ca..05a17750 100644 --- 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 diff --git a/engine/apps/api/views/shift_swap.py b/engine/apps/api/views/shift_swap.py index 8e89d68c..5a4c01af 100644 --- 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): diff --git a/engine/apps/public_api/tests/test_shift_swap.py b/engine/apps/public_api/tests/test_shift_swap.py index 913144f7..a4593320 100644 --- 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() diff --git a/engine/apps/slack/tests/test_interactive_api_endpoint.py b/engine/apps/slack/tests/test_interactive_api_endpoint.py index fdb6c4c3..62d2026c 100644 --- 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",