Check for duplicated positions in TF escalation policies (#1554)

Fixes https://github.com/grafana/oncall-private/issues/1680.
Avoid multiple escalation policies in an escalation chain to have the
same order.

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+### Fixed
+
+- Check for duplicated positions in terraform escalation policies create/update
+
 ### Added
 
 - Add `regex_match` Jinja filter ([1556](https://github.com/grafana/oncall/pull/1556))

engine/apps/public_api/serializers/escalation_policies.py

@@ -136,6 +136,8 @@ class EscalationPolicySerializer(EagerLoadingMixin, OrderedModelSerializerMixin,
             instance = super().create(validated_data)
             self._change_position(order, instance)
         else:
+            # validate will raise if there is a duplicated order
+            self._validate_manual_order(None, validated_data)
             instance = super().create(validated_data)
 
         return instance
@@ -209,6 +211,18 @@ class EscalationPolicySerializer(EagerLoadingMixin, OrderedModelSerializerMixin,
             result.pop(field, None)
         return result
 
+    def _validate_manual_order(self, instance, validated_data):
+        order = validated_data.get("order")
+        if order is None:
+            return
+
+        policies_with_order = self.escalation_chain.escalation_policies.filter(order=order)
+        if instance and instance.id:
+            policies_with_order = policies_with_order.exclude(id=instance.id)
+
+        if policies_with_order.exists():
+            raise BadRequest(detail="Steps cannot have duplicated positions")
+
     def _correct_validated_data(self, validated_data):
         validated_data_fields_to_remove = [
             "notify_to_users_queue",
@@ -302,5 +316,8 @@ class EscalationPolicyUpdateSerializer(EscalationPolicySerializer):
             order = validated_data.pop("order", None)
             self._validate_order(order, {"escalation_chain_id": instance.escalation_chain_id})
             self._change_position(order, instance)
+        else:
+            # validate will raise if there is a duplicated order
+            self._validate_manual_order(instance, validated_data)
 
         return super().update(instance, validated_data)

engine/apps/public_api/tests/test_escalation_policies.py

@@ -145,6 +145,29 @@ def test_create_escalation_policy(
     assert response.data == serializer.data
 
 
+@pytest.mark.django_db
+def test_create_escalation_policy_manual_order_duplicated_position(
+    make_organization_and_user_with_token,
+    escalation_policies_setup,
+):
+    organization, user, token = make_organization_and_user_with_token()
+    escalation_chain, _, _ = escalation_policies_setup(organization, user)
+
+    data_for_create = {
+        "escalation_chain_id": escalation_chain.public_primary_key,
+        "type": "notify_person_next_each_time",
+        "position": 0,
+        "persons_to_notify_next_each_time": [user.public_primary_key],
+        "manual_order": True,
+    }
+
+    client = APIClient()
+    url = reverse("api-public:escalation_policies-list")
+    response = client.post(url, data=data_for_create, format="json", HTTP_AUTHORIZATION=token)
+
+    assert response.status_code == status.HTTP_400_BAD_REQUEST
+
+
 @pytest.mark.django_db
 def test_invalid_step_type(
     make_organization_and_user_with_token,
@@ -219,3 +242,24 @@ def test_create_important_step(
     assert response.status_code == status.HTTP_201_CREATED
     assert escalation_policy.step == EscalationPolicy.STEP_NOTIFY_SCHEDULE_IMPORTANT
     assert response.data["important"] is True
+
+
+@pytest.mark.django_db
+def test_update_escalation_policy_manual_order_duplicated_position(
+    make_organization_and_user_with_token,
+    escalation_policies_setup,
+):
+    organization, user, token = make_organization_and_user_with_token()
+    _, escalation_policies, _ = escalation_policies_setup(organization, user)
+    escalation_policy_wait = escalation_policies[1]
+
+    client = APIClient()
+    url = reverse("api-public:escalation_policies-detail", kwargs={"pk": escalation_policy_wait.public_primary_key})
+    response = client.get(url, format="json", HTTP_AUTHORIZATION=token)
+
+    assert response.data["position"] != 0
+
+    data_to_change = {"position": 0, "manual_order": True}
+    response = client.put(url, data=data_to_change, format="json", HTTP_AUTHORIZATION=token)
+
+    assert response.status_code == status.HTTP_400_BAD_REQUEST