diff --git a/CHANGELOG.md b/CHANGELOG.md index ccf1e172..14458e77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Skip past due swap requests when calculating events ([2718](https://github.com/grafana/oncall/pull/2718)) +- Update schedule slack notifications to use schedule final events by @Ferril ([#2710](https://github.com/grafana/oncall/pull/2710)) ### Fixed diff --git a/engine/apps/alerts/tasks/notify_ical_schedule_shift.py b/engine/apps/alerts/tasks/notify_ical_schedule_shift.py index e5f1d136..0324783f 100644 --- a/engine/apps/alerts/tasks/notify_ical_schedule_shift.py +++ b/engine/apps/alerts/tasks/notify_ical_schedule_shift.py @@ -1,17 +1,11 @@ import datetime import json -from copy import copy +import typing +from typing import TYPE_CHECKING from django.utils import timezone -from apps.schedules.ical_events import ical_events -from apps.schedules.ical_utils import ( - calculate_shift_diff, - event_start_end_all_day_with_respect_to_type, - get_icalendar_tz_or_utc, - get_usernames_from_ical_event, - memoized_users_in_ical, -) +from apps.schedules.ical_utils import calculate_shift_diff, parse_event_uid from apps.slack.scenarios import scenario_step from apps.slack.slack_client import SlackClientWithErrorHandling from apps.slack.slack_client.exceptions import SlackAPIException, SlackAPITokenException @@ -19,159 +13,45 @@ from common.custom_celery_tasks import shared_dedicated_queue_retry_task from .task_logger import task_logger +if TYPE_CHECKING: + from apps.schedules.models import OnCallSchedule -def get_current_shifts_from_ical(calendar, schedule, min_priority=0): - calendar_tz = get_icalendar_tz_or_utc(calendar) - now = datetime.datetime.now(timezone.utc) - events_from_ical_for_three_days = ical_events.get_events_from_ical_between( - calendar, now - datetime.timedelta(days=1), now + datetime.timedelta(days=1) - ) - shifts = {} - current_users = {} - for event in events_from_ical_for_three_days: - usernames, priority = get_usernames_from_ical_event(event) - users = memoized_users_in_ical(tuple(usernames), schedule.organization) - if len(users) > 0: - event_start, event_end, all_day_event = event_start_end_all_day_with_respect_to_type(event, calendar_tz) - if event["UID"] in shifts: - existing_event = shifts[event["UID"]] - if existing_event["start"] < now < existing_event["end"]: - continue - shifts[event["UID"]] = { - "users": [u.pk for u in users], - "start": event_start, - "end": event_end, - "all_day": all_day_event, - "priority": priority + min_priority, # increase priority for overrides - "priority_increased_by": min_priority, +def convert_prev_shifts_to_new_format(prev_shifts: dict, schedule: "OnCallSchedule") -> list: + new_prev_shifts = [] + user_ids = [] + users_info: typing.Dict[int, typing.Dict[str, str]] = {} + for shift in prev_shifts.values(): + user_ids.extend(shift.get("users", [])) + prev_users = schedule.organization.users.filter(id__in=user_ids) + for user in prev_users: + users_info.setdefault( + user.id, + { + "display_name": user.username, + "email": user.email, + "pk": user.public_primary_key, + "avatar_full": user.avatar_full_url, + }, + ) + for uid, shift in prev_shifts.items(): + shift_pk, _ = parse_event_uid(uid) + new_prev_shifts.append( + { + "users": [users_info[user_pk] for user_pk in shift["users"]], + "start": shift["start"], + "end": shift["end"], + "all_day": shift["all_day"], + "priority_level": shift["priority"], + "shift": {"pk": shift_pk}, } - current_users[event["UID"]] = users - - return shifts, current_users - - -def get_next_shifts_from_ical(calendar, schedule, min_priority=0, days_to_lookup=3): - calendar_tz = get_icalendar_tz_or_utc(calendar) - now = datetime.datetime.now(timezone.utc) - next_events_from_ical = ical_events.get_events_from_ical_between( - calendar, now - datetime.timedelta(days=1), now + datetime.timedelta(days=days_to_lookup) - ) - shifts = {} - for event in next_events_from_ical: - usernames, priority = get_usernames_from_ical_event(event) - users = memoized_users_in_ical(tuple(usernames), schedule.organization) - if len(users) > 0: - event_start, event_end, all_day_event = event_start_end_all_day_with_respect_to_type(event, calendar_tz) - - # next_shifts are not stored in db so we can use User objects directly - shifts[f"{event_start.timestamp()}_{event['UID']}"] = { - "users": users, - "start": event_start, - "end": event_end, - "all_day": all_day_event, - "priority": priority + min_priority, # increase priority for overrides - "priority_increased_by": min_priority, - } - - return shifts - - -def recalculate_shifts_with_respect_to_priority(shifts, users=None): - flag = True - while flag: - splitted_shifts = {} - uids_to_pop = set() - splitted = False - flag = False - for outer_k, outer_shift in shifts.items(): - if not splitted: - for inner_k, inner_shift in shifts.items(): - if outer_k == inner_k: - continue - else: - if outer_shift.get("priority", 0) > inner_shift.get("priority", 0): - if outer_shift["start"] > inner_shift["start"] and outer_shift["end"] < inner_shift["end"]: - new_uid_r = f"{inner_k}-split-r" - new_uid_l = f"{inner_k}-split-l" - splitted_shift_left = copy(inner_shift) - splitted_shift_right = copy(inner_shift) - splitted_shift_left["end"] = outer_shift["start"] - splitted_shift_right["start"] = outer_shift["end"] - splitted_shift_left["all_day"] = False - splitted_shift_right["all_day"] = False - splitted_shifts[new_uid_l] = splitted_shift_left - splitted_shifts[new_uid_r] = splitted_shift_right - uids_to_pop.add(inner_k) - if users is not None: - users[new_uid_l] = users[inner_k] - users[new_uid_r] = users[inner_k] - - splitted = True - flag = True - break - elif outer_shift["start"] <= inner_shift["start"] < outer_shift["end"] < inner_shift["end"]: - inner_shift["start"] = outer_shift["end"] - flag = True - elif outer_shift["end"] >= inner_shift["end"] > outer_shift["start"] > inner_shift["start"]: - inner_shift["end"] = outer_shift["start"] - flag = True - elif ( - outer_shift["start"] <= inner_shift["start"] - and outer_shift["end"] >= inner_shift["end"] - ): - uids_to_pop.add(inner_k) - flag = True - else: - flag = False - elif outer_shift.get("priority", 0) < inner_shift.get("priority", 0): - if inner_shift["start"] > outer_shift["start"] and inner_shift["end"] < outer_shift["end"]: - new_uid_r = f"{outer_k}-split-r" - new_uid_l = f"{outer_k}-split-l" - splitted_shift_left = copy(outer_shift) - splitted_shift_right = copy(outer_shift) - splitted_shift_left["all_day"] = False - splitted_shift_right["all_day"] = False - splitted_shift_left["end"] = inner_shift["start"] - splitted_shift_right["start"] = inner_shift["end"] - splitted_shifts[new_uid_l] = splitted_shift_left - splitted_shifts[new_uid_r] = splitted_shift_right - uids_to_pop.add(outer_k) - - if users is not None: - users[new_uid_l] = users[outer_k] - users[new_uid_r] = users[outer_k] - - splitted = True - flag = True - break - elif inner_shift["start"] <= outer_shift["start"] < inner_shift["end"] < outer_shift["end"]: - outer_shift["start"] = inner_shift["end"] - flag = True - elif inner_shift["end"] >= outer_shift["end"] > inner_shift["start"] > outer_shift["start"]: - outer_shift["end"] = inner_shift["start"] - flag = True - elif ( - inner_shift["start"] <= outer_shift["start"] - and inner_shift["end"] >= outer_shift["end"] - ): - uids_to_pop.add(outer_k) - flag = True - else: - flag = False - else: - flag = False - else: - break - - shifts.update(splitted_shifts) - for uid in uids_to_pop: - shifts.pop(uid) + ) + return new_prev_shifts @shared_dedicated_queue_retry_task() def notify_ical_schedule_shift(schedule_pk): - task_logger.info(f"Notify ical schedule shift {schedule_pk}") + task_logger.info(f"Start notify ical schedule shift {schedule_pk}") from apps.schedules.models import OnCallSchedule try: @@ -183,160 +63,104 @@ def notify_ical_schedule_shift(schedule_pk): return 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}") + task_logger.info( + f"Trying to notify ical schedule shift with no slack team identity {schedule_pk}, " + f"organization {schedule.organization_id}" + ) return elif schedule.organization.deleted_at: - task_logger.info(f"Trying to notify ical schedule shift from deleted organization {schedule_pk}") + task_logger.info( + f"Trying to notify ical schedule shift from deleted organization {schedule_pk}, " + f"organization {schedule.organization_id}" + ) return + task_logger.info(f"Notify ical schedule shift {schedule_pk}, organization {schedule.organization_id}") + MIN_DAYS_TO_LOOKUP_FOR_THE_END_OF_EVENT = 3 now = datetime.datetime.now(timezone.utc) - # get list of iCalendars from current iCal files. If there is more than one calendar, primary calendar will always - # be the first - current_calendars = schedule.get_icalendars() - current_shifts = {} - # expected current_shifts structure: - # { - # some uid: { - # "users": [users pks], - # "start": event start date, - # "end": event end date, - # "all_day": bool if event has all-day type, - # "priority": priority level, - # "priority_increased_by": min priority level of primary calendar, (for primary calendar event it is 0) - # }, - # } + current_shifts = schedule.final_events(now, now, with_empty=False, with_gap=False) - # Current_user dict exists because it's bad idea to serialize User objects. - # Instead users' pks are stored in db for calculation related to shift diff. - # When it is needed to pass shift's user (e.g. in def get_report_blocks_ical()) - # we take users from current_users{} by shift uuid and replace users' pk - current_users = {} + prev_shifts = json.loads(schedule.current_shifts) if not schedule.empty_oncall else [] + prev_shifts_updated = False + # convert prev_shifts to new events format for compatibility with the previous version of this task + if prev_shifts and isinstance(prev_shifts, dict): + prev_shifts = convert_prev_shifts_to_new_format(prev_shifts, schedule) + prev_shifts_updated = True - overrides_priority = 0 - for calendar in current_calendars: - if calendar is not None: - current_shifts_result, current_users_result = get_current_shifts_from_ical( - calendar, - schedule, - overrides_priority, - ) - if overrides_priority == 0 and current_shifts_result: - overrides_priority = max([current_shifts_result[uid]["priority"] for uid in current_shifts_result]) + 1 - current_shifts.update(current_shifts_result) - current_users.update(current_users_result) - - recalculate_shifts_with_respect_to_priority(current_shifts, current_users) - - # drop events that don't intersection with current time - drop = [] - for uid, current_shift in current_shifts.items(): - if not current_shift["start"] < now < current_shift["end"]: - drop.append(uid) - for item in drop: - current_shifts.pop(item) - - # compare events from prev and current shifts - prev_shifts = json.loads(schedule.current_shifts) if not schedule.empty_oncall else {} # convert datetimes which was dumped to str back to datetime to calculate shift diff correct str_format = "%Y-%m-%d %X%z" - for prev_shift in prev_shifts.values(): + for prev_shift in prev_shifts: prev_shift["start"] = datetime.datetime.strptime(prev_shift["start"], str_format) prev_shift["end"] = datetime.datetime.strptime(prev_shift["end"], str_format) - shift_changed, diff_uids = calculate_shift_diff(current_shifts, prev_shifts) + shift_changed, diff_shifts = calculate_shift_diff(current_shifts, prev_shifts) - if shift_changed: - task_logger.info(f"shifts_changed: {diff_uids}") - # Get only new/changed shifts to send a reminder message. - new_shifts = [] - for uid in diff_uids: - # using copy to not to mutate original current_shifts dict which will be stored in db as current_shifts - new_shift = copy(current_shifts[uid]) - # replace users' pk by objects to make reminder message from new shifts - new_shift["users"] = current_users[uid] - new_shifts.append(new_shift) - new_shifts = sorted(new_shifts, key=lambda shift: shift["start"]) - - if len(new_shifts) != 0: - days_to_lookup = (new_shifts[-1]["end"].date() - now.date()).days + 1 - days_to_lookup = max([days_to_lookup, MIN_DAYS_TO_LOOKUP_FOR_THE_END_OF_EVENT]) - else: - days_to_lookup = MIN_DAYS_TO_LOOKUP_FOR_THE_END_OF_EVENT - - next_shifts = {} - next_overrides_priority = 0 - - for calendar in current_calendars: - if calendar is not None: - next_shifts_result = get_next_shifts_from_ical( - calendar, - schedule, - next_overrides_priority, - days_to_lookup=days_to_lookup, - ) - if next_overrides_priority == 0 and next_shifts_result: - next_overrides_priority = ( - max([next_shifts_result[uid]["priority"] for uid in next_shifts_result]) + 1 - ) - - next_shifts.update(next_shifts_result) - - recalculate_shifts_with_respect_to_priority(next_shifts) - - # drop events that already started - drop = [] - for uid, next_shift in next_shifts.items(): - if now > next_shift["start"]: - drop.append(uid) - for item in drop: - next_shifts.pop(item) - - next_shifts_from_ical = sorted(next_shifts.values(), key=lambda shift: shift["start"]) - - upcoming_shifts = [] - # Add the earliest next_shift - if len(next_shifts_from_ical) > 0: - earliest_shift = next_shifts_from_ical[0] - upcoming_shifts.append(earliest_shift) - # Check if there are next shifts with the same start as the earliest - for shift in next_shifts_from_ical[1:]: - if shift["start"] == earliest_shift["start"]: - upcoming_shifts.append(shift) - - empty_oncall = len(current_shifts) == 0 - if empty_oncall: - schedule.empty_oncall = True - else: - schedule.empty_oncall = False + # Do not notify if there is no difference between current and previous shifts + if not shift_changed: + task_logger.info(f"No shift diff found for schedule {schedule_pk}, organization {schedule.organization_id}") + # If prev shifts were converted to a new format, update related field in db + if prev_shifts_updated: schedule.current_shifts = json.dumps(current_shifts, default=str) + schedule.save(update_fields=["current_shifts"]) + return - schedule.save(update_fields=["current_shifts", "empty_oncall"]) + new_shifts = sorted(diff_shifts, key=lambda shift: shift["start"]) - if len(new_shifts) > 0 or empty_oncall: - task_logger.info(f"new_shifts: {new_shifts}") + # get days_to_lookup for next shifts + if len(new_shifts) != 0: + max_end_date = max([shift["end"].date() for shift in new_shifts]) + days_to_lookup = (max_end_date - now.date()).days + 1 + days_to_lookup = max([days_to_lookup, MIN_DAYS_TO_LOOKUP_FOR_THE_END_OF_EVENT]) + else: + days_to_lookup = MIN_DAYS_TO_LOOKUP_FOR_THE_END_OF_EVENT + + datetime_end = now + datetime.timedelta(days=days_to_lookup) + + next_shifts_unfiltered = schedule.final_events(now, datetime_end, with_empty=False, with_gap=False) + # drop events that already started + next_shifts = [] + for next_shift in next_shifts_unfiltered: + if now < next_shift["start"]: + next_shifts.append(next_shift) + + upcoming_shifts = [] + # Add the earliest next_shift + if len(next_shifts) > 0: + earliest_shift = next_shifts[0] + upcoming_shifts.append(earliest_shift) + # Check if there are next shifts with the same start as the earliest + for shift in next_shifts[1:]: + if shift["start"] == earliest_shift["start"]: + upcoming_shifts.append(shift) + + schedule.empty_oncall = len(current_shifts) == 0 + if not schedule.empty_oncall: + schedule.current_shifts = json.dumps(current_shifts, default=str) + + schedule.save(update_fields=["current_shifts", "empty_oncall"]) + + if len(new_shifts) > 0 or schedule.empty_oncall: + task_logger.info(f"new_shifts: {new_shifts}") + if schedule.notify_oncall_shift_freq != OnCallSchedule.NotifyOnCallShiftFreq.NEVER: slack_client = SlackClientWithErrorHandling(schedule.organization.slack_team_identity.bot_access_token) step = scenario_step.ScenarioStep.get_step("schedules", "EditScheduleShiftNotifyStep") - report_blocks = step.get_report_blocks_ical(new_shifts, upcoming_shifts, schedule, empty_oncall) + report_blocks = step.get_report_blocks_ical(new_shifts, upcoming_shifts, schedule, schedule.empty_oncall) - if schedule.notify_oncall_shift_freq != OnCallSchedule.NotifyOnCallShiftFreq.NEVER: - try: - slack_client.api_call( - "chat.postMessage", - channel=schedule.channel, - blocks=report_blocks, - text=f"On-call shift for schedule {schedule.name} has changed", - ) - except SlackAPITokenException: - pass - except SlackAPIException as e: - if e.response["error"] == "channel_not_found": - print(e) - elif e.response["error"] == "is_archived": - print(e) - elif e.response["error"] == "invalid_auth": - print(e) - else: - raise e + try: + slack_client.api_call( + "chat.postMessage", + channel=schedule.channel, + blocks=report_blocks, + text=f"On-call shift for schedule {schedule.name} has changed", + ) + except SlackAPITokenException: + pass + except SlackAPIException as e: + expected_exceptions = ["channel_not_found", "is_archived", "invalid_auth"] + if e.response["error"] in expected_exceptions: + print(e) + else: + raise e diff --git a/engine/apps/alerts/tests/test_notify_ical_schedule_shift.py b/engine/apps/alerts/tests/test_notify_ical_schedule_shift.py index e5396eca..d5925c9d 100644 --- a/engine/apps/alerts/tests/test_notify_ical_schedule_shift.py +++ b/engine/apps/alerts/tests/test_notify_ical_schedule_shift.py @@ -3,14 +3,13 @@ import json import textwrap from unittest.mock import Mock, patch -import icalendar import pytest import pytz from django.utils import timezone -from apps.alerts.tasks.notify_ical_schedule_shift import get_current_shifts_from_ical, notify_ical_schedule_shift +from apps.alerts.tasks.notify_ical_schedule_shift import notify_ical_schedule_shift from apps.schedules.ical_utils import memoized_users_in_ical -from apps.schedules.models import CustomOnCallShift, OnCallScheduleCalendar, OnCallScheduleICal +from apps.schedules.models import CustomOnCallShift, OnCallScheduleCalendar, OnCallScheduleICal, OnCallScheduleWeb ICAL_DATA = """ BEGIN:VCALENDAR @@ -72,7 +71,7 @@ def test_current_overrides_ical_schedule_is_none( ) # this should not raise - notify_ical_schedule_shift(ical_schedule.oncallschedule_ptr_id) + notify_ical_schedule_shift(ical_schedule.pk) @pytest.mark.django_db @@ -102,7 +101,7 @@ def test_next_shift_notification_long_shifts( with patch("apps.alerts.tasks.notify_ical_schedule_shift.datetime", Mock(wraps=datetime)) as mock_datetime: mock_datetime.datetime.now.return_value = datetime.datetime(2021, 9, 29, 12, 0, tzinfo=pytz.UTC) with patch("apps.slack.slack_client.SlackClientWithErrorHandling.api_call") as mock_slack_api_call: - notify_ical_schedule_shift(ical_schedule.oncallschedule_ptr_id) + notify_ical_schedule_shift(ical_schedule.pk) slack_blocks = mock_slack_api_call.call_args_list[0][1]["blocks"] notification = slack_blocks[0]["text"]["text"] @@ -176,12 +175,12 @@ def test_overrides_changes_no_current_no_triggering_notification( schedule_class=OnCallScheduleCalendar, name="test_schedule", channel="channel", - prev_ical_file_overrides=ical_before, - cached_ical_file_overrides=ical_after, + prev_ical_file_overrides=None, + cached_ical_file_overrides=ical_before, ) now = timezone.now().replace(microsecond=0) - start_date = now - timezone.timedelta(days=7) + start_date = now - timezone.timedelta(days=7, minutes=1) data = { "start": start_date, @@ -197,14 +196,15 @@ def test_overrides_changes_no_current_no_triggering_notification( on_call_shift.schedules.add(schedule) # setup current shifts before checking/triggering for notifications - calendar = icalendar.Calendar.from_ical(schedule._ical_file_primary) - current_shifts, _ = get_current_shifts_from_ical(calendar, schedule, 0) + current_shifts = schedule.final_events(now, now, False, False) schedule.current_shifts = json.dumps(current_shifts, default=str) schedule.empty_oncall = False + schedule.cached_ical_file_overrides = ical_after + schedule.prev_ical_file_overrides = ical_before schedule.save() with patch("apps.slack.slack_client.SlackClientWithErrorHandling.api_call") as mock_slack_api_call: - notify_ical_schedule_shift(schedule.oncallschedule_ptr_id) + notify_ical_schedule_shift(schedule.pk) assert not mock_slack_api_call.called @@ -231,7 +231,7 @@ def test_no_changes_no_triggering_notification( ) now = timezone.now().replace(microsecond=0) - start_date = now - timezone.timedelta(days=7) + start_date = now - timezone.timedelta(days=7, minutes=1) data = { "start": start_date, "rotation_start": start_date, @@ -246,14 +246,13 @@ def test_no_changes_no_triggering_notification( on_call_shift.schedules.add(schedule) # setup current shifts before checking/triggering for notifications - calendar = icalendar.Calendar.from_ical(schedule._ical_file_primary) - current_shifts, _ = get_current_shifts_from_ical(calendar, schedule, 0) + current_shifts = schedule.final_events(now, now, False, False) schedule.current_shifts = json.dumps(current_shifts, default=str) schedule.empty_oncall = False schedule.save() with patch("apps.slack.slack_client.SlackClientWithErrorHandling.api_call") as mock_slack_api_call: - notify_ical_schedule_shift(schedule.oncallschedule_ptr_id) + notify_ical_schedule_shift(schedule.pk) assert not mock_slack_api_call.called @@ -301,11 +300,146 @@ def test_current_shift_changes_trigger_notification( schedule.save() with patch("apps.slack.slack_client.SlackClientWithErrorHandling.api_call") as mock_slack_api_call: - notify_ical_schedule_shift(schedule.oncallschedule_ptr_id) + notify_ical_schedule_shift(schedule.pk) assert mock_slack_api_call.called +@pytest.mark.django_db +def test_next_shift_changes_no_triggering_notification( + make_organization_and_user_with_slack_identities, + make_user, + make_schedule, + make_on_call_shift, +): + organization, _, _, _ = make_organization_and_user_with_slack_identities() + user1 = make_user(organization=organization, username="user1") + user2 = make_user(organization=organization, username="user2") + # clear users pks <-> organization cache (persisting between tests) + memoized_users_in_ical.cache_clear() + + schedule = make_schedule( + organization, + schedule_class=OnCallScheduleCalendar, + name="test_schedule", + channel="channel", + prev_ical_file_overrides=None, + cached_ical_file_overrides=None, + ) + + now = timezone.now().replace(microsecond=0) + start_date_1 = now - datetime.timedelta(days=7, minutes=1) + data_1 = { + "start": start_date_1, + "rotation_start": start_date_1, + "duration": datetime.timedelta(seconds=3600 * 24), + "priority_level": 1, + "frequency": CustomOnCallShift.FREQUENCY_DAILY, + } + on_call_shift_1 = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data_1 + ) + on_call_shift_1.add_rolling_users([[user1]]) + on_call_shift_1.schedules.add(schedule) + + start_date_2 = now + datetime.timedelta(minutes=10) + data_2 = { + "start": start_date_2, + "rotation_start": start_date_2, + "duration": datetime.timedelta(seconds=3600 * 24), + "priority_level": 2, + "frequency": CustomOnCallShift.FREQUENCY_DAILY, + } + on_call_shift_2 = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data_2 + ) + on_call_shift_2.add_rolling_users([[user1]]) + on_call_shift_2.schedules.add(schedule) + + schedule.refresh_ical_file() + + # setup empty current shifts before checking/triggering for notifications + current_shifts = schedule.final_events(now, now, False, False) + schedule.current_shifts = json.dumps(current_shifts, default=str) + schedule.empty_oncall = False + schedule.save() + + on_call_shift_2.add_rolling_users([[user2]]) + schedule.refresh_ical_file() + + with patch("apps.slack.slack_client.SlackClientWithErrorHandling.api_call") as mock_slack_api_call: + notify_ical_schedule_shift(schedule.pk) + + assert not mock_slack_api_call.called + + +@pytest.mark.django_db +def test_lower_priority_changes_no_triggering_notification( + make_organization_and_user_with_slack_identities, + make_user, + make_schedule, + make_on_call_shift, +): + organization, _, _, _ = make_organization_and_user_with_slack_identities() + user1 = make_user(organization=organization, username="user1") + user2 = make_user(organization=organization, username="user2") + # clear users pks <-> organization cache (persisting between tests) + memoized_users_in_ical.cache_clear() + + schedule = make_schedule( + organization, + schedule_class=OnCallScheduleCalendar, + name="test_schedule", + channel="channel", + prev_ical_file_overrides=None, + cached_ical_file_overrides=None, + ) + + now = timezone.now().replace(microsecond=0) + start_date = now - datetime.timedelta(days=7, minutes=1) + data_1 = { + "start": start_date, + "rotation_start": start_date, + "duration": datetime.timedelta(seconds=3600 * 24), + "priority_level": 2, + "frequency": CustomOnCallShift.FREQUENCY_DAILY, + } + on_call_shift_1 = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data_1 + ) + on_call_shift_1.add_rolling_users([[user1]]) + on_call_shift_1.schedules.add(schedule) + + data_2 = { + "start": start_date, + "rotation_start": start_date, + "duration": datetime.timedelta(seconds=3600 * 24), + "priority_level": 1, + "frequency": CustomOnCallShift.FREQUENCY_DAILY, + } + on_call_shift_2 = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data_2 + ) + on_call_shift_2.add_rolling_users([[user1]]) + on_call_shift_2.schedules.add(schedule) + + schedule.refresh_ical_file() + + # setup empty current shifts before checking/triggering for notifications + current_shifts = schedule.final_events(now, now, False, False) + schedule.current_shifts = json.dumps(current_shifts, default=str) + schedule.empty_oncall = False + schedule.save() + + on_call_shift_2.add_rolling_users([[user2]]) + schedule.refresh_ical_file() + + with patch("apps.slack.slack_client.SlackClientWithErrorHandling.api_call") as mock_slack_api_call: + notify_ical_schedule_shift(schedule.pk) + + assert not mock_slack_api_call.called + + @pytest.mark.django_db def test_vtimezone_changes_no_triggering_notification( make_organization_and_user_with_slack_identities, @@ -414,20 +548,213 @@ def test_vtimezone_changes_no_triggering_notification( name="test_ical_schedule", channel="channel", ical_url_primary="url", - prev_ical_file_primary=ical_before, - cached_ical_file_primary=ical_after, + prev_ical_file_primary=None, + cached_ical_file_primary=ical_before, prev_ical_file_overrides=None, cached_ical_file_overrides=None, ) # setup current shifts before checking/triggering for notifications - calendar = icalendar.Calendar.from_ical(ical_before) - current_shifts, _ = get_current_shifts_from_ical(calendar, schedule, 0) + now = datetime.datetime.now(timezone.utc) + current_shifts = schedule.final_events(now, now, False, False) + schedule.current_shifts = json.dumps(current_shifts, default=str) + schedule.empty_oncall = False + # update schedule cached ical to ical_after + schedule.prev_ical_file_primary = ical_before + schedule.cached_ical_file_primary = ical_after + schedule.save() + + with patch("apps.slack.slack_client.SlackClientWithErrorHandling.api_call") as mock_slack_api_call: + notify_ical_schedule_shift(schedule.pk) + + assert not mock_slack_api_call.called + + +@pytest.mark.django_db +def test_no_changes_no_triggering_notification_from_old_to_new_task_version( + make_organization_and_user_with_slack_identities, + make_user, + make_schedule, + make_on_call_shift, +): + organization, _, _, _ = make_organization_and_user_with_slack_identities() + user1 = make_user(organization=organization, username="user1") + # clear users pks <-> organization cache (persisting between tests) + memoized_users_in_ical.cache_clear() + + schedule = make_schedule( + organization, + schedule_class=OnCallScheduleCalendar, + name="test_schedule", + channel="channel", + prev_ical_file_overrides=None, + cached_ical_file_overrides=None, + ) + + now = timezone.now().replace(microsecond=0) + start_date = now - timezone.timedelta(days=7) + data = { + "start": start_date, + "rotation_start": start_date, + "duration": timezone.timedelta(seconds=3600 * 24), + "priority_level": 1, + "frequency": CustomOnCallShift.FREQUENCY_DAILY, + } + on_call_shift = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data + ) + on_call_shift.add_rolling_users([[user1]]) + on_call_shift.schedules.add(schedule) + + # setup current shifts with old version of shifts structure before checking/triggering for notifications + current_shifts = { + "test_shift_uid": { + "users": [user1.pk], + "start": start_date, + "end": start_date + data["duration"], + "all_day": False, + "priority": data["priority_level"], + "priority_increased_by": 0, + } + } schedule.current_shifts = json.dumps(current_shifts, default=str) schedule.empty_oncall = False schedule.save() with patch("apps.slack.slack_client.SlackClientWithErrorHandling.api_call") as mock_slack_api_call: - notify_ical_schedule_shift(schedule.oncallschedule_ptr_id) + notify_ical_schedule_shift(schedule.pk) assert not mock_slack_api_call.called + + +@pytest.mark.django_db +def test_current_shift_changes_trigger_notification_from_old_to_new_task_version( + make_organization_and_user_with_slack_identities, + make_user, + make_schedule, + make_on_call_shift, +): + organization, _, _, _ = make_organization_and_user_with_slack_identities() + user1 = make_user(organization=organization, username="user1") + user2 = make_user(organization=organization, username="user2") + # clear users pks <-> organization cache (persisting between tests) + memoized_users_in_ical.cache_clear() + + schedule = make_schedule( + organization, + schedule_class=OnCallScheduleCalendar, + name="test_schedule", + channel="channel", + prev_ical_file_overrides=None, + cached_ical_file_overrides=None, + ) + + now = timezone.now().replace(microsecond=0) + start_date = now - datetime.timedelta(days=7) + data = { + "start": start_date, + "rotation_start": start_date, + "duration": datetime.timedelta(seconds=3600 * 24), + "priority_level": 1, + "frequency": CustomOnCallShift.FREQUENCY_DAILY, + } + on_call_shift = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data + ) + on_call_shift.add_rolling_users([[user1]]) + on_call_shift.schedules.add(schedule) + schedule.refresh_ical_file() + + # setup current shifts with old version of shifts structure before checking/triggering for notifications + current_shifts = { + "test_shift_uid": { + "users": [user1.pk], + "start": start_date, + "end": start_date + data["duration"], + "all_day": False, + "priority": data["priority_level"], + "priority_increased_by": 0, + } + } + schedule.current_shifts = json.dumps(current_shifts, default=str) + schedule.empty_oncall = False + schedule.save() + + on_call_shift.add_rolling_users([[user2]]) + schedule.refresh_ical_file() + + with patch("apps.slack.slack_client.SlackClientWithErrorHandling.api_call") as mock_slack_api_call: + notify_ical_schedule_shift(schedule.pk) + + assert mock_slack_api_call.called + + +@pytest.mark.django_db +def test_next_shift_notification_long_and_short_shifts( + make_organization_and_user_with_slack_identities, + make_user, + make_schedule, + make_on_call_shift, +): + organization, _, _, _ = make_organization_and_user_with_slack_identities() + user1 = make_user(organization=organization, username="user1") + user2 = make_user(organization=organization, username="user2") + user3 = make_user(organization=organization, username="user3") + # clear users pks <-> organization cache (persisting between tests) + memoized_users_in_ical.cache_clear() + + schedule = make_schedule( + organization, + schedule_class=OnCallScheduleWeb, + name="test_schedule", + channel="channel", + prev_ical_file_overrides=None, + cached_ical_file_overrides=None, + ) + + now = timezone.now().replace(microsecond=0) + start_date_1 = now - datetime.timedelta(days=1) + data_1 = { + "start": start_date_1, + "rotation_start": start_date_1, + "duration": datetime.timedelta(seconds=3600 * 24 * 7), # one week duration + "priority_level": 1, + "frequency": CustomOnCallShift.FREQUENCY_WEEKLY, + "schedule": schedule, + } + on_call_shift_1 = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data_1 + ) + on_call_shift_1.add_rolling_users([[user1], [user2]]) + + start_date_2 = now - datetime.timedelta(hours=1) + data_2 = { + "start": start_date_2, + "rotation_start": start_date_2, + "duration": datetime.timedelta(seconds=3600 * 24), + "priority_level": 1, + "frequency": CustomOnCallShift.FREQUENCY_WEEKLY, + "schedule": schedule, + } + on_call_shift_2 = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data_2 + ) + on_call_shift_2.add_rolling_users([[user3]]) + + schedule.refresh_ical_file() + + # setup empty current shifts before checking/triggering for notifications + schedule.current_shifts = json.dumps({}, default=str) + schedule.empty_oncall = False + schedule.save() + + with patch("apps.slack.slack_client.SlackClientWithErrorHandling.api_call") as mock_slack_api_call: + notify_ical_schedule_shift(schedule.pk) + + assert mock_slack_api_call.called + notification = mock_slack_api_call.call_args[1]["blocks"][0]["text"]["text"] + new_shift_notification, next_shift_notification = notification.split("\n\n") + + assert "*New on-call shift:*\n[L1] user1" in new_shift_notification + assert "[L1] user3" in new_shift_notification + assert "*Next on-call shift:*\n[L1] user2" in notification diff --git a/engine/apps/schedules/ical_utils.py b/engine/apps/schedules/ical_utils.py index 0324a462..b20cb0ea 100644 --- a/engine/apps/schedules/ical_utils.py +++ b/engine/apps/schedules/ical_utils.py @@ -504,20 +504,23 @@ def ical_date_to_datetime(date, tz, start): return date, all_day -def calculate_shift_diff(first_shift, second_shift): - fields_to_compare = ["users", "end", "start", "all_day", "priority"] +def calculate_shift_diff(shifts: list, prev_shifts: list) -> typing.Tuple[bool, list]: + """ + Get shifts diff comparing with the previous shifts + """ + fields_to_compare = ["users", "end", "start", "all_day", "priority_level", "shift"] - shift_changed = set(first_shift.keys()) != set(second_shift.keys()) - if not shift_changed: - diff = set() - for k, v in first_shift.items(): - for f in fields_to_compare: - if v.get(f) != second_shift[k].get(f): - shift_changed = True - diff.add(k) - break - else: - diff = set(first_shift.keys()) - set(second_shift.keys()) + shifts_fields = [{k: v for k, v in shift.items() if k in fields_to_compare} for shift in shifts] + prev_shifts_fields = [{k: v for k, v in shift.items() if k in fields_to_compare} for shift in prev_shifts] + + shift_changed = len(shifts) != len(prev_shifts) + + diff = [] + + for idx, shift in enumerate(shifts_fields): + if shift not in prev_shifts_fields: + shift_changed = True + diff.append(shifts[idx]) return shift_changed, diff @@ -611,29 +614,6 @@ def user_ical_export(user: "User", schedules: "OnCallScheduleQuerySet") -> bytes return ical_obj.to_ical() -def list_of_gaps_in_schedule( - schedule: "OnCallSchedule", start_date: datetime.date, end_date: datetime.date -) -> DatetimeIntervals: - calendars = schedule.get_icalendars() - intervals: DatetimeIntervals = [] - start_datetime = datetime.datetime.combine(start_date, datetime.time.min) + datetime.timedelta(milliseconds=1) - start_datetime = start_datetime.astimezone(pytz.UTC) - end_datetime = datetime.datetime.combine(end_date, datetime.time.max).astimezone(pytz.UTC) - - for calendar in calendars: - if calendar is not None: - calendar_tz = get_icalendar_tz_or_utc(calendar) - events = ical_events.get_events_from_ical_between( - calendar, - start_datetime, - end_datetime, - ) - for event in events: - start, end, _ = event_start_end_all_day_with_respect_to_type(event, calendar_tz) - intervals.append(DatetimeInterval(start, end)) - return detect_gaps(intervals, start_datetime, end_datetime) - - def detect_gaps(intervals: DatetimeIntervals, start: datetime.datetime, end: datetime.datetime) -> DatetimeIntervals: gaps: DatetimeIntervals = [] intervals = sorted(intervals, key=lambda dt: dt.start) diff --git a/engine/apps/schedules/models/on_call_schedule.py b/engine/apps/schedules/models/on_call_schedule.py index 5f5c49f7..56963e08 100644 --- a/engine/apps/schedules/models/on_call_schedule.py +++ b/engine/apps/schedules/models/on_call_schedule.py @@ -38,7 +38,6 @@ from apps.schedules.ical_utils import ( fetch_ical_file_or_get_error, get_oncall_users_for_multiple_schedules, list_of_empty_shifts_in_schedule, - list_of_gaps_in_schedule, list_of_oncall_shifts_from_ical, ) from apps.schedules.models import CustomOnCallShift @@ -279,9 +278,10 @@ class OnCallSchedule(PolymorphicModel): (self.prev_ical_file_overrides, self.cached_ical_file_overrides), ] - def check_gaps_for_next_week(self): - today = timezone.now().date() - gaps = list_of_gaps_in_schedule(self, today, today + datetime.timedelta(days=7)) + def check_gaps_for_next_week(self) -> bool: + today = timezone.now() + events = self.final_events(today, today + datetime.timedelta(days=7)) + gaps = [event for event in events if event["is_gap"] and not event["is_empty"]] has_gaps = len(gaps) != 0 self.has_gaps = has_gaps self.save(update_fields=["has_gaps"]) @@ -367,7 +367,9 @@ class OnCallSchedule(PolymorphicModel): end = shift["end"] - datetime.timedelta(days=1) if all_day else shift["end"] if all_day and all_day_datetime: start = datetime.datetime.combine(start, datetime.datetime.min.time(), tzinfo=pytz.UTC) - end = datetime.datetime.combine(end, datetime.datetime.max.time(), tzinfo=pytz.UTC) + end = datetime.datetime.combine(end, datetime.datetime.max.time(), tzinfo=pytz.UTC).replace( + microsecond=0 + ) is_gap = shift.get("is_gap", False) shift_json: ScheduleEvent = { "all_day": all_day, @@ -403,9 +405,17 @@ class OnCallSchedule(PolymorphicModel): return events - def final_events(self, datetime_start: datetime.datetime, datetime_end: datetime.datetime) -> ScheduleEvents: + def final_events( + self, + datetime_start: datetime.datetime, + datetime_end: datetime.datetime, + with_empty: bool = True, + with_gap: bool = True, + ) -> ScheduleEvents: """Return schedule final events, after resolving shifts and overrides.""" - events = self.filter_events(datetime_start, datetime_end, with_empty=True, with_gap=True, all_day_datetime=True) + events = self.filter_events( + datetime_start, datetime_end, with_empty=with_empty, with_gap=with_gap, all_day_datetime=True + ) events = self._resolve_schedule(events, datetime_start, datetime_end) return events diff --git a/engine/apps/schedules/tasks/notify_about_empty_shifts_in_schedule.py b/engine/apps/schedules/tasks/notify_about_empty_shifts_in_schedule.py index 1eed8863..a4a123e6 100644 --- a/engine/apps/schedules/tasks/notify_about_empty_shifts_in_schedule.py +++ b/engine/apps/schedules/tasks/notify_about_empty_shifts_in_schedule.py @@ -109,7 +109,7 @@ def notify_about_empty_shifts_in_schedule(schedule_pk): f'From {empty_shift.start.strftime("%b %d")} to {empty_shift.end.strftime("%b %d")}\n' ) text += all_day_text - text += f"*All-day* event in {empty_shift.calendar_tz} TZ\n" + text += '*All-day* event in "UTC" TZ\n' else: text += f"From {format_datetime_to_slack_with_time(start_timestamp)} to {format_datetime_to_slack_with_time(end_timestamp)} (your TZ)\n" text += f"_From {OnCallSchedule.CALENDAR_TYPE_VERBAL[empty_shift.calendar_type]} calendar_\n" diff --git a/engine/apps/schedules/tasks/notify_about_gaps_in_schedule.py b/engine/apps/schedules/tasks/notify_about_gaps_in_schedule.py index 40f0c1b9..5ef72f45 100644 --- a/engine/apps/schedules/tasks/notify_about_gaps_in_schedule.py +++ b/engine/apps/schedules/tasks/notify_about_gaps_in_schedule.py @@ -1,9 +1,10 @@ +import datetime + import pytz from celery.utils.log import get_task_logger from django.core.cache import cache from django.utils import timezone -from apps.schedules.ical_utils import list_of_gaps_in_schedule from apps.slack.utils import format_datetime_to_slack_with_time, post_message_to_channel from common.custom_celery_tasks import shared_dedicated_queue_retry_task @@ -79,20 +80,21 @@ def notify_about_gaps_in_schedule(schedule_pk): task_logger.info(f"Tried to notify_about_gaps_in_schedule for non-existing schedule {schedule_pk}") return - today = timezone.now().date() - gaps = list_of_gaps_in_schedule(schedule, today, today + timezone.timedelta(days=7)) - schedule.gaps_report_sent_at = today + now = timezone.now() + events = schedule.final_events(now, now + datetime.timedelta(days=7)) + gaps = [event for event in events if event["is_gap"] and not event["is_empty"]] + schedule.gaps_report_sent_at = now.date() if len(gaps) != 0: schedule.has_gaps = True text = f"There are time periods that are unassigned in *{schedule.name}* on-call schedule.\n" for idx, gap in enumerate(gaps): - if gap.start: - start_verbal = format_datetime_to_slack_with_time(gap.start.astimezone(pytz.UTC).timestamp()) + if gap["start"]: + start_verbal = format_datetime_to_slack_with_time(gap["start"].astimezone(pytz.UTC).timestamp()) else: start_verbal = "..." - if gap.end: - end_verbal = format_datetime_to_slack_with_time(gap.end.astimezone(pytz.UTC).timestamp()) + if gap["end"]: + end_verbal = format_datetime_to_slack_with_time(gap["end"].astimezone(pytz.UTC).timestamp()) else: end_verbal = "..." text += f"From {start_verbal} to {end_verbal} (your TZ)\n" diff --git a/engine/apps/schedules/tests/test_notify_about_empty_shifts_in_schedule.py b/engine/apps/schedules/tests/test_notify_about_empty_shifts_in_schedule.py new file mode 100644 index 00000000..c4680715 --- /dev/null +++ b/engine/apps/schedules/tests/test_notify_about_empty_shifts_in_schedule.py @@ -0,0 +1,170 @@ +import datetime +from unittest.mock import patch + +import pytest +from django.utils import timezone + +from apps.api.permissions import LegacyAccessControlRole +from apps.schedules.ical_utils import memoized_users_in_ical +from apps.schedules.models import CustomOnCallShift, OnCallScheduleWeb +from apps.schedules.tasks import notify_about_empty_shifts_in_schedule + + +@pytest.mark.django_db +def test_no_empty_shifts_no_triggering_notification( + make_organization_and_user_with_slack_identities, + make_user, + make_schedule, + make_on_call_shift, +): + organization, _, _, _ = make_organization_and_user_with_slack_identities() + user1 = make_user(organization=organization, username="user1") + # clear users pks <-> organization cache (persisting between tests) + memoized_users_in_ical.cache_clear() + + schedule = make_schedule( + organization, + schedule_class=OnCallScheduleWeb, + name="test_schedule", + channel="channel", + prev_ical_file_overrides=None, + cached_ical_file_overrides=None, + ) + + now = timezone.now().replace(microsecond=0) + start_date = now - datetime.timedelta(days=7, minutes=1) + data = { + "start": start_date, + "rotation_start": start_date, + "duration": datetime.timedelta(seconds=3600 * 24), + "priority_level": 1, + "frequency": CustomOnCallShift.FREQUENCY_DAILY, + "schedule": schedule, + } + on_call_shift = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data + ) + on_call_shift.add_rolling_users([[user1]]) + schedule.refresh_ical_file() + + empty_shifts_report_sent_at = schedule.empty_shifts_report_sent_at + + with patch("apps.slack.slack_client.SlackClientWithErrorHandling.api_call") as mock_slack_api_call: + notify_about_empty_shifts_in_schedule(schedule.pk) + + assert not mock_slack_api_call.called + + schedule.refresh_from_db() + assert empty_shifts_report_sent_at != schedule.empty_shifts_report_sent_at + + +@pytest.mark.django_db +def test_empty_shifts_trigger_notification( + make_organization_and_user_with_slack_identities, + make_user, + make_schedule, + make_on_call_shift, +): + organization, _, _, _ = make_organization_and_user_with_slack_identities() + user1 = make_user(organization=organization, username="user1", role=LegacyAccessControlRole.VIEWER) + # clear users pks <-> organization cache (persisting between tests) + memoized_users_in_ical.cache_clear() + + schedule = make_schedule( + organization, + schedule_class=OnCallScheduleWeb, + name="test_schedule", + channel="channel", + prev_ical_file_overrides=None, + cached_ical_file_overrides=None, + ) + + now = timezone.now().replace(microsecond=0) + start_date = now - datetime.timedelta(days=7, minutes=1) + data = { + "start": start_date, + "rotation_start": start_date, + "duration": datetime.timedelta(seconds=3600 * 24), + "priority_level": 1, + "frequency": CustomOnCallShift.FREQUENCY_DAILY, + "schedule": schedule, + } + on_call_shift = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data + ) + on_call_shift.add_rolling_users([[user1]]) + schedule.refresh_ical_file() + + empty_shifts_report_sent_at = schedule.empty_shifts_report_sent_at + + with patch("apps.slack.slack_client.SlackClientWithErrorHandling.api_call") as mock_slack_api_call: + notify_about_empty_shifts_in_schedule(schedule.pk) + + assert mock_slack_api_call.called + + schedule.refresh_from_db() + assert empty_shifts_report_sent_at != schedule.empty_shifts_report_sent_at + assert schedule.has_empty_shifts + + +@pytest.mark.django_db +def test_empty_non_empty_shifts_trigger_notification( + make_organization_and_user_with_slack_identities, + make_user, + make_schedule, + make_on_call_shift, +): + organization, _, _, _ = make_organization_and_user_with_slack_identities() + user1 = make_user(organization=organization, username="user1") + user2 = make_user(organization=organization, username="user2", role=LegacyAccessControlRole.VIEWER) + # clear users pks <-> organization cache (persisting between tests) + memoized_users_in_ical.cache_clear() + + schedule = make_schedule( + organization, + schedule_class=OnCallScheduleWeb, + name="test_schedule", + channel="channel", + prev_ical_file_overrides=None, + cached_ical_file_overrides=None, + ) + # non-empty shift has higher priority + now = timezone.now().replace(microsecond=0) + start_date = now - datetime.timedelta(days=7, minutes=1) + data = { + "start": start_date, + "rotation_start": start_date, + "duration": datetime.timedelta(seconds=3600 * 24), + "priority_level": 2, + "frequency": CustomOnCallShift.FREQUENCY_DAILY, + "schedule": schedule, + } + on_call_shift_1 = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data + ) + on_call_shift_1.add_rolling_users([[user1]]) + + data = { + "start": start_date, + "rotation_start": start_date, + "duration": datetime.timedelta(seconds=3600 * 24), + "priority_level": 1, + "frequency": CustomOnCallShift.FREQUENCY_DAILY, + "schedule": schedule, + } + on_call_shift_2 = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data + ) + on_call_shift_2.add_rolling_users([[user2]]) + schedule.refresh_ical_file() + + empty_shifts_report_sent_at = schedule.empty_shifts_report_sent_at + + with patch("apps.slack.slack_client.SlackClientWithErrorHandling.api_call") as mock_slack_api_call: + notify_about_empty_shifts_in_schedule(schedule.pk) + + assert mock_slack_api_call.called + + schedule.refresh_from_db() + assert empty_shifts_report_sent_at != schedule.empty_shifts_report_sent_at + assert schedule.has_empty_shifts diff --git a/engine/apps/schedules/tests/test_notify_about_gaps_in_schedule.py b/engine/apps/schedules/tests/test_notify_about_gaps_in_schedule.py new file mode 100644 index 00000000..7f4946c3 --- /dev/null +++ b/engine/apps/schedules/tests/test_notify_about_gaps_in_schedule.py @@ -0,0 +1,281 @@ +import datetime +from unittest.mock import patch + +import pytest +from django.utils import timezone + +from apps.schedules.ical_utils import memoized_users_in_ical +from apps.schedules.models import CustomOnCallShift, OnCallScheduleWeb +from apps.schedules.tasks import notify_about_gaps_in_schedule + + +@pytest.mark.django_db +def test_no_gaps_no_triggering_notification( + make_organization_and_user_with_slack_identities, + make_user, + make_schedule, + make_on_call_shift, +): + organization, _, _, _ = make_organization_and_user_with_slack_identities() + user1 = make_user(organization=organization, username="user1") + # clear users pks <-> organization cache (persisting between tests) + memoized_users_in_ical.cache_clear() + + schedule = make_schedule( + organization, + schedule_class=OnCallScheduleWeb, + name="test_schedule", + channel="channel", + prev_ical_file_overrides=None, + cached_ical_file_overrides=None, + ) + + now = timezone.now().replace(microsecond=0) + start_date = now - datetime.timedelta(days=7, minutes=1) + data = { + "start": start_date, + "rotation_start": start_date, + "duration": datetime.timedelta(seconds=3600 * 24), + "priority_level": 1, + "frequency": CustomOnCallShift.FREQUENCY_DAILY, + "schedule": schedule, + } + on_call_shift = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data + ) + on_call_shift.add_rolling_users([[user1]]) + schedule.refresh_ical_file() + + gaps_report_sent_at = schedule.gaps_report_sent_at + + with patch("apps.slack.slack_client.SlackClientWithErrorHandling.api_call") as mock_slack_api_call: + notify_about_gaps_in_schedule(schedule.pk) + + assert not mock_slack_api_call.called + + schedule.refresh_from_db() + assert gaps_report_sent_at != schedule.gaps_report_sent_at + assert schedule.check_gaps_for_next_week() is False + + +@pytest.mark.django_db +def test_gaps_in_the_past_no_triggering_notification( + make_organization_and_user_with_slack_identities, + make_user, + make_schedule, + make_on_call_shift, +): + organization, _, _, _ = make_organization_and_user_with_slack_identities() + user1 = make_user(organization=organization, username="user1") + # clear users pks <-> organization cache (persisting between tests) + memoized_users_in_ical.cache_clear() + + schedule = make_schedule( + organization, + schedule_class=OnCallScheduleWeb, + name="test_schedule", + channel="channel", + prev_ical_file_overrides=None, + cached_ical_file_overrides=None, + ) + + now = timezone.now().replace(microsecond=0) + start_date_1 = now - datetime.timedelta(days=1, minutes=1) + data = { + "start": start_date_1, + "rotation_start": start_date_1, + "duration": datetime.timedelta(seconds=3600 * 24), + "priority_level": 1, + "frequency": CustomOnCallShift.FREQUENCY_DAILY, + "schedule": schedule, + } + on_call_shift_1 = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data + ) + on_call_shift_1.add_rolling_users([[user1]]) + + start_date_2 = now - datetime.timedelta(days=5, minutes=1) + until_date = start_date_2 + datetime.timedelta(days=3) + data = { + "start": start_date_2, + "rotation_start": start_date_2, + "duration": datetime.timedelta(seconds=3600 * 24), + "priority_level": 1, + "frequency": CustomOnCallShift.FREQUENCY_DAILY, + "schedule": schedule, + "until": until_date, + } + on_call_shift_2 = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data + ) + on_call_shift_2.add_rolling_users([[user1]]) + schedule.refresh_ical_file() + + gaps_report_sent_at = schedule.gaps_report_sent_at + + with patch("apps.slack.slack_client.SlackClientWithErrorHandling.api_call") as mock_slack_api_call: + notify_about_gaps_in_schedule(schedule.pk) + + assert not mock_slack_api_call.called + + schedule.refresh_from_db() + assert gaps_report_sent_at != schedule.gaps_report_sent_at + assert schedule.check_gaps_for_next_week() is False + + +@pytest.mark.django_db +def test_gaps_now_trigger_notification( + make_organization_and_user_with_slack_identities, + make_user, + make_schedule, + make_on_call_shift, +): + organization, _, _, _ = make_organization_and_user_with_slack_identities() + user1 = make_user(organization=organization, username="user1") + # clear users pks <-> organization cache (persisting between tests) + memoized_users_in_ical.cache_clear() + + schedule = make_schedule( + organization, + schedule_class=OnCallScheduleWeb, + name="test_schedule", + channel="channel", + prev_ical_file_overrides=None, + cached_ical_file_overrides=None, + ) + + now = timezone.now().replace(microsecond=0) + start_date = now - datetime.timedelta(days=1, minutes=1) + data = { + "start": start_date, + "rotation_start": start_date, + "duration": datetime.timedelta(seconds=3600 * 24), + "priority_level": 1, + "frequency": CustomOnCallShift.FREQUENCY_DAILY, + "schedule": schedule, + "interval": 2, + } + on_call_shift = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data + ) + on_call_shift.add_rolling_users([[user1]]) + schedule.refresh_ical_file() + + gaps_report_sent_at = schedule.gaps_report_sent_at + + assert schedule.has_gaps is False + + with patch("apps.slack.slack_client.SlackClientWithErrorHandling.api_call") as mock_slack_api_call: + notify_about_gaps_in_schedule(schedule.pk) + + assert mock_slack_api_call.called + + schedule.refresh_from_db() + assert gaps_report_sent_at != schedule.gaps_report_sent_at + assert schedule.has_gaps is True + assert schedule.check_gaps_for_next_week() is True + + +@pytest.mark.django_db +def test_gaps_near_future_trigger_notification( + make_organization_and_user_with_slack_identities, + make_user, + make_schedule, + make_on_call_shift, +): + organization, _, _, _ = make_organization_and_user_with_slack_identities() + user1 = make_user(organization=organization, username="user1") + # clear users pks <-> organization cache (persisting between tests) + memoized_users_in_ical.cache_clear() + + schedule = make_schedule( + organization, + schedule_class=OnCallScheduleWeb, + name="test_schedule", + channel="channel", + prev_ical_file_overrides=None, + cached_ical_file_overrides=None, + ) + + now = timezone.now().replace(microsecond=0) + start_date = now - datetime.timedelta(days=7, minutes=1) + until_date = now + datetime.timedelta(days=3) + data = { + "start": start_date, + "rotation_start": start_date, + "duration": datetime.timedelta(seconds=3600 * 24), + "priority_level": 1, + "frequency": CustomOnCallShift.FREQUENCY_DAILY, + "schedule": schedule, + "until": until_date, + } + on_call_shift = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data + ) + on_call_shift.add_rolling_users([[user1]]) + schedule.refresh_ical_file() + + gaps_report_sent_at = schedule.gaps_report_sent_at + + assert schedule.has_gaps is False + + with patch("apps.slack.slack_client.SlackClientWithErrorHandling.api_call") as mock_slack_api_call: + notify_about_gaps_in_schedule(schedule.pk) + + assert mock_slack_api_call.called + + schedule.refresh_from_db() + assert gaps_report_sent_at != schedule.gaps_report_sent_at + assert schedule.has_gaps is True + assert schedule.check_gaps_for_next_week() is True + + +@pytest.mark.django_db +def test_gaps_later_than_7_days_no_triggering_notification( + make_organization_and_user_with_slack_identities, + make_user, + make_schedule, + make_on_call_shift, +): + organization, _, _, _ = make_organization_and_user_with_slack_identities() + user1 = make_user(organization=organization, username="user1") + # clear users pks <-> organization cache (persisting between tests) + memoized_users_in_ical.cache_clear() + + now = timezone.now().replace(microsecond=0) + + schedule = make_schedule( + organization, + schedule_class=OnCallScheduleWeb, + name="test_schedule", + channel="channel", + prev_ical_file_overrides=None, + cached_ical_file_overrides=None, + ) + start_date = now - datetime.timedelta(days=7, minutes=1) + until_date = now + datetime.timedelta(days=8) + data = { + "start": start_date, + "rotation_start": start_date, + "duration": datetime.timedelta(seconds=3600 * 24), + "priority_level": 1, + "frequency": CustomOnCallShift.FREQUENCY_DAILY, + "schedule": schedule, + "until": until_date, + } + on_call_shift = make_on_call_shift( + organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data + ) + on_call_shift.add_rolling_users([[user1]]) + schedule.refresh_ical_file() + + gaps_report_sent_at = schedule.gaps_report_sent_at + + with patch("apps.slack.slack_client.SlackClientWithErrorHandling.api_call") as mock_slack_api_call: + notify_about_gaps_in_schedule(schedule.pk) + + assert not mock_slack_api_call.called + + schedule.refresh_from_db() + assert gaps_report_sent_at != schedule.gaps_report_sent_at + assert schedule.check_gaps_for_next_week() is False diff --git a/engine/apps/schedules/tests/test_on_call_schedule.py b/engine/apps/schedules/tests/test_on_call_schedule.py index aa478fe2..603c69ae 100644 --- a/engine/apps/schedules/tests/test_on_call_schedule.py +++ b/engine/apps/schedules/tests/test_on_call_schedule.py @@ -299,13 +299,13 @@ def test_filter_events_ical_all_day(make_organization, make_user_for_organizatio True, ["@Alex"], datetime.datetime(2021, 1, 27, 0, 0, tzinfo=pytz.UTC), - datetime.datetime(2021, 1, 27, 23, 59, 59, 999999, tzinfo=pytz.UTC), + datetime.datetime(2021, 1, 27, 23, 59, 59, tzinfo=pytz.UTC), ), ( True, ["@Alice"], datetime.datetime(2021, 1, 27, 0, 0, tzinfo=pytz.UTC), - datetime.datetime(2021, 1, 28, 23, 59, 59, 999999, tzinfo=pytz.UTC), + datetime.datetime(2021, 1, 28, 23, 59, 59, tzinfo=pytz.UTC), ), ( False, diff --git a/engine/apps/slack/scenarios/schedules.py b/engine/apps/slack/scenarios/schedules.py index 84306683..b470fea2 100644 --- a/engine/apps/slack/scenarios/schedules.py +++ b/engine/apps/slack/scenarios/schedules.py @@ -169,9 +169,10 @@ class EditScheduleShiftNotifyStep(scenario_step.ScenarioStep): now_text = "Inviting . No one on-call now!\n" elif schedule.notify_empty_oncall == schedule.NotifyEmptyOnCall.PREV: user_ids: typing.List[str] = [] - for item in json.loads(schedule.current_shifts).values(): - user_ids.extend(item.get("users", [])) - prev_users = organization.users.filter(id__in=user_ids) + for item in json.loads(schedule.current_shifts): + user_ids_from_shift = [u["pk"] for u in item.get("users", [])] + user_ids.extend(user_ids_from_shift) + prev_users = organization.users.filter(public_primary_key__in=user_ids) users_verbal = " ".join( [f"{user.get_username_with_slack_verbal(mention=True)}" for user in prev_users] ) @@ -183,6 +184,8 @@ class EditScheduleShiftNotifyStep(scenario_step.ScenarioStep): now_text = "" for shift in new_shifts: users = shift["users"] + user_ids_from_shift = [u["pk"] for u in users] + users = organization.users.filter(public_primary_key__in=user_ids_from_shift) now_text += cls.get_ical_shift_notification_text(shift, schedule.mention_oncall_start, users) now_text = "*New on-call shift:*\n" + now_text @@ -195,6 +198,8 @@ class EditScheduleShiftNotifyStep(scenario_step.ScenarioStep): next_text = "" for shift in next_shifts: users = shift["users"] + user_ids_from_shift = [u["pk"] for u in users] + users = organization.users.filter(public_primary_key__in=user_ids_from_shift) next_text += cls.get_ical_shift_notification_text(shift, schedule.mention_oncall_next, users) next_text = "\n*Next on-call shift:*\n" + next_text @@ -242,30 +247,29 @@ class EditScheduleShiftNotifyStep(scenario_step.ScenarioStep): @classmethod def get_ical_shift_notification_text(cls, shift, mention, users) -> str: - if shift["all_day"]: - notification = " ".join([f"{user.get_username_with_slack_verbal(mention=mention)}" for user in users]) - user_verbal = shift["users"][0].get_username_with_slack_verbal( - mention=False, - ) - if shift["start"].day == shift["end"].day: - all_day_text = shift["start"].strftime("%b %d") + notification = "" + for user in users: + if shift["all_day"]: + user_notification = user.get_username_with_slack_verbal(mention=mention) + if shift["start"].day == shift["end"].day: + all_day_text = shift["start"].strftime("%b %d") + else: + all_day_text = f'From {shift["start"].strftime("%b %d")} to {shift["end"].strftime("%b %d")}' + user_notification += f' {all_day_text} _All-day event in timezone "UTC"_\n' else: - all_day_text = f'From {shift["start"].strftime("%b %d")} to {shift["end"].strftime("%b %d")}' - notification += ( - f" {all_day_text} _All-day event in *{user_verbal}'s* timezone_ " f'- {shift["users"][0].timezone}.\n' - ) - else: - shift_start_timestamp = shift["start"].astimezone(pytz.UTC).timestamp() - shift_end_timestamp = shift["end"].astimezone(pytz.UTC).timestamp() + shift_start_timestamp = shift["start"].astimezone(pytz.UTC).timestamp() + shift_end_timestamp = shift["end"].astimezone(pytz.UTC).timestamp() - notification = ( - " ".join([f"{user.get_username_with_slack_verbal(mention=mention)}" for user in users]) - + f" from {format_datetime_to_slack_with_time(shift_start_timestamp)}" - f" to {format_datetime_to_slack_with_time(shift_end_timestamp)}\n" - ) - priority = shift.get("priority", 0) - shift.get("priority_increased_by", 0) - if priority != 0: - notification = f"[L{shift.get('priority')}] {notification}" + user_notification = ( + user.get_username_with_slack_verbal(mention=mention) + + f" from {format_datetime_to_slack_with_time(shift_start_timestamp)}" + f" to {format_datetime_to_slack_with_time(shift_end_timestamp)}\n" + ) + if not shift["is_override"]: + priority = shift.get("priority_level", 0) or 0 + if priority != 0: + user_notification = f"[L{priority}] {user_notification}" + notification += user_notification return notification