diff --git a/CHANGELOG.md b/CHANGELOG.md index 1172a2d3..b79881ea 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/engine/apps/public_api/serializers/escalation_policies.py b/engine/apps/public_api/serializers/escalation_policies.py index e7903cae..0193e5d3 100644 --- a/engine/apps/public_api/serializers/escalation_policies.py +++ b/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) diff --git a/engine/apps/public_api/tests/test_escalation_policies.py b/engine/apps/public_api/tests/test_escalation_policies.py index 06aa0345..b1b705e1 100644 --- a/engine/apps/public_api/tests/test_escalation_policies.py +++ b/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