centralcloud/oncall-engine
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

chore: update schedule checks notification period and improve wording (#5412)

Browse source 
Related to https://github.com/grafana/oncall-private/issues/2994

- Extend gaps/empty shift checks to consider 30 days (customizable via
param, eventually make it customizable per schedule?); ie. every week
(per beat schedule), check the schedule next 30 days
- Trigger checks via async task on schedule API updates (instead of a
sync call)
- Update notifications wording / link to schedule
This commit is contained in:
Matias Bordese 2025-01-16 09:19:16 -03:00 • committed by GitHub
parent dcb37417ef
commit 4c92826c26
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 58 additions and 42 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
engine/apps/api/serializers/schedule_base.py
View file

@ -2,8 +2,13 @@ from rest_framework import serializers
from apps.api.serializers.slack_channel import SlackChannelSerializer
from apps.api.serializers.user_group import UserGroupSerializer
from apps.schedules.constants import SCHEDULE_CHECK_NEXT_DAYS
from apps.schedules.models import OnCallSchedule
from apps.schedules.tasks import schedule_notify_about_empty_shifts_in_schedule, schedule_notify_about_gaps_in_schedule
from apps.schedules.tasks import (
check_gaps_and_empty_shifts_in_schedule,
schedule_notify_about_empty_shifts_in_schedule,
schedule_notify_about_gaps_in_schedule,
)
from common.api_helpers.custom_fields import TeamPrimaryKeyRelatedField
from common.api_helpers.mixins import EagerLoadingMixin
from common.api_helpers.utils import CurrentOrganizationDefault
@ -44,8 +49,8 @@ class ScheduleBaseSerializer(EagerLoadingMixin, serializers.ModelSerializer):
"Cannot update the user group, make sure to grant user group modification rights to "
"non-admin users in Slack workspace settings"
)
SCHEDULE_HAS_GAPS_WARNING = "Schedule has unassigned time periods during next 7 days"
SCHEDULE_HAS_EMPTY_SHIFTS_WARNING = "Schedule has empty shifts during next 7 days"
SCHEDULE_HAS_GAPS_WARNING = f"Schedule has unassigned time periods during next {SCHEDULE_CHECK_NEXT_DAYS} days"
SCHEDULE_HAS_EMPTY_SHIFTS_WARNING = f"Schedule has empty shifts during next {SCHEDULE_CHECK_NEXT_DAYS} days"
def get_warnings(self, obj):
can_update_user_groups = self.context.get("can_update_user_groups", False)
@ -81,7 +86,7 @@ class ScheduleBaseSerializer(EagerLoadingMixin, serializers.ModelSerializer):
def create(self, validated_data):
created_schedule = super().create(validated_data)
created_schedule.check_gaps_and_empty_shifts_for_next_week()
check_gaps_and_empty_shifts_in_schedule.apply_async((created_schedule.pk,))
schedule_notify_about_empty_shifts_in_schedule.apply_async((created_schedule.pk,))
schedule_notify_about_gaps_in_schedule.apply_async((created_schedule.pk,))
return created_schedule

8
engine/apps/api/serializers/schedule_calendar.py
View file

@ -2,7 +2,11 @@ from rest_framework import serializers
from apps.api.serializers.schedule_base import ScheduleBaseSerializer
from apps.schedules.models import OnCallScheduleCalendar
from apps.schedules.tasks import schedule_notify_about_empty_shifts_in_schedule, schedule_notify_about_gaps_in_schedule
from apps.schedules.tasks import (
check_gaps_and_empty_shifts_in_schedule,
schedule_notify_about_empty_shifts_in_schedule,
schedule_notify_about_gaps_in_schedule,
)
from apps.slack.models import SlackChannel, SlackUserGroup
from common.api_helpers.custom_fields import OrganizationFilteredPrimaryKeyRelatedField, TimeZoneField
from common.api_helpers.utils import validate_ical_url
@ -58,7 +62,7 @@ class ScheduleCalendarCreateSerializer(ScheduleCalendarSerializer):
or old_enable_web_overrides != updated_enable_web_overrides
):
updated_schedule.drop_cached_ical()
updated_schedule.check_gaps_and_empty_shifts_for_next_week()
check_gaps_and_empty_shifts_in_schedule.apply_async((instance.pk,))
schedule_notify_about_empty_shifts_in_schedule.apply_async((instance.pk,))
schedule_notify_about_gaps_in_schedule.apply_async((instance.pk,))
return updated_schedule

3
engine/apps/api/serializers/schedule_ical.py
View file

