Skip to content

Commit

Permalink
Truncate exported final shifts to match the requested period (#2924)
Browse files Browse the repository at this point in the history
When calculating on-call load, if an involved shift extends outside the
requested time span you can get unexpected results. Truncate the
returned events times to keep the details for the specified start/end
dates.
  • Loading branch information
matiasb authored Aug 31, 2023
1 parent 1ac9a2c commit 6b8c1ac
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Performance and UX tweaks to integrations page ([#2869](https://github.com/grafana/oncall/pull/2869))
- Expand users details in filter swaps internal endpoint ([#2921](https://github.com/grafana/oncall/pull/2921))
- Truncate exported final shifts to match the requested period ([#2924](https://github.com/grafana/oncall/pull/2924))

### Fixed

Expand Down
44 changes: 42 additions & 2 deletions engine/apps/public_api/tests/test_schedules.py
Original file line number Diff line number Diff line change
Expand Up @@ -969,7 +969,7 @@ def test_oncall_shifts_export_from_ical_schedule(
client = APIClient()

url = reverse("api-public:schedules-final-shifts", kwargs={"pk": schedule.public_primary_key})
response = client.get(f"{url}?start_date=2023-07-01&end_date=2023-08-01", format="json", HTTP_AUTHORIZATION=token)
response = client.get(f"{url}?start_date=2023-07-01&end_date=2023-07-31", format="json", HTTP_AUTHORIZATION=token)
assert response.status_code == status.HTTP_200_OK

expected_on_call_times = {
Expand Down Expand Up @@ -1006,11 +1006,51 @@ def test_oncall_shifts_export_from_api_schedule(
client = APIClient()

url = reverse("api-public:schedules-final-shifts", kwargs={"pk": schedule.public_primary_key})
response = client.get(f"{url}?start_date=2023-07-01&end_date=2023-08-01", format="json", HTTP_AUTHORIZATION=token)
response = client.get(f"{url}?start_date=2023-07-01&end_date=2023-07-31", format="json", HTTP_AUTHORIZATION=token)
assert response.status_code == status.HTTP_200_OK

expected_on_call_times = {
user1.public_primary_key: 32, # daily 2h * 16d
user2.public_primary_key: 30, # daily 2h * 15d
}
assert_expected_shifts_export_response(response, (user1, user2), expected_on_call_times)


@pytest.mark.django_db
def test_oncall_shifts_export_truncate_events(
make_organization_and_user_with_token,
make_user,
make_schedule,
make_on_call_shift,
):
organization, _, token = make_organization_and_user_with_token()
user1 = make_user(organization=organization)

user1_public_primary_key = user1.public_primary_key
schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb)

# 24h shifts starting 9am on Mo, We and Fr
start_date = timezone.datetime(2023, 1, 1, 9, 0, 0, tzinfo=pytz.UTC)
make_on_call_shift(
organization=organization,
schedule=schedule,
shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT,
frequency=CustomOnCallShift.FREQUENCY_DAILY,
priority_level=1,
interval=1,
by_day=["MO", "WE", "FR"],
start=start_date,
rolling_users=[{user1.pk: user1_public_primary_key}],
rotation_start=start_date,
duration=timezone.timedelta(hours=24),
)

client = APIClient()

# request shifts on a Tu (ie. 00:00 - 09:00)
url = reverse("api-public:schedules-final-shifts", kwargs={"pk": schedule.public_primary_key})
response = client.get(f"{url}?start_date=2023-01-03&end_date=2023-01-03", format="json", HTTP_AUTHORIZATION=token)
assert response.status_code == status.HTTP_200_OK

expected_on_call_times = {user1_public_primary_key: 9}
assert_expected_shifts_export_response(response, (user1,), expected_on_call_times)
7 changes: 4 additions & 3 deletions engine/apps/public_api/views/schedules.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ def final_shifts(self, request, pk):

datetime_start = datetime.datetime.combine(start_date, datetime.time.min, tzinfo=pytz.UTC)
datetime_end = datetime_start + datetime.timedelta(
days=days_between_start_and_end - 1, hours=23, minutes=59, seconds=59
days=days_between_start_and_end, hours=23, minutes=59, seconds=59
)

final_schedule_events: ScheduleEvents = schedule.final_events(datetime_start, datetime_end)
Expand All @@ -158,8 +158,9 @@ def final_shifts(self, request, pk):
"user_pk": user["pk"],
"user_email": user["email"],
"user_username": user["display_name"],
"shift_start": event["start"],
"shift_end": event["end"],
# truncate shift start/end exceeding the requested period
"shift_start": event["start"] if event["start"] >= datetime_start else datetime_start,
"shift_end": event["end"] if event["end"] <= datetime_end else datetime_end,
}
for event in final_schedule_events
for user in event["users"]
Expand Down

0 comments on commit 6b8c1ac

Please sign in to comment.