Skip to content

Commit

Permalink
Update schedule related users to use cached final representation (#5101)
Browse files Browse the repository at this point in the history
Related to #4936

Cached final schedule keeps a (now - 15d, now + 6m) window
representation of a schedule (this representation always use users'
username; it will un-relate users that are not anymore associated to a
schedule).
  • Loading branch information
matiasb authored Oct 1, 2024
1 parent 6d3b836 commit 8754f60
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 21 deletions.
1 change: 1 addition & 0 deletions engine/apps/alerts/tests/test_paging.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ def test_user_is_oncall(make_organization, make_user_for_organization, make_sche
)
on_call_shift.add_rolling_users([[oncall_user]])
schedule.refresh_ical_file()
schedule.refresh_ical_final_schedule()

assert user_is_oncall(not_oncall_user) is False
assert user_is_oncall(oncall_user) is True
Expand Down
4 changes: 4 additions & 0 deletions engine/apps/api/tests/test_schedules.py
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,7 @@ def test_get_list_schedules_by_mine(
)
override.add_rolling_users([[user]])
web_schedule.refresh_ical_file()
web_schedule.refresh_ical_final_schedule()

url = reverse("api-internal:schedule-list") + query_param
response = client.get(url, format="json", **make_user_auth_headers(user, token))
Expand Down Expand Up @@ -1497,6 +1498,7 @@ def test_next_shifts_per_user(
override.add_rolling_users([[user_c]])

# final schedule: 7-12: B, 15-16: A, 16-17: B, 17-18: C (override), 18-20: C
schedule.refresh_ical_final_schedule()

url = reverse("api-internal:schedule-next-shifts-per-user", kwargs={"pk": schedule.public_primary_key})
response = client.get(url, format="json", **make_user_auth_headers(admin, token))
Expand Down Expand Up @@ -1569,6 +1571,7 @@ def test_next_shifts_per_user_ical_schedule_using_emails(
schedule_class=OnCallScheduleICal,
cached_ical_file_primary=cached_ical_primary_schedule,
)
schedule.refresh_ical_final_schedule()

url = reverse("api-internal:schedule-next-shifts-per-user", kwargs={"pk": schedule.public_primary_key})
response = client.get(url, format="json", **make_user_auth_headers(admin, token))
Expand Down Expand Up @@ -1628,6 +1631,7 @@ def test_related_users(
)
override.add_rolling_users([[user_c]])
schedule.refresh_ical_file()
schedule.refresh_ical_final_schedule()

url = reverse("api-internal:schedule-related-users", kwargs={"pk": schedule.public_primary_key})
response = client.get(url, format="json", **make_user_auth_headers(admin, token))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ def test_notify_shift_swap_request_success(
on_call_shift.add_rolling_users([[user]])

schedule.refresh_ical_file()
schedule.refresh_ical_final_schedule()
schedule.refresh_from_db()

swap_start = now + timezone.timedelta(days=100)
Expand Down
22 changes: 3 additions & 19 deletions engine/apps/schedules/models/on_call_schedule.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from django.conf import settings
from django.core.validators import MinLengthValidator
from django.db import models
from django.db.models import Q
from django.db.utils import DatabaseError
from django.utils import timezone
from django.utils.functional import cached_property
Expand Down Expand Up @@ -158,10 +157,7 @@ def get_oncall_users(self, events_datetime=None):
def related_to_user(self, user):
username_regex = RE_ICAL_SEARCH_USERNAME.format(user.username)
return self.filter(
Q(cached_ical_file_primary__regex=username_regex)
| Q(cached_ical_file_primary__contains=user.email)
| Q(cached_ical_file_overrides__regex=username_regex)
| Q(cached_ical_file_overrides__contains=user.email),
cached_ical_final_schedule__regex=username_regex,
organization=user.organization,
)

Expand Down Expand Up @@ -344,10 +340,8 @@ def _drop_overrides_ical_file(self):
def related_users(self):
"""Return users referenced in the schedule."""
usernames = []
if self.cached_ical_file_primary:
usernames += RE_ICAL_FETCH_USERNAME.findall(self.cached_ical_file_primary)
if self.cached_ical_file_overrides:
usernames += RE_ICAL_FETCH_USERNAME.findall(self.cached_ical_file_overrides)
if self.cached_ical_final_schedule:
usernames += RE_ICAL_FETCH_USERNAME.findall(self.cached_ical_final_schedule)
return self.organization.users.filter(username__in=usernames)

def filter_events(
Expand Down Expand Up @@ -1128,16 +1122,6 @@ def _refresh_overrides_ical_file(self):
)
self.save(update_fields=["cached_ical_file_overrides", "prev_ical_file_overrides", "ical_file_error_overrides"])

def related_users(self):
"""Return users referenced in the schedule."""
# combine users based on usernames and users via email (allowed in iCal based schedules)
usernames = []
if self.cached_ical_file_primary:
usernames += RE_ICAL_FETCH_USERNAME.findall(self.cached_ical_file_primary)
if self.cached_ical_file_overrides:
usernames += RE_ICAL_FETCH_USERNAME.findall(self.cached_ical_file_overrides)
return self.organization.users.filter(Q(username__in=usernames) | Q(email__in=usernames))

# Insight logs
@property
def insight_logs_serialized(self):
Expand Down
41 changes: 39 additions & 2 deletions engine/apps/schedules/tests/test_on_call_schedule.py
Original file line number Diff line number Diff line change
Expand Up @@ -1206,7 +1206,7 @@ def test_schedule_related_users(make_organization, make_user_for_organization, m
now = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0)
start_date = now - timezone.timedelta(days=7)

user_a, _, _, user_d, user_e = (make_user_for_organization(organization, username=i) for i in "ABCDE")
user_a, user_b, _, user_d, user_e = (make_user_for_organization(organization, username=i) for i in "ABCDE")

shifts = (
# user, priority, start time (h), duration (hs)
Expand Down Expand Up @@ -1239,7 +1239,20 @@ def test_schedule_related_users(make_organization, make_user_for_organization, m
)
override.add_rolling_users([[user_e]])

# override: 22-23, a month ago / B (won't be considered a related user)
override_data = {
"start": start_date - timezone.timedelta(hours=22, days=30),
"rotation_start": start_date - timezone.timedelta(hours=22, days=30),
"duration": timezone.timedelta(hours=1),
"schedule": schedule,
}
override = make_on_call_shift(
organization=organization, shift_type=CustomOnCallShift.TYPE_OVERRIDE, **override_data
)
override.add_rolling_users([[user_b]])

schedule.refresh_ical_file()
schedule.refresh_ical_final_schedule()
schedule.refresh_from_db()

users = schedule.related_users()
Expand Down Expand Up @@ -1282,6 +1295,7 @@ def test_schedule_related_users_usernames(
on_call_shift.add_rolling_users([[user]])

schedule.refresh_ical_file()
schedule.refresh_ical_final_schedule()
schedule.refresh_from_db()

assert set(schedule.related_users()) == set(users)
Expand All @@ -1304,7 +1318,7 @@ def test_schedule_related_users_emails(make_organization, make_user_for_organiza
DTSTAMP:20230127T151619Z
UID:something
SUMMARY:testing@testing.com
RRULE:FREQ=WEEKLY;UNTIL=20221231T010101
RRULE:FREQ=WEEKLY
DTSTART;TZID=Europe/Madrid:20220309T130000
DTEND;TZID=Europe/Madrid:20220309T133000
SEQUENCE:4
Expand All @@ -1317,6 +1331,8 @@ def test_schedule_related_users_emails(make_organization, make_user_for_organiza
schedule_class=OnCallScheduleICal,
cached_ical_file_primary=cached_ical_primary_schedule,
)
schedule.refresh_ical_final_schedule()
schedule.refresh_from_db()

assert set(schedule.related_users()) == {user}

Expand Down Expand Up @@ -1604,6 +1620,7 @@ def test_user_related_schedules(
)
on_call_shift.add_rolling_users([[user]])
schedule1.refresh_ical_file()
schedule1.refresh_ical_final_schedule()

schedule2 = make_schedule(organization, schedule_class=OnCallScheduleWeb)
override_data = {
Expand All @@ -1617,10 +1634,27 @@ def test_user_related_schedules(
)
override.add_rolling_users([[admin]])
schedule2.refresh_ical_file()
schedule2.refresh_ical_final_schedule()

# schedule3
make_schedule(organization, schedule_class=OnCallScheduleWeb)

# schedule4
schedule4 = make_schedule(organization, schedule_class=OnCallScheduleWeb)
# user was part of the schedule some time ago (outside of the final schedule window)
override_data = {
"start": today - timezone.timedelta(days=21),
"rotation_start": today - timezone.timedelta(days=21),
"duration": timezone.timedelta(hours=1),
"schedule": schedule4,
}
override = make_on_call_shift(
organization=organization, shift_type=CustomOnCallShift.TYPE_OVERRIDE, **override_data
)
override.add_rolling_users([[admin]])
schedule4.refresh_ical_file()
schedule4.refresh_ical_final_schedule()

schedules = OnCallSchedule.objects.related_to_user(admin)
assert set(schedules) == {schedule1, schedule2}

Expand Down Expand Up @@ -1658,6 +1692,7 @@ def test_user_related_schedules_only_username(
)
on_call_shift.add_rolling_users([[user]])
schedule1.refresh_ical_file()
schedule1.refresh_ical_final_schedule()

schedule2 = make_schedule(organization, schedule_class=OnCallScheduleWeb)
override_data = {
Expand All @@ -1671,6 +1706,7 @@ def test_user_related_schedules_only_username(
)
override.add_rolling_users([[user]])
schedule2.refresh_ical_file()
schedule2.refresh_ical_final_schedule()

# schedule3
schedule3 = make_schedule(organization, schedule_class=OnCallScheduleWeb)
Expand All @@ -1685,6 +1721,7 @@ def test_user_related_schedules_only_username(
)
override.add_rolling_users([[other_user]])
schedule3.refresh_ical_file()
schedule3.refresh_ical_final_schedule()

schedules = OnCallSchedule.objects.related_to_user(user)
assert set(schedules) == {schedule1, schedule2}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ def test_add_user_no_warning(manage_responders_setup, make_schedule, make_on_cal
)
on_call_shift.add_rolling_users([[user]])
schedule.refresh_ical_file()
schedule.refresh_ical_final_schedule()
# setup notification policy
make_user_notification_policy(
user=user,
Expand Down Expand Up @@ -199,6 +200,7 @@ def test_get_users_select(make_organization, make_user, make_schedule, make_on_c
)
on_call_shift.add_rolling_users([[oncall_user]])
schedule.refresh_ical_file()
schedule.refresh_ical_final_schedule()

select_input = _get_users_select(
organization=organization, input_id_prefix="test", action_id="test", max_options_per_group=2
Expand Down
2 changes: 2 additions & 0 deletions engine/apps/slack/tests/test_scenario_steps/test_paging.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ def test_add_user_no_warning(make_organization_and_user_with_slack_identities, m
)
on_call_shift.add_rolling_users([[user]])
schedule.refresh_ical_file()
schedule.refresh_ical_final_schedule()

payload = make_paging_view_slack_payload(selected_org=organization, user=user)

Expand Down Expand Up @@ -221,6 +222,7 @@ def test_add_user_maximum_exceeded(make_organization_and_user_with_slack_identit
)
on_call_shift.add_rolling_users([[user]])
schedule.refresh_ical_file()
schedule.refresh_ical_final_schedule()

payload = make_paging_view_slack_payload(selected_org=organization, user=user)

Expand Down

0 comments on commit 8754f60

Please sign in to comment.