From 195181f57112b441d0dfc9806139dcabcb9fcb86 Mon Sep 17 00:00:00 2001 From: Yulya Artyukhina Date: Tue, 11 Jul 2023 12:32:10 +0200 Subject: [PATCH] Exclude schedules from deleted organizations from notification list (#2493) # What this PR does On notifying about shifts change in slack we don't check whether organization was deleted. This PR excludes schedules from deleted organizations from shift notification list ## Which issue(s) this PR fixes ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] 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) --- CHANGELOG.md | 1 + .../tasks/notify_ical_schedule_shift.py | 3 +++ .../schedules/tasks/refresh_ical_files.py | 4 +-- .../tests/test_tasks_refresh_ical_files.py | 27 +++++++++++++++++-- 4 files changed, 31 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a817aa6..e6fd2a9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Address issue where we were improperly parsing Grafana feature flags that were enabled via the `feature_flags.enabled` method by @joeyorlando ([#2477](https://github.com/grafana/oncall/pull/2477)) - Fix cuddled list Markdown issue by @vadimkerr ([#2488](https://github.com/grafana/oncall/pull/2488)) +- Fixed schedules slack notifications for deleted organizations ([#2493](https://github.com/grafana/oncall/pull/2493)) ## v1.3.7 (2023-07-06) diff --git a/engine/apps/alerts/tasks/notify_ical_schedule_shift.py b/engine/apps/alerts/tasks/notify_ical_schedule_shift.py index aff7a4d1..af46779e 100644 --- a/engine/apps/alerts/tasks/notify_ical_schedule_shift.py +++ b/engine/apps/alerts/tasks/notify_ical_schedule_shift.py @@ -186,6 +186,9 @@ def notify_ical_schedule_shift(schedule_pk): if schedule.organization.slack_team_identity is None: task_logger.info(f"Trying to notify ical schedule shift with no slack team identity {schedule_pk}") return + elif schedule.organization.deleted_at: + task_logger.info(f"Trying to notify ical schedule shift from deleted organization {schedule_pk}") + return MIN_DAYS_TO_LOOKUP_FOR_THE_END_OF_EVENT = 3 diff --git a/engine/apps/schedules/tasks/refresh_ical_files.py b/engine/apps/schedules/tasks/refresh_ical_files.py index 18b78936..262a3032 100644 --- a/engine/apps/schedules/tasks/refresh_ical_files.py +++ b/engine/apps/schedules/tasks/refresh_ical_files.py @@ -16,7 +16,7 @@ def start_refresh_ical_files(): task_logger.info("Start refresh ical files") - schedules = OnCallSchedule.objects.all() + schedules = OnCallSchedule.objects.filter(organization__deleted_at__isnull=True) for schedule in schedules: refresh_ical_file.apply_async((schedule.pk,)) @@ -30,7 +30,7 @@ def start_refresh_ical_final_schedules(): task_logger.info("Start refresh ical final schedules") - schedules = OnCallSchedule.objects.all() + schedules = OnCallSchedule.objects.filter(organization__deleted_at__isnull=True) for schedule in schedules: refresh_ical_final_schedule.apply_async((schedule.pk,)) diff --git a/engine/apps/schedules/tests/test_tasks_refresh_ical_files.py b/engine/apps/schedules/tests/test_tasks_refresh_ical_files.py index 0a7ed738..a99fd8e8 100644 --- a/engine/apps/schedules/tests/test_tasks_refresh_ical_files.py +++ b/engine/apps/schedules/tests/test_tasks_refresh_ical_files.py @@ -1,9 +1,10 @@ +import datetime from unittest.mock import patch import pytest -from apps.schedules.models import OnCallScheduleICal -from apps.schedules.tasks.refresh_ical_files import refresh_ical_file +from apps.schedules.models import OnCallScheduleICal, OnCallScheduleWeb +from apps.schedules.tasks.refresh_ical_files import refresh_ical_file, start_refresh_ical_files @pytest.mark.django_db @@ -56,3 +57,25 @@ def test_refresh_ical_file_trigger_run( assert mock_notify_empty.apply_async.called == run_task assert mock_notify_gaps.apply_async.called == run_task + + +@pytest.mark.django_db +@patch("apps.slack.tasks.start_update_slack_user_group_for_schedules.apply_async") +def test_refresh_ical_files_filter_orgs( + mocked_start_update_slack_user_group_for_schedules, + make_organization, + make_schedule, +): + organization = make_organization() + deleted_organization = make_organization(deleted_at=datetime.datetime.now()) + + schedule_from_deleted_org = make_schedule(deleted_organization, schedule_class=OnCallScheduleWeb) + schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb) + + with patch("apps.schedules.tasks.refresh_ical_file.apply_async") as mocked_refresh_ical_file: + start_refresh_ical_files() + assert mocked_refresh_ical_file.called + called_args = mocked_refresh_ical_file.call_args_list + assert len(called_args) == 1 + assert schedule.id in called_args[0].args[0] + assert schedule_from_deleted_org.id not in called_args[0].args[0]