Require users when creating a schedule rotation (#2220)

CHANGELOG.md

@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Removed `SlackActionRecord` model and database table by @joeyorlando [#2201](https://github.com/grafana/oncall/pull/2201)
+- Require users when creating a schedule rotation using the web UI [#2220](https://github.com/grafana/oncall/pull/2220)
 
 ### Fixed
 

engine/apps/api/serializers/on_call_shifts.py

@@ -189,11 +189,18 @@ class OnCallShiftSerializer(EagerLoadingMixin, serializers.ModelSerializer):
 
         return validated_data
 
+    def _require_users(self, validated_data):
+        users = validated_data.get("rolling_users")
+        if not users:
+            raise serializers.ValidationError({"rolling_users": ["User(s) are required"]})
+
     def create(self, validated_data):
         validated_data = self._correct_validated_data(validated_data["type"], validated_data)
         validated_data["name"] = CustomOnCallShift.generate_name(
             validated_data["schedule"], validated_data["priority_level"], validated_data["type"]
         )
+        # before creation, require users set
+        self._require_users(validated_data)
         instance = super().create(validated_data)
 
         instance.start_drop_ical_and_check_schedule_tasks(instance.schedule)
@@ -226,6 +233,9 @@ class OnCallShiftUpdateSerializer(OnCallShiftSerializer):
             elif instance.event_is_finished:
                 raise serializers.ValidationError(["This event cannot be updated"])
 
+        # before update, require users set
+        self._require_users(validated_data)
+
         if not force_update and create_or_update_last_shift:
             result = instance.create_or_update_last_shift(validated_data)
         else:

engine/apps/api/tests/test_oncall_shift.py

@@ -57,6 +57,38 @@ def test_create_on_call_shift_rotation(on_call_shift_internal_api_setup, make_us
     assert response.json() == expected_payload
 
 
+@pytest.mark.django_db
+def test_create_on_call_shift_rotation_missing_users(on_call_shift_internal_api_setup, make_user_auth_headers):
+    token, user1, user2, _, schedule = on_call_shift_internal_api_setup
+    client = APIClient()
+    url = reverse("api-internal:oncall_shifts-list")
+    start_date = timezone.now().replace(microsecond=0, tzinfo=None)
+
+    data = {
+        "name": "Test Shift",
+        "type": CustomOnCallShift.TYPE_ROLLING_USERS_EVENT,
+        "schedule": schedule.public_primary_key,
+        "priority_level": 1,
+        "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": 1,
+        "interval": 1,
+        "by_day": [
+            CustomOnCallShift.ICAL_WEEKDAY_MAP[CustomOnCallShift.MONDAY],
+            CustomOnCallShift.ICAL_WEEKDAY_MAP[CustomOnCallShift.FRIDAY],
+        ],
+        "week_start": CustomOnCallShift.ICAL_WEEKDAY_MAP[CustomOnCallShift.MONDAY],
+        "rolling_users": [],
+    }
+
+    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["rolling_users"][0] == "User(s) are required"
+
+
 @pytest.mark.django_db
 def test_create_on_call_shift_override(on_call_shift_internal_api_setup, make_user_auth_headers):
     token, user1, user2, _, schedule = on_call_shift_internal_api_setup
@@ -335,6 +367,48 @@ def test_update_future_on_call_shift(
     assert on_call_shift.priority_level == data_to_update["priority_level"]
 
 
+@pytest.mark.django_db
+def test_update_future_on_call_shift_removing_users(
+    on_call_shift_internal_api_setup,
+    make_on_call_shift,
+    make_user_auth_headers,
+):
+    token, user1, _, _, schedule = on_call_shift_internal_api_setup
+
+    client = APIClient()
+    start_date = (timezone.now() + timezone.timedelta(days=1)).replace(microsecond=0)
+
+    name = "Test Shift Rotation"
+    on_call_shift = make_on_call_shift(
+        schedule.organization,
+        shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT,
+        schedule=schedule,
+        name=name,
+        start=start_date,
+        duration=timezone.timedelta(hours=1),
+        rotation_start=start_date,
+        rolling_users=[{user1.pk: user1.public_primary_key}],
+    )
+    data_to_update = {
+        "name": name,
+        "priority_level": 2,
+        "shift_start": start_date.strftime("%Y-%m-%dT%H:%M:%SZ"),
+        "shift_end": (start_date + timezone.timedelta(hours=1)).strftime("%Y-%m-%dT%H:%M:%SZ"),
+        "rotation_start": start_date.strftime("%Y-%m-%dT%H:%M:%SZ"),
+        "until": None,
+        "frequency": None,
+        "interval": None,
+        "by_day": None,
+        "rolling_users": [],
+    }
+
+    url = reverse("api-internal:oncall_shifts-detail", kwargs={"pk": on_call_shift.public_primary_key})
+    response = client.put(url, data=data_to_update, format="json", **make_user_auth_headers(user1, token))
+
+    assert response.status_code == status.HTTP_400_BAD_REQUEST
+    assert response.data["rolling_users"][0] == "User(s) are required"
+
+
 @pytest.mark.django_db
 def test_update_started_on_call_shift(
     on_call_shift_internal_api_setup,