Fix duplicate orders on routes and escalation policies (#2568)

# What this PR does

Fix duplicate `order` values for models `EscalationPolicy` and
`ChannelFilter` using the same approach as
https://github.com/grafana/oncall/pull/2278.

- Make internal API action `move_to_position` a part of
[OrderedModelViewSet](https://github.com/grafana/oncall/pull/2568/files#diff-eb62521ccbcb072d1bd2156adeadae3b5895bce6d0d54432a23db3948b0ada54R11-R34),
so all ordered model views use the same logic.
- Make public API serializers for ordered models inherit from
[OrderedModelSerializer](https://github.com/grafana/oncall/pull/2568/files#diff-d749f94af5a49adaf5074325cdfad10ddd5a52dbfd78b49582867ebb9c92fae5R6-R38),
so ordered model views are consistent with each other in public API.
- Remove `order` from plugin fronted, since it's not being used
anywhere. The frontend uses step indices & `move_to_position` action
instead.
- Make escalation snapshot use step indices instead of orders, since
orders are not guaranteed to be sequential (+fix a minor off-by-one bug)

## Which issue(s) this PR fixes

https://github.com/grafana/oncall-private/issues/1680

## Checklist

- [x] Unit, integration, and e2e (if applicable) tests updated
- [x] Documentation added (or `pr:no public docs` PR label added if not
required)
- [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required)

CHANGELOG.md

@@ -13,6 +13,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Update Slack "invite" feature to use direct paging by @vadimkerr ([#2562](https://github.com/grafana/oncall/pull/2562))
 - Change "Current responders" to "Additional Responders" in web UI by @vadimkerr ([#2567](https://github.com/grafana/oncall/pull/2567))
 
+### Fixed
+
+- Fix duplicate orders on routes and escalation policies by @vadimkerr ([#2568](https://github.com/grafana/oncall/pull/2568))
+
 ## v1.3.14 (2023-07-17)
 
 ### Changed

engine/apps/alerts/escalation_snapshot/snapshot_classes/escalation_snapshot.py

@@ -88,9 +88,7 @@ class EscalationSnapshot:
         """
         if self.last_active_escalation_policy_order is None:
             return []
-        elif self.last_active_escalation_policy_order == 0:
-            return [self.escalation_policies_snapshots[0]]
-        return self.escalation_policies_snapshots[: self.last_active_escalation_policy_order]
+        return self.escalation_policies_snapshots[: self.last_active_escalation_policy_order + 1]
 
     def next_step_eta_is_valid(self) -> typing.Optional[bool]:
         """
@@ -147,7 +145,8 @@ class EscalationSnapshot:
             self.stop_escalation = execution_result.stop_escalation  # result of STEP_FINAL_RESOLVE
             self.pause_escalation = execution_result.pause_escalation  # result of STEP_NOTIFY_IF_NUM_ALERTS_IN_WINDOW
 
-            last_active_escalation_policy_order = escalation_policy_snapshot.order
+            # use the index of last escalation policy snapshot, since orders are not guaranteed to be sequential
+            last_active_escalation_policy_order = self.escalation_policies_snapshots.index(escalation_policy_snapshot)
 
             if execution_result.start_from_beginning:  # result of STEP_REPEAT_ESCALATION_N_TIMES
                 last_active_escalation_policy_order = None

engine/apps/alerts/migrations/0024_auto_20230718_0953.py

@@ -0,0 +1,54 @@
+# Generated by Django 3.2.20 on 2023-07-18 09:53
+
+from django.db import migrations, models
+import django_migration_linter as linter
+from django.db.models import Count
+
+from common.database import get_random_readonly_database_key_if_present_otherwise_default
+
+
+def fix_duplicate_orders(apps, schema_editor):
+    EscalationPolicy = apps.get_model('alerts', 'EscalationPolicy')
+
+    # it should be safe to use a readonly database because duplicates are pretty infrequent
+    db = get_random_readonly_database_key_if_present_otherwise_default()
+
+    # find all (escalation_chain_id, order) tuples that have more than one entry (meaning duplicates)
+    items_with_duplicate_orders = EscalationPolicy.objects.using(db).values(
+        "escalation_chain_id", "order"
+    ).annotate(count=Count("order")).order_by().filter(count__gt=1)  # use order_by() to reset any existing ordering
+
+    # make sure we don't fix the same escalation chain more than once
+    escalation_chain_ids = set(item["escalation_chain_id"] for item in items_with_duplicate_orders)
+
+    for escalation_chain_id in escalation_chain_ids:
+        policies = EscalationPolicy.objects.filter(escalation_chain_id=escalation_chain_id).order_by("order", "id")
+        # assign correct sequential order for each policy starting from 0
+        for idx, policy in enumerate(policies):
+            policy.order = idx
+        EscalationPolicy.objects.bulk_update(policies, fields=["order"])
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('alerts', '0023_auto_20230718_0952'),
+    ]
+
+    operations = [
+        linter.IgnoreMigration(),  # adding a unique constraint after fixing duplicates should be fine
+        migrations.AlterModelOptions(
+            name='escalationpolicy',
+            options={'ordering': ['order']},
+        ),
+        migrations.AlterField(
+            model_name='escalationpolicy',
+            name='order',
+            field=models.PositiveIntegerField(db_index=True, editable=False, null=True),
+        ),
+        migrations.RunPython(fix_duplicate_orders, migrations.RunPython.noop),
+        migrations.AddConstraint(
+            model_name='escalationpolicy',
+            constraint=models.UniqueConstraint(fields=('escalation_chain_id', 'order'), name='unique_escalation_policy_order'),
+        ),
+    ]

engine/apps/alerts/migrations/0025_auto_20230718_1042.py

@@ -0,0 +1,56 @@
+# Generated by Django 3.2.20 on 2023-07-18 10:42
+
+from django.db import migrations, models
+import django_migration_linter as linter
+from django.db.models import Count
+
+from common.database import get_random_readonly_database_key_if_present_otherwise_default
+
+
+def fix_duplicate_orders(apps, schema_editor):
+    ChannelFilter = apps.get_model('alerts', 'ChannelFilter')
+
+    # it should be safe to use a readonly database because duplicates are pretty infrequent
+    db = get_random_readonly_database_key_if_present_otherwise_default()
+
+    # find all (alert_receive_channel_id, is_default, order) tuples that have more than one entry (meaning duplicates)
+    items_with_duplicate_orders = ChannelFilter.objects.using(db).values(
+        "alert_receive_channel_id", "is_default", "order"
+    ).annotate(count=Count("order")).order_by().filter(count__gt=1)  # use order_by() to reset any existing ordering
+
+    # make sure we don't fix the same (alert_receive_channel_id, is_default) pair more than once
+    values_to_fix = set((item["alert_receive_channel_id"], item["is_default"]) for item in items_with_duplicate_orders)
+
+    for alert_receive_channel_id, is_default in values_to_fix:
+        channel_filters = ChannelFilter.objects.filter(
+            alert_receive_channel_id=alert_receive_channel_id, is_default=is_default
+        ).order_by("order", "id")
+        # assign correct sequential order for each route starting from 0
+        for idx, channel_filter in enumerate(channel_filters):
+            channel_filter.order = idx
+        ChannelFilter.objects.bulk_update(channel_filters, fields=["order"])
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('alerts', '0024_auto_20230718_0953'),
+    ]
+
+    operations = [
+        linter.IgnoreMigration(),  # adding a unique constraint after fixing duplicates should be fine
+        migrations.AlterModelOptions(
+            name='channelfilter',
+            options={'ordering': ['alert_receive_channel_id', 'is_default', 'order']},
+        ),
+        migrations.AlterField(
+            model_name='channelfilter',
+            name='order',
+            field=models.PositiveIntegerField(db_index=True, editable=False, null=True),
+        ),
+        migrations.RunPython(fix_duplicate_orders, migrations.RunPython.noop),
+        migrations.AddConstraint(
+            model_name='channelfilter',
+            constraint=models.UniqueConstraint(fields=('alert_receive_channel_id', 'is_default', 'order'), name='unique_channel_filter_order'),
+        ),
+    ]

engine/apps/alerts/models/channel_filter.py

@@ -6,10 +6,10 @@ from django.apps import apps
 from django.conf import settings
 from django.core.validators import MinLengthValidator
 from django.db import models
-from ordered_model.models import OrderedModel
 
 from common.jinja_templater import apply_jinja_template
 from common.jinja_templater.apply_jinja_template import JinjaTemplateError, JinjaTemplateWarning
+from common.ordered_model.ordered_model import OrderedModel
 from common.public_primary_keys import generate_public_primary_key, increase_public_primary_key_length
 
 logger = logging.getLogger(__name__)
@@ -34,7 +34,7 @@ class ChannelFilter(OrderedModel):
     Actually it's a Router based on terms now. Not a Filter.
     """
 
-    order_with_respect_to = ("alert_receive_channel", "is_default")
+    order_with_respect_to = ["alert_receive_channel_id", "is_default"]
 
     public_primary_key = models.CharField(
         max_length=20,
@@ -82,11 +82,12 @@ class ChannelFilter(OrderedModel):
     is_default = models.BooleanField(default=False)
 
     class Meta:
-        ordering = (
-            "alert_receive_channel",
-            "is_default",
-            "order",
-        )
+        ordering = ["alert_receive_channel_id", "is_default", "order"]
+        constraints = [
+            models.UniqueConstraint(
+                fields=["alert_receive_channel_id", "is_default", "order"], name="unique_channel_filter_order"
+            )
+        ]
 
     def __str__(self):
         return f"{self.pk}: {self.filtering_term or 'default'}"

engine/apps/alerts/models/escalation_policy.py

@@ -3,8 +3,8 @@ import datetime
 from django.conf import settings
 from django.core.validators import MinLengthValidator
 from django.db import models
-from ordered_model.models import OrderedModel
 
+from common.ordered_model.ordered_model import OrderedModel
 from common.public_primary_keys import generate_public_primary_key, increase_public_primary_key_length
 
 
@@ -23,7 +23,7 @@ def generate_public_primary_key_for_escalation_policy():
 
 
 class EscalationPolicy(OrderedModel):
-    order_with_respect_to = "escalation_chain"
+    order_with_respect_to = ["escalation_chain_id"]
 
     MAX_TIMES_REPEAT = 5
 
@@ -312,6 +312,12 @@ class EscalationPolicy(OrderedModel):
     num_alerts_in_window = models.PositiveIntegerField(null=True, default=None)
     num_minutes_in_window = models.PositiveIntegerField(null=True, default=None)
 
+    class Meta:
+        ordering = ["order"]
+        constraints = [
+            models.UniqueConstraint(fields=["escalation_chain_id", "order"], name="unique_escalation_policy_order")
+        ]
+
     def __str__(self):
         return f"{self.pk}: {self.step_type_verbal}"
 

engine/apps/alerts/tests/test_escalation_snapshot.py

@@ -213,5 +213,56 @@ def test_executed_escalation_policy_snapshots(escalation_snapshot_test_setup):
         escalation_snapshot.escalation_policies_snapshots[0]
     ]
 
-    escalation_snapshot.last_active_escalation_policy_order = len(escalation_snapshot.escalation_policies_snapshots)
+    escalation_snapshot.last_active_escalation_policy_order = len(escalation_snapshot.escalation_policies_snapshots) - 1
     assert escalation_snapshot.executed_escalation_policy_snapshots == escalation_snapshot.escalation_policies_snapshots
+
+
+@pytest.mark.django_db
+def test_escalation_snapshot_non_sequential_orders(
+    make_organization,
+    make_alert_receive_channel,
+    make_escalation_chain,
+    make_channel_filter,
+    make_escalation_policy,
+    make_alert_group,
+):
+    organization = make_organization()
+
+    alert_receive_channel = make_alert_receive_channel(organization)
+
+    escalation_chain = make_escalation_chain(organization)
+    channel_filter = make_channel_filter(
+        alert_receive_channel,
+        escalation_chain=escalation_chain,
+        notification_backends={"BACKEND": {"channel_id": "abc123"}},
+    )
+
+    step_1 = make_escalation_policy(
+        escalation_chain=channel_filter.escalation_chain,
+        escalation_policy_step=EscalationPolicy.STEP_WAIT,
+        order=12,
+    )
+    step_2 = make_escalation_policy(
+        escalation_chain=channel_filter.escalation_chain,
+        escalation_policy_step=EscalationPolicy.STEP_WAIT,
+        order=42,
+    )
+
+    alert_group = make_alert_group(alert_receive_channel, channel_filter=channel_filter)
+    alert_group.raw_escalation_snapshot = alert_group.build_raw_escalation_snapshot()
+    alert_group.save()
+
+    escalation_snapshot = alert_group.escalation_snapshot
+    assert escalation_snapshot.last_active_escalation_policy_order is None
+    assert escalation_snapshot.next_active_escalation_policy_snapshot.id == step_1.id
+
+    escalation_snapshot.execute_actual_escalation_step()
+    assert escalation_snapshot.last_active_escalation_policy_order == 0
+    assert escalation_snapshot.next_active_escalation_policy_snapshot.id == step_2.id
+
+    escalation_snapshot.execute_actual_escalation_step()
+    assert escalation_snapshot.last_active_escalation_policy_order == 1
+    assert escalation_snapshot.next_active_escalation_policy_snapshot is None
+
+    policy_ids = [p.id for p in escalation_snapshot.executed_escalation_policy_snapshots]
+    assert policy_ids == [step_1.id, step_2.id]

engine/apps/api/serializers/channel_filter.py

@@ -7,12 +7,12 @@ from apps.base.messaging import get_messaging_backend_from_id
 from apps.telegram.models import TelegramToOrganizationConnector
 from common.api_helpers.custom_fields import OrganizationFilteredPrimaryKeyRelatedField
 from common.api_helpers.exceptions import BadRequest
-from common.api_helpers.mixins import EagerLoadingMixin, OrderedModelSerializerMixin
+from common.api_helpers.mixins import EagerLoadingMixin
 from common.jinja_templater.apply_jinja_template import JinjaTemplateError
 from common.utils import is_regex_valid
 
 
-class ChannelFilterSerializer(OrderedModelSerializerMixin, EagerLoadingMixin, serializers.ModelSerializer):
+class ChannelFilterSerializer(EagerLoadingMixin, serializers.ModelSerializer):
     id = serializers.CharField(read_only=True, source="public_primary_key")
     alert_receive_channel = OrganizationFilteredPrimaryKeyRelatedField(queryset=AlertReceiveChannel.objects)
     escalation_chain = OrganizationFilteredPrimaryKeyRelatedField(
@@ -27,7 +27,6 @@ class ChannelFilterSerializer(OrderedModelSerializerMixin, EagerLoadingMixin, se
         queryset=TelegramToOrganizationConnector.objects, filter_field="organization", allow_null=True, required=False
     )
     telegram_channel_details = serializers.SerializerMethodField()
-    order = serializers.IntegerField(required=False)
     filtering_term_as_jinja2 = serializers.SerializerMethodField()
     filtering_term = serializers.CharField(required=False, allow_null=True, allow_blank=True)
 
@@ -37,7 +36,6 @@ class ChannelFilterSerializer(OrderedModelSerializerMixin, EagerLoadingMixin, se
         model = ChannelFilter
         fields = [
             "id",
-            "order",
             "alert_receive_channel",
             "escalation_chain",
             "slack_channel",
@@ -148,7 +146,6 @@ class ChannelFilterCreateSerializer(ChannelFilterSerializer):
         model = ChannelFilter
         fields = [
             "id",
-            "order",
             "alert_receive_channel",
             "escalation_chain",
             "slack_channel",
@@ -181,14 +178,8 @@ class ChannelFilterCreateSerializer(ChannelFilterSerializer):
         return result
 
     def create(self, validated_data):
-        order = validated_data.pop("order", None)
-        if order is not None:
-            alert_receive_channel_id = validated_data.get("alert_receive_channel")
-            self._validate_order(order, {"alert_receive_channel_id": alert_receive_channel_id, "is_default": False})
-            instance = super().create(validated_data)
-            self._change_position(order, instance)
-        else:
-            instance = super().create(validated_data)
+        instance = super().create(validated_data)
+        instance.to_index(0)  # the new route should be the first one
         return instance
 
 
@@ -200,18 +191,8 @@ class ChannelFilterUpdateSerializer(ChannelFilterCreateSerializer):
         extra_kwargs = {"filtering_term": {"required": False}}
 
     def update(self, instance, validated_data):
-        order = validated_data.get("order")
         filtering_term = validated_data.get("filtering_term")
-
-        if instance.is_default and order is not None and instance.order != order:
-            raise BadRequest(detail="The order of default channel filter cannot be changed")
-
         if instance.is_default and filtering_term is not None:
             raise BadRequest(detail="Filtering term of default channel filter cannot be changed")
 
-        if order is not None:
-            self._validate_order(
-                order, {"alert_receive_channel_id": instance.alert_receive_channel_id, "is_default": False}
-            )
-            self._change_position(order, instance)
         return super().update(instance, validated_data)

engine/apps/api/serializers/escalation_policy.py

@@ -85,7 +85,6 @@ class EscalationPolicySerializer(EagerLoadingMixin, serializers.ModelSerializer)
         model = EscalationPolicy
         fields = [
             "id",
-            "order",
             "step",
             "wait_delay",
             "escalation_chain",
@@ -101,7 +100,6 @@ class EscalationPolicySerializer(EagerLoadingMixin, serializers.ModelSerializer)
             "notify_to_group",
             "important",
         ]
-        read_only_fields = ["order"]
 
     SELECT_RELATED = [
         "escalation_chain",
@@ -199,7 +197,6 @@ class EscalationPolicySerializer(EagerLoadingMixin, serializers.ModelSerializer)
 
 class EscalationPolicyCreateSerializer(EscalationPolicySerializer):
     class Meta(EscalationPolicySerializer.Meta):
-        read_only_fields = ["order"]
         extra_kwargs = {"escalation_chain": {"required": True, "allow_null": False}}
 
     def create(self, validated_data):
@@ -212,7 +209,7 @@ class EscalationPolicyUpdateSerializer(EscalationPolicySerializer):
     escalation_chain = serializers.CharField(read_only=True, source="escalation_chain.public_primary_key")
 
     class Meta(EscalationPolicySerializer.Meta):
-        read_only_fields = ["order", "escalation_chain"]
+        read_only_fields = ["escalation_chain"]
 
     def update(self, instance, validated_data):
         step = validated_data.get("step", instance.step)

engine/apps/api/serializers/user_notification_policy.py

@@ -33,7 +33,11 @@ class UserNotificationPolicyBaseSerializer(EagerLoadingMixin, serializers.ModelS
 
     class Meta:
         model = UserNotificationPolicy
-        fields = ["id", "step", "order", "notify_by", "wait_delay", "important", "user"]
+        fields = ["id", "step", "notify_by", "wait_delay", "important", "user"]
+
+        # Field "order" is not consumed by the plugin frontend, but is used by the mobile app
+        # TODO: remove this field when the mobile app is updated
+        fields += ["order"]
         read_only_fields = ["order"]
 
     def to_internal_value(self, data):
@@ -100,7 +104,7 @@ class UserNotificationPolicyUpdateSerializer(UserNotificationPolicyBaseSerialize
     )
 
     class Meta(UserNotificationPolicyBaseSerializer.Meta):
-        read_only_fields = ["order", "user", "important"]
+        read_only_fields = UserNotificationPolicyBaseSerializer.Meta.read_only_fields + ["user", "important"]
 
     def update(self, instance, validated_data):
         self_or_admin = instance.user.self_or_admin(

engine/apps/api/tests/test_channel_filter.py

@@ -6,6 +6,7 @@ from rest_framework import status
 from rest_framework.response import Response
 from rest_framework.test import APIClient
 
+from apps.alerts.models import ChannelFilter
 from apps.api.permissions import LegacyAccessControlRole
 
 
@@ -220,7 +221,7 @@ def test_channel_filter_move_to_position_permissions(
 
 
 @pytest.mark.django_db
-def test_channel_filter_create_with_order(
+def test_channel_filter_create_order(
     make_organization_and_user_with_plugin_token,
     make_alert_receive_channel,
     make_escalation_chain,
@@ -230,7 +231,6 @@ def test_channel_filter_create_with_order(
     organization, user, token = make_organization_and_user_with_plugin_token()
     alert_receive_channel = make_alert_receive_channel(organization)
     make_escalation_chain(organization)
-    # create default channel filter
     make_channel_filter(alert_receive_channel, is_default=True)
     channel_filter = make_channel_filter(alert_receive_channel, filtering_term="a", is_default=False)
     client = APIClient()
@@ -239,46 +239,18 @@ def test_channel_filter_create_with_order(
     data_for_creation = {
         "alert_receive_channel": alert_receive_channel.public_primary_key,
         "filtering_term": "b",
-        "order": 0,
     }
 
     response = client.post(url, data=data_for_creation, format="json", **make_user_auth_headers(user, token))
     channel_filter.refresh_from_db()
 
     assert response.status_code == status.HTTP_201_CREATED
-    assert response.json()["order"] == 0
+
+    # check that orders are correct
+    assert ChannelFilter.objects.get(public_primary_key=response.json()["id"]).order == 0
     assert channel_filter.order == 1
 
 
-@pytest.mark.django_db
-def test_channel_filter_create_without_order(
-    make_organization_and_user_with_plugin_token,
-    make_alert_receive_channel,
-    make_escalation_chain,
-    make_channel_filter,
-    make_user_auth_headers,
-):
-    organization, user, token = make_organization_and_user_with_plugin_token()
-    alert_receive_channel = make_alert_receive_channel(organization)
-    make_escalation_chain(organization)
-    make_channel_filter(alert_receive_channel, is_default=True)
-    channel_filter = make_channel_filter(alert_receive_channel, filtering_term="a", is_default=False)
-    client = APIClient()
-
-    url = reverse("api-internal:channel_filter-list")
-    data_for_creation = {
-        "alert_receive_channel": alert_receive_channel.public_primary_key,
-        "filtering_term": "b",
-    }
-
-    response = client.post(url, data=data_for_creation, format="json", **make_user_auth_headers(user, token))
-    channel_filter.refresh_from_db()
-
-    assert response.status_code == status.HTTP_201_CREATED
-    assert response.json()["order"] == 1
-    assert channel_filter.order == 0
-
-
 @pytest.mark.django_db
 def test_move_to_position(
     make_organization_and_user_with_plugin_token,
@@ -297,7 +269,7 @@ def test_move_to_position(
     url = reverse(
         "api-internal:channel_filter-move-to-position", kwargs={"pk": first_channel_filter.public_primary_key}
     )
-    url += f"?position=2"
+    url += f"?position=1"
     response = client.put(url, **make_user_auth_headers(user, token))
 
     assert response.status_code == status.HTTP_200_OK
@@ -307,6 +279,30 @@ def test_move_to_position(
     assert second_channel_filter.order == 1
 
 
+@pytest.mark.django_db
+def test_move_to_position_invalid_index(
+    make_organization_and_user_with_plugin_token,
+    make_alert_receive_channel,
+    make_channel_filter,
+    make_user_auth_headers,
+):
+    organization, user, token = make_organization_and_user_with_plugin_token()
+    alert_receive_channel = make_alert_receive_channel(organization)
+    # create default channel filter
+    make_channel_filter(alert_receive_channel, is_default=True, order=0)
+    first_channel_filter = make_channel_filter(alert_receive_channel, filtering_term="a", is_default=False, order=1)
+    make_channel_filter(alert_receive_channel, filtering_term="b", is_default=False, order=2)
+
+    client = APIClient()
+    url = reverse(
+        "api-internal:channel_filter-move-to-position", kwargs={"pk": first_channel_filter.public_primary_key}
+    )
+    url += f"?position=2"
+    response = client.put(url, **make_user_auth_headers(user, token))
+
+    assert response.status_code == status.HTTP_400_BAD_REQUEST
+
+
 @pytest.mark.django_db
 def test_move_to_position_cant_move_default(
     make_organization_and_user_with_plugin_token,
@@ -331,42 +327,7 @@ def test_move_to_position_cant_move_default(
 
 
 @pytest.mark.django_db
-def test_channel_filter_update_with_order(
-    make_organization_and_user_with_plugin_token,
-    make_alert_receive_channel,
-    make_channel_filter,
-    make_user_auth_headers,
-):
-    organization, user, token = make_organization_and_user_with_plugin_token()
-    alert_receive_channel = make_alert_receive_channel(organization)
-    # create default channel filter
-    make_channel_filter(alert_receive_channel, is_default=True)
-    first_channel_filter = make_channel_filter(alert_receive_channel, filtering_term="a", is_default=False)
-    second_channel_filter = make_channel_filter(alert_receive_channel, filtering_term="b", is_default=False)
-
-    client = APIClient()
-
-    url = reverse("api-internal:channel_filter-detail", kwargs={"pk": first_channel_filter.public_primary_key})
-    data_for_update = {
-        "id": first_channel_filter.public_primary_key,
-        "alert_receive_channel": alert_receive_channel.public_primary_key,
-        "order": 1,
-        "filtering_term": first_channel_filter.filtering_term,
-    }
-
-    response = client.put(url, data=data_for_update, format="json", **make_user_auth_headers(user, token))
-
-    first_channel_filter.refresh_from_db()
-    second_channel_filter.refresh_from_db()
-
-    assert response.status_code == status.HTTP_200_OK
-    assert response.json()["order"] == 1
-    assert first_channel_filter.order == 1
-    assert second_channel_filter.order == 0
-
-
-@pytest.mark.django_db
-def test_channel_filter_update_without_order(
+def test_channel_filter_update(
     make_organization_and_user_with_plugin_token,
     make_alert_receive_channel,
     make_channel_filter,
@@ -394,7 +355,6 @@ def test_channel_filter_update_without_order(
     second_channel_filter.refresh_from_db()
 
     assert response.status_code == status.HTTP_200_OK
-    assert response.json()["order"] == 0
     assert first_channel_filter.order == 0
     assert second_channel_filter.order == 1
 

engine/apps/api/tests/test_escalation_policy.py

@@ -53,7 +53,7 @@ def test_create_escalation_policy(escalation_policy_internal_api_setup, make_use
 
     response = client.post(url, data, format="json", **make_user_auth_headers(user, token))
     assert response.status_code == status.HTTP_201_CREATED
-    assert response.data["order"] == max_order + 1
+    assert EscalationPolicy.objects.get(public_primary_key=response.data["id"]).order == max_order + 1
 
 
 @pytest.mark.django_db
@@ -77,9 +77,9 @@ def test_create_escalation_policy_webhook(
 
     response = client.post(url, data, format="json", **make_user_auth_headers(user, token))
     assert response.status_code == status.HTTP_201_CREATED
-    assert response.data["order"] == max_order + 1
     assert response.data["custom_webhook"] == webhook.public_primary_key
     escalation_policy = EscalationPolicy.objects.get(public_primary_key=response.data["id"])
+    assert escalation_policy.order == max_order + 1
     assert escalation_policy.custom_webhook == webhook
 
 
@@ -107,7 +107,7 @@ def test_move_to_position(escalation_policy_internal_api_setup, make_user_auth_h
     token, _, escalation_policy, user, _ = escalation_policy_internal_api_setup
     client = APIClient()
 
-    position_to_move = 1
+    position_to_move = 0
     url = reverse(
         "api-internal:escalation_policy-move-to-position", kwargs={"pk": escalation_policy.public_primary_key}
     )
@@ -119,6 +119,22 @@ def test_move_to_position(escalation_policy_internal_api_setup, make_user_auth_h
     assert escalation_policy.order == position_to_move
 
 
+@pytest.mark.django_db
+def test_move_to_position_invalid_index(escalation_policy_internal_api_setup, make_user_auth_headers):
+    token, _, escalation_policy, user, _ = escalation_policy_internal_api_setup
+    client = APIClient()
+
+    position_to_move = 1
+    url = reverse(
+        "api-internal:escalation_policy-move-to-position", kwargs={"pk": escalation_policy.public_primary_key}
+    )
+    response = client.put(
+        f"{url}?position={position_to_move}", content_type="application/json", **make_user_auth_headers(user, token)
+    )
+    escalation_policy.refresh_from_db()
+    assert response.status_code == status.HTTP_400_BAD_REQUEST
+
+
 @pytest.mark.django_db
 @pytest.mark.parametrize(
     "role,expected_status",
@@ -736,7 +752,6 @@ def test_escalation_policy_switch_importance(
     data_for_update = {
         "id": escalation_policy.public_primary_key,
         "step": escalation_policy.step,
-        "order": escalation_policy.order,
         "escalation_chain": escalation_chain.public_primary_key,
         "notify_to_users_queue": [],
         "from_time": None,
@@ -792,7 +807,6 @@ def test_escalation_policy_filter_by_user(
     expected_payload = [
         {
             "id": escalation_policy_with_one_user.public_primary_key,
-            "order": 0,
             "step": 13,
             "wait_delay": None,
             "escalation_chain": escalation_chain.public_primary_key,
@@ -810,7 +824,6 @@ def test_escalation_policy_filter_by_user(
         },
         {
             "id": escalation_policy_with_two_users.public_primary_key,
-            "order": 1,
             "step": 13,
             "wait_delay": None,
             "escalation_chain": escalation_chain.public_primary_key,
@@ -873,7 +886,6 @@ def test_escalation_policy_filter_by_slack_channel(
     expected_payload = [
         {
             "id": escalation_policy_from_alert_receive_channel_with_slack_channel.public_primary_key,
-            "order": 0,
             "step": 0,
             "wait_delay": None,
             "escalation_chain": escalation_chain.public_primary_key,

engine/apps/api/views/channel_filter.py

@@ -3,7 +3,6 @@ from rest_framework import status
 from rest_framework.decorators import action
 from rest_framework.permissions import IsAuthenticated
 from rest_framework.response import Response
-from rest_framework.viewsets import ModelViewSet
 
 from apps.alerts.models import ChannelFilter
 from apps.api.permissions import RBACPermission
@@ -21,12 +20,16 @@ from common.api_helpers.mixins import (
     TeamFilteringMixin,
     UpdateSerializerMixin,
 )
-from common.api_helpers.serializers import get_move_to_position_param
 from common.insight_log import EntityEvent, write_resource_insight_log
+from common.ordered_model.viewset import OrderedModelViewSet
 
 
 class ChannelFilterView(
-    TeamFilteringMixin, PublicPrimaryKeyMixin, CreateSerializerMixin, UpdateSerializerMixin, ModelViewSet
+    TeamFilteringMixin,
+    PublicPrimaryKeyMixin,
+    CreateSerializerMixin,
+    UpdateSerializerMixin,
+    OrderedModelViewSet,
 ):
     authentication_classes = (PluginAuthentication,)
     permission_classes = (IsAuthenticated, RBACPermission)
@@ -109,24 +112,10 @@ class ChannelFilterView(
     @action(detail=True, methods=["put"])
     def move_to_position(self, request, pk):
         instance = self.get_object()
-        position = get_move_to_position_param(request)
-
         if instance.is_default:
             raise BadRequest(detail="Unable to change position for default filter")
 
-        prev_state = instance.insight_logs_serialized
-        instance.to(position)
-        new_state = instance.insight_logs_serialized
-
-        write_resource_insight_log(
-            instance=instance,
-            author=self.request.user,
-            event=EntityEvent.UPDATED,
-            prev_state=prev_state,
-            new_state=new_state,
-        )
-
-        return Response(status=status.HTTP_200_OK)
+        return super().move_to_position(request, pk)
 
     @action(detail=True, methods=["post"])
     def convert_from_regex_to_jinja2(self, request, pk):

engine/apps/api/views/escalation_policy.py

@@ -1,10 +1,8 @@
 from django.conf import settings
 from django.db.models import Q
-from rest_framework import status
 from rest_framework.decorators import action
 from rest_framework.permissions import IsAuthenticated
 from rest_framework.response import Response
-from rest_framework.viewsets import ModelViewSet
 
 from apps.alerts.models import EscalationPolicy
 from apps.api.permissions import RBACPermission
@@ -20,12 +18,16 @@ from common.api_helpers.mixins import (
     TeamFilteringMixin,
     UpdateSerializerMixin,
 )
-from common.api_helpers.serializers import get_move_to_position_param
 from common.insight_log import EntityEvent, write_resource_insight_log
+from common.ordered_model.viewset import OrderedModelViewSet
 
 
 class EscalationPolicyView(
-    TeamFilteringMixin, PublicPrimaryKeyMixin, CreateSerializerMixin, UpdateSerializerMixin, ModelViewSet
+    TeamFilteringMixin,
+    PublicPrimaryKeyMixin,
+    CreateSerializerMixin,
+    UpdateSerializerMixin,
+    OrderedModelViewSet,
 ):
     authentication_classes = (PluginAuthentication,)
     permission_classes = (IsAuthenticated, RBACPermission)
@@ -108,25 +110,6 @@ class EscalationPolicyView(
         )
         instance.delete()
 
-    @action(detail=True, methods=["put"])
-    def move_to_position(self, request, pk):
-        instance = self.get_object()
-        position = get_move_to_position_param(request)
-
-        prev_state = instance.insight_logs_serialized
-        instance.to(position)
-        new_state = instance.insight_logs_serialized
-
-        write_resource_insight_log(
-            instance=instance,
-            author=self.request.user,
-            event=EntityEvent.UPDATED,
-            prev_state=prev_state,
-            new_state=new_state,
-        )
-
-        return Response(status=status.HTTP_200_OK)
-
     @action(detail=False, methods=["get"])
     def escalation_options(self, request):
         choices = []

engine/apps/api/views/user_notification_policy.py

@@ -1,10 +1,8 @@
 from django.conf import settings
 from django.http import Http404
-from rest_framework import status
 from rest_framework.decorators import action
 from rest_framework.permissions import IsAuthenticated
 from rest_framework.response import Response
-from rest_framework.viewsets import ModelViewSet
 
 from apps.api.permissions import IsOwnerOrHasRBACPermissions, RBACPermission
 from apps.api.serializers.user_notification_policy import (
@@ -19,12 +17,12 @@ from apps.mobile_app.auth import MobileAppAuthTokenAuthentication
 from apps.user_management.models import User
 from common.api_helpers.exceptions import BadRequest
 from common.api_helpers.mixins import UpdateSerializerMixin
-from common.api_helpers.serializers import get_move_to_position_param
 from common.exceptions import UserNotificationPolicyCouldNotBeDeleted
 from common.insight_log import EntityEvent, write_resource_insight_log
+from common.ordered_model.viewset import OrderedModelViewSet
 
 
-class UserNotificationPolicyView(UpdateSerializerMixin, ModelViewSet):
+class UserNotificationPolicyView(UpdateSerializerMixin, OrderedModelViewSet):
     authentication_classes = (
         MobileAppAuthTokenAuthentication,
         PluginAuthentication,
@@ -78,9 +76,7 @@ class UserNotificationPolicyView(UpdateSerializerMixin, ModelViewSet):
 
             queryset = self.model.objects.filter(user=target_user, important=important)
 
-        queryset = self.serializer_class.setup_eager_loading(queryset)
-
-        return queryset.order_by("order")
+        return self.serializer_class.setup_eager_loading(queryset)
 
     def get_object(self):
         # we need overriden get object, because original one call get_queryset first and raise 404 trying to access
@@ -138,18 +134,6 @@ class UserNotificationPolicyView(UpdateSerializerMixin, ModelViewSet):
             new_state=new_state,
         )
 
-    @action(detail=True, methods=["put"])
-    def move_to_position(self, request, pk):
-        instance = self.get_object()
-        position = get_move_to_position_param(request)
-
-        try:
-            instance.to_index(position)
-        except IndexError:
-            raise BadRequest(detail="Invalid position")
-
-        return Response(status=status.HTTP_200_OK)
-
     @action(detail=False, methods=["get"])
     def delay_options(self, request):
         choices = []

engine/apps/base/models/user_notification_policy.py

@@ -9,9 +9,9 @@ from django.db import IntegrityError, models
 from django.db.models import Q
 
 from apps.base.messaging import get_messaging_backends
-from apps.base.models.ordered_model import OrderedModel
 from apps.user_management.models import User
 from common.exceptions import UserNotificationPolicyCouldNotBeDeleted
+from common.ordered_model.ordered_model import OrderedModel
 from common.public_primary_keys import generate_public_primary_key, increase_public_primary_key_length
 
 

engine/apps/public_api/serializers/escalation_policies.py

@@ -14,7 +14,8 @@ from common.api_helpers.custom_fields import (
     UsersFilteredByOrganizationField,
 )
 from common.api_helpers.exceptions import BadRequest
-from common.api_helpers.mixins import EagerLoadingMixin, OrderedModelSerializerMixin
+from common.api_helpers.mixins import EagerLoadingMixin
+from common.ordered_model.serializer import OrderedModelSerializer
 
 
 class EscalationPolicyTypeField(fields.CharField):
@@ -35,12 +36,11 @@ class EscalationPolicyTypeField(fields.CharField):
         return step_type
 
 
-class EscalationPolicySerializer(EagerLoadingMixin, OrderedModelSerializerMixin, serializers.ModelSerializer):
+class EscalationPolicySerializer(EagerLoadingMixin, OrderedModelSerializer):
     id = serializers.CharField(read_only=True, source="public_primary_key")
     escalation_chain_id = OrganizationFilteredPrimaryKeyRelatedField(
         queryset=EscalationChain.objects, source="escalation_chain"
     )
-    position = serializers.IntegerField(required=False, source="order")
     type = EscalationPolicyTypeField(source="step", allow_null=True)
     duration = serializers.ChoiceField(required=False, source="wait_delay", choices=EscalationPolicy.DURATION_CHOICES)
     persons_to_notify = UsersFilteredByOrganizationField(
@@ -67,17 +67,15 @@ class EscalationPolicySerializer(EagerLoadingMixin, OrderedModelSerializerMixin,
         required=False,
         source="custom_button_trigger",
     )
-    manual_order = serializers.BooleanField(default=False, write_only=True)
     important = serializers.BooleanField(required=False)
     notify_if_time_from = CustomTimeField(required=False, source="from_time")
     notify_if_time_to = CustomTimeField(required=False, source="to_time")
 
     class Meta:
         model = EscalationPolicy
-        fields = [
+        fields = OrderedModelSerializer.Meta.fields + [
             "id",
             "escalation_chain_id",
-            "position",
             "type",
             "duration",
             "important",
@@ -86,7 +84,6 @@ class EscalationPolicySerializer(EagerLoadingMixin, OrderedModelSerializerMixin,
             "notify_on_call_from_schedule",
             "group_to_notify",
             "action_to_trigger",
-            "manual_order",
             "notify_if_time_from",
             "notify_if_time_to",
             "num_alerts_in_window",
@@ -126,21 +123,7 @@ class EscalationPolicySerializer(EagerLoadingMixin, OrderedModelSerializerMixin,
 
     def create(self, validated_data):
         validated_data = self._correct_validated_data(validated_data)
-        manual_order = validated_data.pop("manual_order")
-        if not manual_order:
-            order = validated_data.pop("order", None)
-            escalation_chain_id = validated_data.get("escalation_chain")
-            # validate 'order' value before creation
-            self._validate_order(order, {"escalation_chain_id": escalation_chain_id})
-
-            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
+        return super().create(validated_data)
 
     def to_representation(self, instance):
         step = instance.step
@@ -211,18 +194,6 @@ 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",
@@ -310,14 +281,4 @@ class EscalationPolicyUpdateSerializer(EscalationPolicySerializer):
                     instance.num_alerts_in_window = None
                     instance.num_minutes_in_window = None
 
-        manual_order = validated_data.pop("manual_order")
-
-        if not manual_order:
-            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/serializers/personal_notification_rules.py

@@ -8,20 +8,18 @@ from apps.base.models.user_notification_policy import NotificationChannelPublicA
 from common.api_helpers.custom_fields import UserIdField
 from common.api_helpers.exceptions import BadRequest
 from common.api_helpers.mixins import EagerLoadingMixin
+from common.ordered_model.serializer import OrderedModelSerializer
 
 
-class PersonalNotificationRuleSerializer(EagerLoadingMixin, serializers.ModelSerializer):
+class PersonalNotificationRuleSerializer(EagerLoadingMixin, OrderedModelSerializer):
     id = serializers.CharField(read_only=True, source="public_primary_key")
     user_id = UserIdField(required=True, source="user")
-    position = serializers.IntegerField(required=False, source="order")
     type = serializers.CharField(
         required=False,
     )
-
     duration = serializers.ChoiceField(
         required=False, source="wait_delay", choices=UserNotificationPolicy.DURATION_CHOICES
     )
-    manual_order = serializers.BooleanField(default=False, write_only=True)
 
     SELECT_RELATED = ["user"]
 
@@ -31,7 +29,7 @@ class PersonalNotificationRuleSerializer(EagerLoadingMixin, serializers.ModelSer
 
     class Meta:
         model = UserNotificationPolicy
-        fields = ["id", "user_id", "position", "type", "duration", "manual_order", "important"]
+        fields = OrderedModelSerializer.Meta.fields + ["id", "user_id", "type", "duration", "important"]
 
     def create(self, validated_data):
         if "type" not in validated_data:
@@ -44,17 +42,7 @@ class PersonalNotificationRuleSerializer(EagerLoadingMixin, serializers.ModelSer
         if "wait_delay" in validated_data and validated_data["step"] != UserNotificationPolicy.Step.WAIT:
             raise exceptions.ValidationError({"duration": "Duration can't be set"})
 
-        # Remove "manual_order" and "order" fields from validated_data, so they are not passed to create method.
-        # Policies are always created at the end of the list, and then moved to the desired position by _adjust_order.
-        manual_order = validated_data.pop("manual_order")
-        order = validated_data.pop("order", None)
-
-        instance = UserNotificationPolicy.objects.create(**validated_data)
-
-        if order is not None:
-            self._adjust_order(instance, manual_order, order, created=True)
-
-        return instance
+        return super().create(validated_data)
 
     def to_internal_value(self, data):
         if "duration" in data:
@@ -119,33 +107,6 @@ class PersonalNotificationRuleSerializer(EagerLoadingMixin, serializers.ModelSer
 
         raise exceptions.ValidationError({"type": "Invalid type"})
 
-    @staticmethod
-    def _adjust_order(instance, manual_order, order, created):
-        # Passing order=-1 means that the policy should be moved to the end of the list.
-        if order == -1:
-            if created:
-                # The policy was just created, so it is already at the end of the list.
-                return
-
-            order = instance.max_order()
-            # max_order() can't be None here because at least one instance exists – the one we are moving.
-            assert order is not None
-
-        # Negative order is not allowed.
-        if order < 0:
-            raise BadRequest(detail="Invalid value for position field")
-
-        # manual_order=True is intended for use by Terraform provider only, and is not a documented feature.
-        # Orders are swapped instead of moved when using Terraform, because Terraform may issue concurrent requests
-        # to create / update / delete multiple policies. "Move to" operation is not deterministic in this case, and
-        # final order of policies may be different depending on the order in which requests are processed. On the other
-        # hand, the result of concurrent "swap" operations is deterministic and does not depend on the order in
-        # which requests are processed.
-        if manual_order:
-            instance.swap(order)
-        else:
-            instance.to(order)
-
 
 class PersonalNotificationRuleUpdateSerializer(PersonalNotificationRuleSerializer):
     user_id = UserIdField(read_only=True, source="user")
@@ -165,10 +126,4 @@ class PersonalNotificationRuleUpdateSerializer(PersonalNotificationRuleSerialize
             if "wait_delay" in validated_data and instance.step != UserNotificationPolicy.Step.WAIT:
                 raise exceptions.ValidationError({"duration": "Duration can't be set"})
 
-        # Remove "manual_order" and "order" fields from validated_data, so they are not passed to update method.
-        manual_order = validated_data.pop("manual_order")
-        order = validated_data.pop("order", None)
-        if order is not None:
-            self._adjust_order(instance, manual_order, order, created=False)
-
         return super().update(instance, validated_data)

engine/apps/public_api/serializers/routes.py

@@ -6,12 +6,12 @@ from apps.api.serializers.alert_receive_channel import valid_jinja_template_for_
 from apps.base.messaging import get_messaging_backend_from_id, get_messaging_backends
 from common.api_helpers.custom_fields import OrganizationFilteredPrimaryKeyRelatedField
 from common.api_helpers.exceptions import BadRequest
-from common.api_helpers.mixins import OrderedModelSerializerMixin
 from common.jinja_templater.apply_jinja_template import JinjaTemplateError
+from common.ordered_model.serializer import OrderedModelSerializer
 from common.utils import is_regex_valid
 
 
-class BaseChannelFilterSerializer(OrderedModelSerializerMixin, serializers.ModelSerializer):
+class BaseChannelFilterSerializer(OrderedModelSerializer):
     """Base Channel Filter serializer with validation methods"""
 
     def __init__(self, *args, **kwargs):
@@ -148,7 +148,6 @@ class ChannelFilterSerializer(BaseChannelFilterSerializer):
     telegram = serializers.DictField(required=False)
     routing_type = RoutingTypeField(allow_null=False, required=False, source="filtering_term_type")
     routing_regex = serializers.CharField(allow_null=False, required=True, source="filtering_term")
-    position = serializers.IntegerField(required=False, source="order")
     integration_id = OrganizationFilteredPrimaryKeyRelatedField(
         queryset=AlertReceiveChannel.objects, source="alert_receive_channel"
     )
@@ -159,39 +158,24 @@ class ChannelFilterSerializer(BaseChannelFilterSerializer):
     )
 
     is_the_last_route = serializers.BooleanField(read_only=True, source="is_default")
-    manual_order = serializers.BooleanField(default=False, write_only=True)
 
     class Meta:
         model = ChannelFilter
-        fields = [
+        fields = OrderedModelSerializer.Meta.fields + [
             "id",
             "integration_id",
             "escalation_chain_id",
             "routing_type",
             "routing_regex",
-            "position",
             "is_the_last_route",
             "slack",
             "telegram",
-            "manual_order",
         ]
         read_only_fields = ["is_the_last_route"]
 
     def create(self, validated_data):
         validated_data = self._correct_validated_data(validated_data)
-        manual_order = validated_data.pop("manual_order")
-        if manual_order:
-            self._validate_manual_order(validated_data.get("order", None))
-            instance = super().create(validated_data)
-        else:
-            order = validated_data.pop("order", None)
-            alert_receive_channel_id = validated_data.get("alert_receive_channel")
-            # validate 'order' value before creation
-            self._validate_order(order, {"alert_receive_channel_id": alert_receive_channel_id, "is_default": False})
-            instance = super().create(validated_data)
-            self._change_position(order, instance)
-
-        return instance
+        return super().create(validated_data)
 
     def validate(self, data):
         filtering_term = data.get("routing_regex")
@@ -224,18 +208,6 @@ class ChannelFilterUpdateSerializer(ChannelFilterSerializer):
 
     def update(self, instance, validated_data):
         validated_data = self._correct_validated_data(validated_data)
-
-        manual_order = validated_data.pop("manual_order")
-        if manual_order:
-            self._validate_manual_order(validated_data.get("order", None))
-        else:
-            order = validated_data.pop("order", None)
-            self._validate_order(
-                order,
-                {"alert_receive_channel_id": instance.alert_receive_channel_id, "is_default": instance.is_default},
-            )
-            self._change_position(order, instance)
-
         if validated_data.get("notification_backends"):
             validated_data["notification_backends"] = self._update_notification_backends(
                 validated_data["notification_backends"]

engine/apps/public_api/tests/test_escalation_policies.py

@@ -154,7 +154,7 @@ def test_create_escalation_policy_manual_order_duplicated_position(
     escalation_policies_setup,
 ):
     organization, user, token = make_organization_and_user_with_token()
-    escalation_chain, _, _ = escalation_policies_setup(organization, user)
+    escalation_chain, escalation_policies, _ = escalation_policies_setup(organization, user)
 
     data_for_create = {
         "escalation_chain_id": escalation_chain.public_primary_key,
@@ -168,7 +168,43 @@ def test_create_escalation_policy_manual_order_duplicated_position(
     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
+    assert response.status_code == status.HTTP_201_CREATED
+    assert response.data["position"] == 0
+
+    for escalation_policy in escalation_policies:
+        escalation_policy.refresh_from_db()
+
+    orders = [escalation_policy.order for escalation_policy in escalation_policies]
+    assert orders == [3, 1, 2]  # Check orders are swapped when manual_order is True
+
+
+@pytest.mark.django_db
+def test_create_escalation_policy_no_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, _ = 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],
+    }
+
+    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_201_CREATED
+    assert response.data["position"] == 0
+
+    for escalation_policy in escalation_policies:
+        escalation_policy.refresh_from_db()
+
+    orders = [escalation_policy.order for escalation_policy in escalation_policies]
+    assert orders == [1, 2, 3]  # Check policies are moved down manual_order is False
 
 
 @pytest.mark.django_db
@@ -265,4 +301,10 @@ def test_update_escalation_policy_manual_order_duplicated_position(
     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
+    assert response.status_code == status.HTTP_200_OK
+
+    for escalation_policy in escalation_policies:
+        escalation_policy.refresh_from_db()
+
+    orders = [escalation_policy.order for escalation_policy in escalation_policies]
+    assert orders == [1, 0, 2]  # Check orders are swapped when manual_order is True

engine/apps/public_api/tests/test_routes.py

@@ -444,7 +444,7 @@ def test_update_route_with_manual_ordering(
 
     url = reverse("api-public:routes-detail", kwargs={"pk": channel_filter.public_primary_key})
 
-    # Test negative value. Note, that for "manual_order"=False, -1 is valud option (It will move route to the bottom)
+    # Test negative value. Note, that for "manual_order"=False, -1 is valid option (It will move route to the bottom)
     data_to_update = {"position": -1, "manual_order": True}
 
     response = client.put(url, format="json", HTTP_AUTHORIZATION=token, data=data_to_update)

engine/common/api_helpers/mixins.py

@@ -141,38 +141,6 @@ class RateLimitHeadersMixin:
         return super().handle_exception(exc)
 
 
-class OrderedModelSerializerMixin:
-    def _change_position(self, order, instance):
-        if order is not None:
-            if order >= 0:
-                instance.to(order)
-            elif order == -1:
-                instance.bottom()
-            else:
-                raise BadRequest(detail="Invalid value for position field")
-
-    def _validate_order(self, order, filter_kwargs):
-        if order is not None and (self.instance is None or self.instance.order != order):
-            last_instance = self.Meta.model.objects.filter(**filter_kwargs).order_by("order").last()
-            max_order = last_instance.order if last_instance else -1
-            if self.instance is None:
-                max_order += 1
-            if order > max_order:
-                raise BadRequest(detail="Invalid value for position field")
-
-    def _validate_manual_order(self, order):
-        """
-        For manual ordering validate just that order is valid PositiveIntegrer.
-        User of manual ordering is responsible for correct ordering.
-        However, manual ordering not intended for use somewhere, except terraform provider.
-        """
-
-        # https://docs.djangoproject.com/en/4.1/ref/models/fields/#positiveintegerfield
-        MAX_POSITIVE_INTEGER = 2147483647
-        if order is not None and order < 0 or order > MAX_POSITIVE_INTEGER:
-            raise BadRequest(detail="Invalid value for position field")
-
-
 class PublicPrimaryKeyMixin:
     def get_object(self):
         pk = self.kwargs["pk"]

engine/common/ordered_model/serializer.py

@@ -0,0 +1,69 @@
+from rest_framework import serializers
+
+from common.api_helpers.exceptions import BadRequest
+
+
+class OrderedModelSerializer(serializers.ModelSerializer):
+    """Ordered model serializer to be used in public API."""
+
+    position = serializers.IntegerField(required=False, source="order")
+    # manual_order=True is intended for use by Terraform provider only, and is not a documented feature.
+    manual_order = serializers.BooleanField(default=False, write_only=True)
+
+    class Meta:
+        fields = ["position", "manual_order"]
+
+    def create(self, validated_data):
+        # Remove "manual_order" and "order" fields from validated_data, so they are not passed to create method.
+        manual_order = validated_data.pop("manual_order")
+        order = validated_data.pop("order", None)
+
+        # Create the instance.
+        # Instances are always created at the end of the list, and then moved to the desired position by _adjust_order.
+        instance = super().create(validated_data)
+
+        # Adjust order of the instance if necessary.
+        if order is not None:
+            self._adjust_order(instance, manual_order, order, created=True)
+
+        return instance
+
+    def update(self, instance, validated_data):
+        # Remove "manual_order" and "order" fields from validated_data, so they are not passed to update method.
+        manual_order = validated_data.pop("manual_order")
+        order = validated_data.pop("order", None)
+
+        # Adjust order of the instance if necessary.
+        if order is not None:
+            self._adjust_order(instance, manual_order, order, created=False)
+
+        # Proceed with the update.
+        return super().update(instance, validated_data)
+
+    @staticmethod
+    def _adjust_order(instance, manual_order, order, created):
+        # Passing order=-1 means that the policy should be moved to the end of the list.
+        # Works only for public API but not for Terraform provider.
+        if order == -1 and not manual_order:
+            if created:
+                # The policy was just created, so it is already at the end of the list.
+                return
+
+            order = instance.max_order()
+            # max_order() can't be None here because at least one instance exists – the one we are moving.
+            assert order is not None
+
+        # Check the order is in the valid range.
+        # https://docs.djangoproject.com/en/4.1/ref/models/fields/#positiveintegerfield
+        if order < 0 or order > 2147483647:
+            raise BadRequest(detail="Invalid value for position field")
+
+        # Orders are swapped instead of moved when using Terraform, because Terraform may issue concurrent requests
+        # to create / update / delete multiple policies. "Move to" operation is not deterministic in this case, and
+        # final order of policies may be different depending on the order in which requests are processed. On the other
+        # hand, the result of concurrent "swap" operations is deterministic and does not depend on the order in
+        # which requests are processed.
+        if manual_order:
+            instance.swap(order)
+        else:
+            instance.to(order)

engine/common/ordered_model/viewset.py

@@ -0,0 +1,56 @@
+from rest_framework import serializers, status
+from rest_framework.decorators import action
+from rest_framework.request import Request
+from rest_framework.response import Response
+from rest_framework.viewsets import ModelViewSet
+
+from common.api_helpers.exceptions import BadRequest
+from common.insight_log import EntityEvent, write_resource_insight_log
+
+
+class OrderedModelViewSet(ModelViewSet):
+    """Ordered model viewset to be used in internal API."""
+
+    @action(detail=True, methods=["put"])
+    def move_to_position(self, request: Request, pk: int) -> Response:
+        instance = self.get_object()
+        position = self._get_move_to_position_param(request)
+
+        prev_state = self._get_insight_logs_serialized(instance)
+        try:
+            instance.to_index(position)
+        except IndexError:
+            raise BadRequest(detail="Invalid position")
+        new_state = self._get_insight_logs_serialized(instance)
+
+        write_resource_insight_log(
+            instance=instance,
+            author=self.request.user,
+            event=EntityEvent.UPDATED,
+            prev_state=prev_state,
+            new_state=new_state,
+        )
+
+        return Response(status=status.HTTP_200_OK)
+
+    @staticmethod
+    def _get_insight_logs_serialized(instance):
+        try:
+            return instance.insight_logs_serialized
+        except AttributeError:
+            return instance.user.insight_logs_serialized  # workaround for UserNotificationPolicy
+
+    @staticmethod
+    def _get_move_to_position_param(request: Request) -> int:
+        """
+        Get "position" parameter from query params + validate it.
+        Used by actions on ordered models (e.g. move_to_position).
+        """
+
+        class MoveToPositionQueryParamsSerializer(serializers.Serializer):
+            position = serializers.IntegerField()
+
+        serializer = MoveToPositionQueryParamsSerializer(data=request.query_params)
+        serializer.is_valid(raise_exception=True)
+
+        return serializer.validated_data["position"]

engine/common/tests/test_ordered_model.py

@@ -4,7 +4,7 @@ import threading
 import pytest
 from django.db import models
 
-from apps.base.models.ordered_model import OrderedModel
+from common.ordered_model.ordered_model import OrderedModel
 
 
 class TestOrderedModel(OrderedModel):

engine/requirements.txt

@@ -4,6 +4,7 @@ slackclient==1.3.0
 whitenoise==5.3.0
 twilio~=6.37.0
 phonenumbers==8.10.0
+# TODO: remove django-ordered-model after migration to custom OrderModel
 django-ordered-model==3.1.1
 celery[amqp,redis]==5.3.1
 redis==4.6.0

grafana-plugin/src/containers/ChannelFilterForm/ChannelFilterForm.tsx

@@ -64,7 +64,6 @@ const ChannelFilterForm = observer((props: ChannelFilterFormProps) => {
   const onUpdateClickCallback = useCallback(() => {
     (id === 'new'
       ? alertReceiveChannelStore.createChannelFilter({
-          order: 0,
           alert_receive_channel: alertReceiveChannelId,
           filtering_term: filteringTerm,
           filtering_term_type: filteringTermType,

grafana-plugin/src/models/channel_filter.ts

@@ -4,7 +4,6 @@ import { TelegramChannel } from 'models/telegram_channel/telegram_channel.types'
 
 export interface ChannelFilter {
   id: string;
-  order: number;
   alert_receive_channel: AlertReceiveChannel['id'];
   slack_channel_id?: SlackChannel['id'];
   telegram_channel?: TelegramChannel['id'];

grafana-plugin/src/models/channel_filter/channel_filter.types.ts

@@ -10,7 +10,6 @@ export enum FilteringTermType {
 
 export interface ChannelFilter {
   id: string;
-  order: number;
   alert_receive_channel: AlertReceiveChannel['id'];
   slack_channel_id?: SlackChannel['id'];
   slack_channel?: SlackChannel;

grafana-plugin/src/models/escalation_policy.ts

@@ -10,8 +10,7 @@ import { UserDTO as User } from './user';
 export interface EscalationPolicyType {
   id: string;
   notify_to_user: User['pk'] | null;
-  order: number;
-  //  is't option value from api/internal/v1/escalation_policies/escalation_options/
+  //  it's option value from api/internal/v1/escalation_policies/escalation_options/
   step: number;
   wait_delay: string | null;
   is_final: boolean;

grafana-plugin/src/models/escalation_policy/escalation_policy.types.ts

@@ -8,8 +8,7 @@ import { UserGroup } from 'models/user_group/user_group.types';
 export interface EscalationPolicy {
   id: string;
   notify_to_user: User['pk'] | null;
-  order: number;
-  //  is't option value from api/internal/v1/escalation_policies/escalation_options/
+  //  it's option value from api/internal/v1/escalation_policies/escalation_options/
   step: EscalationPolicyOption['value'];
   wait_delay: string | null;
   is_final: boolean;

grafana-plugin/src/models/notification_policy.ts

@@ -2,7 +2,6 @@ import { UserDTO as User } from './user';
 
 export interface NotificationPolicyType {
   id: string;
-  order: number;
   step: number;
   notify_by: User['pk'] | null;
   wait_delay: string | null;

grafana-plugin/src/pages/integration/Integration.tsx

@@ -406,7 +406,6 @@ class Integration extends React.Component<IntegrationProps, IntegrationState> {
       () => {
         alertReceiveChannelStore
           .createChannelFilter({
-            order: 0,
             alert_receive_channel: id,
             filtering_term: NEW_ROUTE_DEFAULT,
             filtering_term_type: 1, // non-regex