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

Update shift API to use a default for interval when missing (#1810)

Browse source 
Fixes #1527.
This commit is contained in:
Matias Bordese 2023-04-21 16:13:09 -03:00 • committed by GitHub
parent 62d5fc3bf7
commit e404d2f4b6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 140 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
CHANGELOG.md
View file

@ -16,6 +16,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Add helm chart support for twilio existing secrets by @atownsend247 ([#1435](https://github.com/grafana/oncall/pull/1435))
### Changed
- Update shift API to use a default interval value (`1`) when a `frequency` is set and no `interval` is given
## v1.2.14 (2023-04-19)
### Fixed

2
docs/sources/oncall-api-reference/on_call_shifts.md
View file

@ -55,7 +55,7 @@ The above command returns JSON structured in the following way:
| `start` | No | Yes | Start time of the on-call shift. This parameter takes a date format as `yyyy-MM-dd'T'HH:mm:ss` (for example "2020-09-05T08:00:00"). |
| `duration` | No | Yes | Duration of the event. |
| `frequency` | No | If type = `recurrent_event` or `rolling_users` | One of: `hourly`, `daily`, `weekly`, `monthly`. |
| `interval` | No | Optional | This parameter takes a positive integer that represents the intervals that the recurrence rule repeats. |
| `interval` | No | Optional | This parameter takes a positive integer that represents the intervals that the recurrence rule repeats. If `frequency` is set, the default assumed value for this will be `1`. |
| `until` | No | Optional | When the recurrence rule ends (endless if None). This parameter takes a date format as `yyyy-MM-dd'T'HH:mm:ss` (for example "2020-09-05T08:00:00"). |
| `week_start` | No | Optional | Start day of the week in iCal format. One of: `SU` (Sunday), `MO` (Monday), `TU` (Tuesday), `WE` (Wednesday), `TH` (Thursday), `FR` (Friday), `SA` (Saturday). Default: `SU`. |
| `by_day` | No | Optional | List of days in iCal format. Valid values are: `SU`, `MO`, `TU`, `WE`, `TH`, `FR`, `SA`. |

4
engine/apps/api/serializers/on_call_shifts.py
View file

@ -120,6 +120,10 @@ class OnCallShiftSerializer(EagerLoadingMixin, serializers.ModelSerializer):
)
if frequency not in (CustomOnCallShift.FREQUENCY_WEEKLY, CustomOnCallShift.FREQUENCY_DAILY) and by_day:
raise serializers.ValidationError({"by_day": ["Cannot set days value for this frequency type"]})
if interval is None:
raise serializers.ValidationError(
{"interval": ["If frequency is set, interval must be a positive integer"]}
)
if frequency == CustomOnCallShift.FREQUENCY_DAILY and by_day and interval > len(by_day):
raise serializers.ValidationError(
{"interval": ["Interval must be less than or equal to the number of selected days"]}

56
engine/apps/api/tests/test_oncall_shift.py
View file

@ -41,7 +41,7 @@ def test_create_on_call_shift_rotation(on_call_shift_internal_api_setup, make_us
"rotation_start": start_date.strftime("%Y-%m-%dT%H:%M:%SZ"),
"until": None,
"frequency": 1,
"interval": None,
"interval": 1,
"by_day": [
CustomOnCallShift.ICAL_WEEKDAY_MAP[CustomOnCallShift.MONDAY],
CustomOnCallShift.ICAL_WEEKDAY_MAP[CustomOnCallShift.FRIDAY],
@ -451,7 +451,7 @@ def test_update_old_on_call_shift_with_future_version(
"rotation_start": start_date.strftime("%Y-%m-%dT%H:%M:%SZ"),
"until": None,
"frequency": CustomOnCallShift.FREQUENCY_DAILY,
"interval": None,
"interval": 1,
"by_day": None,
"rolling_users": [[user1.public_primary_key]],
}
@ -715,7 +715,7 @@ def test_create_on_call_shift_invalid_data_until(on_call_shift_internal_api_setu
"rotation_start": start_date.strftime("%Y-%m-%dT%H:%M:%SZ"),
"until": (start_date - timezone.timedelta(hours=2)).strftime("%Y-%m-%dT%H:%M:%SZ"),
"frequency": 1,
"interval": None,
"interval": 1,
"by_day": [
CustomOnCallShift.ICAL_WEEKDAY_MAP[CustomOnCallShift.MONDAY],
CustomOnCallShift.ICAL_WEEKDAY_MAP[CustomOnCallShift.FRIDAY],
@ -828,26 +828,32 @@ def test_create_on_call_shift_invalid_data_interval(on_call_shift_internal_api_s
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.data["interval"][0] == "Cannot set interval for non-recurrent shifts"
# by_day, daily, interval > len(by_day)
data = {
"title": "Test Shift 2",
"type": CustomOnCallShift.TYPE_ROLLING_USERS_EVENT,
"schedule": schedule.public_primary_key,
"priority_level": 0,
"shift_start": start_date.strftime("%Y-%m-%dT%H:%M:%SZ"),
"shift_end": (start_date + timezone.timedelta(hours=2)).strftime("%Y-%m-%dT%H:%M:%SZ"),
"rotation_start": start_date.strftime("%Y-%m-%dT%H:%M:%SZ"),
"until": None,
"frequency": CustomOnCallShift.FREQUENCY_DAILY,
"interval": 2,
"by_day": [CustomOnCallShift.ICAL_WEEKDAY_MAP[CustomOnCallShift.MONDAY]],
"rolling_users": [[user1.public_primary_key]],
}
invalid_intervals = (
(None, "If frequency is set, interval must be a positive integer"),
(2, "Interval must be less than or equal to the number of selected days"),
)
for interval, expected_error in invalid_intervals:
# by_day, daily shift
data = {
"title": "Test Shift 2",
"type": CustomOnCallShift.TYPE_ROLLING_USERS_EVENT,
"schedule": schedule.public_primary_key,
"priority_level": 0,
"shift_start": start_date.strftime("%Y-%m-%dT%H:%M:%SZ"),
"shift_end": (start_date + timezone.timedelta(hours=2)).strftime("%Y-%m-%dT%H:%M:%SZ"),
"rotation_start": start_date.strftime("%Y-%m-%dT%H:%M:%SZ"),
"until": None,
"frequency": CustomOnCallShift.FREQUENCY_DAILY,
"by_day": [CustomOnCallShift.ICAL_WEEKDAY_MAP[CustomOnCallShift.MONDAY]],
"rolling_users": [[user1.public_primary_key]],
}
if interval:
data["interval"] = interval
response = client.post(url, data, format="json", **make_user_auth_headers(user1, token))
response = client.post(url, data, format="json", **make_user_auth_headers(user1, token))
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.data["interval"][0] == "Interval must be less than or equal to the number of selected days"
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.data["interval"][0] == expected_error
@pytest.mark.django_db
@ -1275,6 +1281,7 @@ def test_on_call_shift_preview_permissions(
"rolling_users": [[user.public_primary_key]],
"priority_level": 2,
"frequency": CustomOnCallShift.FREQUENCY_DAILY,
"interval": 1,
}
url = reverse("api-internal:oncall_shifts-preview")
@ -1299,6 +1306,7 @@ def test_on_call_shift_preview_missing_data(
"rolling_users": [[user.public_primary_key]],
"priority_level": 2,
"frequency": CustomOnCallShift.FREQUENCY_DAILY,
"interval": 1,
}
url = reverse("api-internal:oncall_shifts-preview")
@ -1336,6 +1344,7 @@ def test_on_call_shift_preview(
"duration": timezone.timedelta(hours=9),
"priority_level": 1,
"frequency": CustomOnCallShift.FREQUENCY_DAILY,
"interval": 1,
"schedule": schedule,
}
on_call_shift = make_on_call_shift(
@ -1357,6 +1366,7 @@ def test_on_call_shift_preview(
"rolling_users": [[other_user.public_primary_key]],
"priority_level": 2,
"frequency": CustomOnCallShift.FREQUENCY_DAILY,
"interval": 1,
}
response = client.post(url, shift_data, format="json", **make_user_auth_headers(user, token))
assert response.status_code == status.HTTP_200_OK
@ -1450,6 +1460,7 @@ def test_on_call_shift_preview_without_users(
"rolling_users": [],
"priority_level": 2,
"frequency": CustomOnCallShift.FREQUENCY_DAILY,
"interval": 1,
}
response = client.post(url, shift_data, format="json", **make_user_auth_headers(user, token))
assert response.status_code == status.HTTP_200_OK
@ -1528,6 +1539,7 @@ def test_on_call_shift_preview_merge_events(
"rolling_users": [[user.public_primary_key, other_user.public_primary_key]],
"priority_level": 2,
"frequency": CustomOnCallShift.FREQUENCY_DAILY,
"interval": 1,
}
response = client.post(url, shift_data, format="json", **make_user_auth_headers(user, token))
assert response.status_code == status.HTTP_200_OK
@ -1637,6 +1649,7 @@ def test_on_call_shift_preview_update(
"rolling_users": [[other_user.public_primary_key]],
"priority_level": 1,
"frequency": CustomOnCallShift.FREQUENCY_DAILY,
"interval": 1,
}
response = client.post(url, shift_data, format="json", **make_user_auth_headers(user, token))
assert response.status_code == status.HTTP_200_OK
@ -1752,6 +1765,7 @@ def test_on_call_shift_preview_update_not_started_reuse_pk(
"rolling_users": [[other_user.public_primary_key]],
"priority_level": 1,
"frequency": CustomOnCallShift.FREQUENCY_DAILY,
"interval": 1,
}
response = client.post(url, shift_data, format="json", **make_user_auth_headers(user, token))
assert response.status_code == status.HTTP_200_OK

19
engine/apps/public_api/serializers/on_call_shifts.py
View file

@ -120,7 +120,10 @@ class CustomOnCallShiftSerializer(EagerLoadingMixin, serializers.ModelSerializer
def create(self, validated_data):
self._validate_frequency_and_week_start(
validated_data["type"], validated_data.get("frequency"), validated_data.get("week_start")
validated_data["type"],
validated_data.get("frequency"),
validated_data.get("interval", 1), # if field is missing, the default value will be used
validated_data.get("week_start"),
)
validated_data = self._correct_validated_data(validated_data["type"], validated_data)
self._validate_start_rotation_from_user_index(
@ -196,12 +199,15 @@ class CustomOnCallShiftSerializer(EagerLoadingMixin, serializers.ModelSerializer
result.append(users_dict)
return result
def _validate_frequency_and_week_start(self, event_type, frequency, week_start):
def _validate_frequency_and_week_start(self, event_type, frequency, interval, week_start):
if event_type not in (CustomOnCallShift.TYPE_SINGLE_EVENT, CustomOnCallShift.TYPE_OVERRIDE):
if frequency is None:
raise BadRequest(detail="Field 'frequency' is required for this on-call shift type")
elif frequency == CustomOnCallShift.FREQUENCY_WEEKLY and week_start is None:
raise BadRequest(detail="Field 'week_start' is required for frequency type 'weekly'")
# frequency is not None
if interval is None:
raise BadRequest(detail="Field 'interval' must be a positive integer")
def _validate_frequency_daily(self, event_type, frequency, interval, by_day, by_monthday):
if event_type == CustomOnCallShift.TYPE_ROLLING_USERS_EVENT:
@ -337,6 +343,9 @@ class CustomOnCallShiftSerializer(EagerLoadingMixin, serializers.ModelSerializer
validated_data[field] = None
if validated_data.get("start") is not None:
validated_data["start"] = validated_data["start"].replace(tzinfo=None)
if validated_data.get("frequency") and validated_data.get("interval") is None:
# if there is frequency but no interval is given, default to 1
validated_data["interval"] = 1
# Populate "rolling_users" field using "users" field for web overrides
# This emulates the behavior of the web UI, which creates overrides populating the rolling_users field
@ -367,10 +376,10 @@ class CustomOnCallShiftUpdateSerializer(CustomOnCallShiftSerializer):
"start_rotation_from_user_index", instance.start_rotation_from_user_index
)
week_start = validated_data.get("week_start")
if frequency != instance.frequency:
self._validate_frequency_and_week_start(event_type, frequency, week_start)
interval = validated_data.get("interval", instance.interval)
if frequency != instance.frequency:
self._validate_frequency_and_week_start(event_type, frequency, interval, week_start)
by_day = validated_data.get("by_day", instance.by_day)
by_monthday = validated_data.get("by_monthday", instance.by_monthday)
self._validate_frequency_daily(event_type, frequency, interval, by_day, by_monthday)

82
engine/apps/public_api/tests/test_on_call_shifts.py
View file

@ -178,6 +178,88 @@ def test_create_on_call_shift(make_organization_and_user_with_token):
assert response.data == result
@pytest.mark.django_db
def test_create_on_call_shift_using_default_interval(make_organization_and_user_with_token):
_, user, token = make_organization_and_user_with_token()
client = APIClient()
url = reverse("api-public:on_call_shifts-list")
start = datetime.datetime.now()
until = start + datetime.timedelta(days=30)
data = {
"team_id": None,
"name": "test name",
"type": "recurrent_event",
"level": 1,
"start": start.strftime("%Y-%m-%dT%H:%M:%S"),
"rotation_start": start.strftime("%Y-%m-%dT%H:%M:%S"),
"duration": 10800,
"users": [user.public_primary_key],
"week_start": "MO",
"frequency": "weekly",
"until": until.strftime("%Y-%m-%dT%H:%M:%S"),
"by_day": ["MO", "WE", "FR"],
}
response = client.post(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}")
on_call_shift = CustomOnCallShift.objects.get(public_primary_key=response.data["id"])
expected = {
"id": on_call_shift.public_primary_key,
"team_id": None,
"name": data["name"],
"type": "recurrent_event",
"time_zone": None,
"level": data["level"],
"start": data["start"],
"rotation_start": data["rotation_start"],
"duration": data["duration"],
"frequency": data["frequency"],
"interval": 1,
"until": data["until"],
"week_start": data["week_start"],
"by_day": data["by_day"],
"users": [user.public_primary_key],
"by_month": None,
"by_monthday": None,
}
assert response.status_code == status.HTTP_201_CREATED
assert response.data == expected
@pytest.mark.django_db
def test_create_on_call_shift_using_none_interval_fails(make_organization_and_user_with_token):
_, user, token = make_organization_and_user_with_token()
client = APIClient()
url = reverse("api-public:on_call_shifts-list")
start = datetime.datetime.now()
until = start + datetime.timedelta(days=30)
data = {
"team_id": None,
"name": "test name",
"type": "recurrent_event",
"level": 1,
"start": start.strftime("%Y-%m-%dT%H:%M:%S"),
"rotation_start": start.strftime("%Y-%m-%dT%H:%M:%S"),
"duration": 10800,
"users": [user.public_primary_key],
"week_start": "MO",
"frequency": "weekly",
"interval": None,
"until": until.strftime("%Y-%m-%dT%H:%M:%S"),
"by_day": ["MO", "WE", "FR"],
}
response = client.post(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}")
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.json() == {"detail": "Field 'interval' must be a positive integer"}
@pytest.mark.django_db
def test_create_override_on_call_shift(make_organization_and_user_with_token):
_, user, token = make_organization_and_user_with_token()