@ -1,6 +1,7 @@
from apps.api.serializers.schedule_base import ScheduleBaseSerializer
from apps.schedules.models import OnCallScheduleICal
from apps.schedules.tasks import (
check_gaps_and_empty_shifts_in_schedule,
refresh_ical_final_schedule,
schedule_notify_about_empty_shifts_in_schedule,
schedule_notify_about_gaps_in_schedule,
@ -87,7 +88,7 @@ class ScheduleICalUpdateSerializer(ScheduleICalCreateSerializer):
if old_ical_url_primary != updated_ical_url_primary or old_ical_url_overrides != updated_ical_url_overrides:
updated_schedule.drop_cached_ical()
updated_schedule.check_gaps_and_empty_shifts_for_next_week()
check_gaps_and_empty_shifts_in_schedule.apply_async((instance.pk,))
schedule_notify_about_empty_shifts_in_schedule.apply_async((instance.pk,))
schedule_notify_about_gaps_in_schedule.apply_async((instance.pk,))
# for iCal-based schedules we need to refresh final schedule information

8
engine/apps/api/serializers/schedule_web.py
View file

@ -1,6 +1,10 @@
from apps.api.serializers.schedule_base import ScheduleBaseSerializer
from apps.schedules.models import OnCallScheduleWeb
from apps.schedules.tasks import schedule_notify_about_empty_shifts_in_schedule, schedule_notify_about_gaps_in_schedule
from apps.schedules.tasks import (
check_gaps_and_empty_shifts_in_schedule,
schedule_notify_about_empty_shifts_in_schedule,
schedule_notify_about_gaps_in_schedule,
)
from apps.slack.models import SlackChannel, SlackUserGroup
from common.api_helpers.custom_fields import OrganizationFilteredPrimaryKeyRelatedField, TimeZoneField
@ -41,7 +45,7 @@ class ScheduleWebCreateSerializer(ScheduleWebSerializer):
updated_time_zone = updated_schedule.time_zone
if old_time_zone != updated_time_zone:
updated_schedule.drop_cached_ical()
updated_schedule.check_gaps_and_empty_shifts_for_next_week()
check_gaps_and_empty_shifts_in_schedule.apply_async((instance.pk,))
schedule_notify_about_empty_shifts_in_schedule.apply_async((instance.pk,))
schedule_notify_about_gaps_in_schedule.apply_async((instance.pk,))
return updated_schedule

2
engine/apps/api/views/schedule.py
View file

@ -481,7 +481,7 @@ class ScheduleView(
def reload_ical(self, request, pk):
schedule = self.get_object(annotate=False)
schedule.drop_cached_ical()
schedule.check_gaps_and_empty_shifts_for_next_week()
schedule.check_gaps_and_empty_shifts_for_next_days()
if schedule.user_group is not None:
update_slack_user_group_for_schedules.apply_async((schedule.user_group.pk,))

1
engine/apps/schedules/constants.py
View file

@ -29,5 +29,6 @@ EXPORT_WINDOW_DAYS_BEFORE = 15
SCHEDULE_ONCALL_CACHE_KEY_PREFIX = "schedule_oncall_users_"
SCHEDULE_ONCALL_CACHE_TTL = 15 * 60 # 15 minutes in seconds
SCHEDULE_CHECK_NEXT_DAYS = 30
PREFETCHED_SHIFT_SWAPS = "prefetched_shift_swaps"

13
engine/apps/schedules/models/on_call_schedule.py
View file

@ -33,6 +33,7 @@ from apps.schedules.constants import (
ICAL_SUMMARY,
ICAL_UID,
PREFETCHED_SHIFT_SWAPS,
SCHEDULE_CHECK_NEXT_DAYS,
)
from apps.schedules.ical_utils import (
EmptyShifts,
@ -293,9 +294,9 @@ class OnCallSchedule(PolymorphicModel):
(self.prev_ical_file_overrides, self.cached_ical_file_overrides),
]
def check_gaps_and_empty_shifts_for_next_week(self) -> None:
def check_gaps_and_empty_shifts_for_next_days(self, days=SCHEDULE_CHECK_NEXT_DAYS) -> None:
datetime_start = timezone.now()
datetime_end = datetime_start + datetime.timedelta(days=7)
datetime_end = datetime_start + datetime.timedelta(days=days)
# get empty shifts from all events and gaps from final events
events = self.filter_events(
@ -313,14 +314,14 @@ class OnCallSchedule(PolymorphicModel):
self.has_empty_shifts = has_empty_shifts
self.save(update_fields=["has_gaps", "has_empty_shifts"])
def get_gaps_for_next_week(self) -> ScheduleEvents:
def get_gaps_for_next_days(self, days=SCHEDULE_CHECK_NEXT_DAYS) -> ScheduleEvents:
today = timezone.now()
events = self.final_events(today, today + datetime.timedelta(days=7))
events = self.final_events(today, today + datetime.timedelta(days=days))
return [event for event in events if event["is_gap"]]
def get_empty_shifts_for_next_week(self) -> EmptyShifts:
def get_empty_shifts_for_next_days(self, days=SCHEDULE_CHECK_NEXT_DAYS) -> EmptyShifts:
today = timezone.now().date()
return list_of_empty_shifts_in_schedule(self, today, today + datetime.timedelta(days=7))
return list_of_empty_shifts_in_schedule(self, today, today + datetime.timedelta(days=days))
def drop_cached_ical(self):
self._drop_primary_ical_file()

2
engine/apps/schedules/tasks/check_gaps_and_empty_shifts.py
View file

@ -19,5 +19,5 @@ def check_gaps_and_empty_shifts_in_schedule(schedule_pk):
task_logger.info(f"Tried to check_gaps_and_empty_shifts_in_schedule for non-existing schedule {schedule_pk}")
return
schedule.check_gaps_and_empty_shifts_for_next_week()
schedule.check_gaps_and_empty_shifts_for_next_days()
task_logger.info(f"Finish check_gaps_and_empty_shifts_in_schedule {schedule_pk}")

9
engine/apps/schedules/tasks/notify_about_empty_shifts_in_schedule.py
View file

@ -48,15 +48,15 @@ def notify_about_empty_shifts_in_schedule_task(schedule_pk):
task_logger.info(f"Tried to notify_about_empty_shifts_in_schedule_task for non-existing schedule {schedule_pk}")
return
empty_shifts = schedule.get_empty_shifts_for_next_week()
empty_shifts = schedule.get_empty_shifts_for_next_days()
schedule.empty_shifts_report_sent_at = timezone.now().date()
if len(empty_shifts) != 0:
schedule.has_empty_shifts = True
text = (
f'Tried to parse schedule *"{schedule.name}"* and found events without associated users.\n'
f"To ensure you don't miss any notifications, use a Grafana username as the event name in the calendar. "
f"The user should have Editor or Admin access.\n\n"
f"Reviewing *{schedule.slack_url}* on-call schedule found events without valid associated users.\n"
f"To ensure you don't miss any notifications, make sure user exists (or you provided a valid Grafana username). "
f"The user should have the right permissions, or be an Editor or Admin.\n\n"
)
for idx, empty_shift in enumerate(empty_shifts):
start_timestamp = empty_shift.start.astimezone(pytz.UTC).timestamp()
@ -80,7 +80,6 @@ def notify_about_empty_shifts_in_schedule_task(schedule_pk):
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"
if idx != len(empty_shifts) - 1:
text += "\n\n"
post_message_to_channel(schedule.organization, schedule.slack_channel_slack_id, text)

9
engine/apps/schedules/tasks/notify_about_gaps_in_schedule.py
View file

@ -48,13 +48,13 @@ def notify_about_gaps_in_schedule_task(schedule_pk):
task_logger.info(f"Tried to notify_about_gaps_in_schedule_task for non-existing schedule {schedule_pk}")
return
gaps = schedule.get_gaps_for_next_week()
gaps = schedule.get_gaps_for_next_days()
schedule.gaps_report_sent_at = timezone.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):
text = f"There are time periods that are unassigned in *{schedule.slack_url}* on-call schedule.\n"
for gap in gaps:
if gap["start"]:
start_verbal = format_datetime_to_slack_with_time(gap["start"].astimezone(pytz.UTC).timestamp())
else:
@ -64,8 +64,7 @@ def notify_about_gaps_in_schedule_task(schedule_pk):
else:
end_verbal = "..."
text += f"From {start_verbal} to {end_verbal} (your TZ)\n"
if idx != len(gaps) - 1:
text += "\n\n"
text += "\n\n"
post_message_to_channel(schedule.organization, schedule.slack_channel_slack_id, text)
else:
schedule.has_gaps = False

25
engine/apps/schedules/tests/test_check_gaps_and_empty_shifts.py
View file

@ -4,6 +4,7 @@ import pytest
from django.utils import timezone
from apps.api.permissions import LegacyAccessControlRole
from apps.schedules.constants import SCHEDULE_CHECK_NEXT_DAYS
from apps.schedules.models import CustomOnCallShift, OnCallScheduleWeb
@ -34,7 +35,7 @@ def test_no_empty_shifts_no_gaps(
)
on_call_shift.add_rolling_users([[user1]])
schedule.refresh_ical_file()
schedule.check_gaps_and_empty_shifts_for_next_week()
schedule.check_gaps_and_empty_shifts_for_next_days()
schedule.refresh_from_db()
assert schedule.has_gaps is False
@ -73,7 +74,7 @@ def test_no_empty_shifts_but_gaps_now(
assert schedule.has_gaps is False
assert schedule.has_empty_shifts is False
schedule.check_gaps_and_empty_shifts_for_next_week()
schedule.check_gaps_and_empty_shifts_for_next_days()
schedule.refresh_from_db()
assert schedule.has_gaps is True
@ -111,7 +112,7 @@ def test_empty_shifts_no_gaps(
assert schedule.has_gaps is False
assert schedule.has_empty_shifts is False
schedule.check_gaps_and_empty_shifts_for_next_week()
schedule.check_gaps_and_empty_shifts_for_next_days()
schedule.refresh_from_db()
assert schedule.has_gaps is False
@ -150,7 +151,7 @@ def test_empty_shifts_and_gaps(
assert schedule.has_gaps is False
assert schedule.has_empty_shifts is False
schedule.check_gaps_and_empty_shifts_for_next_week()
schedule.check_gaps_and_empty_shifts_for_next_days()
schedule.refresh_from_db()
assert schedule.has_gaps is True
@ -206,7 +207,7 @@ def test_empty_shifts_and_gaps_in_the_past(
assert schedule.has_gaps is False
assert schedule.has_empty_shifts is False
schedule.check_gaps_and_empty_shifts_for_next_week()
schedule.check_gaps_and_empty_shifts_for_next_days()
schedule.refresh_from_db()
assert schedule.has_gaps is False
@ -225,9 +226,9 @@ def test_empty_shifts_and_gaps_in_the_future(
user2 = make_user(organization=organization, username="user2", role=LegacyAccessControlRole.ADMIN)
schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb, name="test_schedule")
# empty shift with gaps starts in 7 days 1 min
# empty shift with gaps starts in SCHEDULE_CHECK_NEXT_DAYS days 1 min
now = timezone.now().replace(microsecond=0)
start_date = now + datetime.timedelta(days=7, minutes=1)
start_date = now + datetime.timedelta(days=SCHEDULE_CHECK_NEXT_DAYS, minutes=1)
data = {
"start": start_date,
"rotation_start": start_date,
@ -241,9 +242,9 @@ def test_empty_shifts_and_gaps_in_the_future(
organization=organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, **data
)
on_call_shift.add_rolling_users([[user1]])
# normal shift ends in 7 days 1 min
start_date2 = now - datetime.timedelta(days=7, minutes=1)
until = now + datetime.timedelta(days=7, minutes=1)
# normal shift ends in SCHEDULE_CHECK_NEXT_DAYS days 1 min
start_date2 = now - datetime.timedelta(days=SCHEDULE_CHECK_NEXT_DAYS, minutes=1)
until = now + datetime.timedelta(days=SCHEDULE_CHECK_NEXT_DAYS, minutes=1)
data2 = {
"start": start_date2,
"rotation_start": start_date2,
@ -262,8 +263,8 @@ def test_empty_shifts_and_gaps_in_the_future(
assert schedule.has_gaps is False
assert schedule.has_empty_shifts is False
schedule.check_gaps_and_empty_shifts_for_next_week()
schedule.check_gaps_and_empty_shifts_for_next_days()
schedule.refresh_from_db()
# no gaps and empty shifts in the next 7 days
# no gaps and empty shifts in the next SCHEDULE_CHECK_NEXT_DAYS days
assert schedule.has_gaps is False
assert schedule.has_empty_shifts is False

7
engine/apps/schedules/tests/test_notify_about_gaps_in_schedule.py
View file

@ -4,6 +4,7 @@ from unittest.mock import patch
import pytest
from django.utils import timezone
from apps.schedules.constants import SCHEDULE_CHECK_NEXT_DAYS
from apps.schedules.models import CustomOnCallShift, OnCallScheduleCalendar, OnCallScheduleICal, OnCallScheduleWeb
from apps.schedules.tasks import notify_about_gaps_in_schedule_task, start_notify_about_gaps_in_schedule
@ -236,7 +237,7 @@ def test_gaps_near_future_trigger_notification(
@pytest.mark.django_db
def test_gaps_later_than_7_days_no_triggering_notification(
def test_gaps_later_than_days_no_triggering_notification(
make_slack_team_identity,
make_slack_channel,
make_organization,
@ -259,8 +260,8 @@ def test_gaps_later_than_7_days_no_triggering_notification(
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)
start_date = now - datetime.timedelta(days=SCHEDULE_CHECK_NEXT_DAYS, minutes=1)
until_date = now + datetime.timedelta(days=SCHEDULE_CHECK_NEXT_DAYS + 1)
data = {
"start": start_date,
"rotation_start": start_date,