diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b372914..865a83fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +## v1.3.88 (2024-01-16) + +### Fixed + +- Fix updating a shift swap with no Slack message by @vadimkerr ([#3686](https://github.com/grafana/oncall/pull/3686)) + ## v1.3.87 (2024-01-15) ### Fixed diff --git a/engine/apps/slack/scenarios/shift_swap_requests.py b/engine/apps/slack/scenarios/shift_swap_requests.py index b7ffcf86..0cbd7f79 100644 --- a/engine/apps/slack/scenarios/shift_swap_requests.py +++ b/engine/apps/slack/scenarios/shift_swap_requests.py @@ -165,6 +165,9 @@ class BaseShiftSwapRequestStep(scenario_step.ScenarioStep): ) def update_message(self, shift_swap_request: "ShiftSwapRequest") -> None: + if not shift_swap_request.slack_message: + return + self._slack_client.chat_update( channel=shift_swap_request.slack_channel_id, ts=shift_swap_request.slack_message.slack_id, 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 df91ae47..0ffe428a 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 @@ -183,6 +183,19 @@ class TestBaseShiftSwapRequestStep: channel=ssr.slack_channel_id, ts=ts, blocks=mock_generate_blocks.return_value ) + @patch("apps.slack.scenarios.shift_swap_requests.BaseShiftSwapRequestStep._generate_blocks") + @pytest.mark.django_db + def test_update_message_no_slack_message(self, mock_generate_blocks, setup, make_slack_message) -> None: + ssr, _, _, _ = setup() # SSR without a Slack message + organization = ssr.organization + slack_team_identity = organization.slack_team_identity + + step = scenarios.BaseShiftSwapRequestStep(slack_team_identity, organization) + + with patch.object(step, "_slack_client") as mock_slack_client: + step.update_message(ssr) + mock_slack_client.chat_update.assert_not_called() + @pytest.mark.django_db def test_post_message_to_thread(self, setup, make_slack_message) -> None: ts = "12345.67" diff --git a/engine/common/custom_celery_tasks/dedicated_queue_retry_task.py b/engine/common/custom_celery_tasks/dedicated_queue_retry_task.py index 1a2bd8b9..966c73d1 100644 --- a/engine/common/custom_celery_tasks/dedicated_queue_retry_task.py +++ b/engine/common/custom_celery_tasks/dedicated_queue_retry_task.py @@ -1,9 +1,12 @@ from celery import shared_task +from celery.utils.log import get_task_logger from common.custom_celery_tasks.log_exception_on_failure_task import LogExceptionOnFailureTask RETRY_QUEUE = "retry" +logger = logger = get_task_logger(__name__) + class DedicatedQueueRetryTask(LogExceptionOnFailureTask): """ @@ -14,6 +17,8 @@ class DedicatedQueueRetryTask(LogExceptionOnFailureTask): def retry( self, args=None, kwargs=None, exc=None, throw=True, eta=None, countdown=None, max_retries=None, **options ): + logger.warn("Retrying celery task", exc_info=exc) + # Just call retry with queue argument return super().retry( args=args, diff --git a/engine/common/custom_celery_tasks/tests/test_dedicated_queue_retry_task.py b/engine/common/custom_celery_tasks/tests/test_dedicated_queue_retry_task.py new file mode 100644 index 00000000..f6a18ce8 --- /dev/null +++ b/engine/common/custom_celery_tasks/tests/test_dedicated_queue_retry_task.py @@ -0,0 +1,19 @@ +import pytest + +from common.custom_celery_tasks.dedicated_queue_retry_task import shared_dedicated_queue_retry_task + +EXCEPTION_MSG = "my exception" + + +def test_dedicated_queue_retry_task_logs_stack_trace_on_task_retry_when_using_autoretry_for(caplog): + @shared_dedicated_queue_retry_task(autoretry_for=(Exception,), retry_backoff=False, max_retries=1) + def my_task(): + raise ValueError(EXCEPTION_MSG) + + with pytest.raises(ValueError): + my_task.apply().get() + + assert "Traceback (most recent call last):" in caplog.text + assert "Retrying celery task" in caplog.text + assert f"raise ValueError(EXCEPTION_MSG)\nValueError: {EXCEPTION_MSG}" in caplog.text + caplog.clear() diff --git a/engine/common/tests/test_task_queue_assignment.py b/engine/common/tests/test_task_queue_assignment.py index 604efcd2..4b1caad3 100644 --- a/engine/common/tests/test_task_queue_assignment.py +++ b/engine/common/tests/test_task_queue_assignment.py @@ -9,6 +9,7 @@ we should avoid @shared_dedicated_queue_retry_task or @shared_task and remove entirely if it is not needed. """ COMMON_IGNORED_TASKS = { + "common.custom_celery_tasks.tests.test_dedicated_queue_retry_task.my_task", "common.custom_celery_tasks.tests.test_log_exception_on_failure_task.my_task", "common.custom_celery_tasks.tests.test_log_exception_on_failure_task.my_task_two", }