fix(versioning): fix issue creating duplicate priority segment overri…

…des (#4603)

api/features/feature_segments/serializers.py

@@ -63,24 +63,21 @@ def save(self, **kwargs) -> FeatureSegment:
                 kwargs["environment_feature_version"] is not None
             ), "Must provide environment_feature_version for environment using v2 versioning"
 
-        if (
-            priority is not None
-            and (
-                collision := FeatureSegment.objects.filter(
-                    environment=kwargs["environment"],
-                    feature=kwargs["feature"],
-                    environment_feature_version=kwargs.get(
-                        "environment_feature_version"
-                    ),
-                    priority=priority,
-                ).first()
+        if priority is not None:
+            collision_qs = FeatureSegment.objects.filter(
+                environment=kwargs["environment"],
+                feature=kwargs["feature"],
+                environment_feature_version=kwargs.get("environment_feature_version"),
+                priority=priority,
             )
-            is not None
-        ):
-            # Since there is no unique clause on the priority field, if a priority
-            # is set, it will just save the feature segment and not move others
-            # down. This ensures that the incoming priority space is 'free'.
-            collision.to(priority + 1)
+            if self.instance is not None:
+                collision_qs = collision_qs.exclude(id=self.instance.id)
+            collision = collision_qs.first()
+            if collision:
+                # Since there is no unique clause on the priority field, if a priority
+                # is set, it will just save the feature segment and not move others
+                # down. This ensures that the incoming priority space is 'free'.
+                collision.to(priority + 1)
 
         return super().save(**kwargs)
 

api/tests/unit/features/versioning/test_unit_versioning_views.py

@@ -1124,3 +1124,77 @@ def test_create_new_version_delete_segment_override_updates_overrides_immediatel
     get_feature_segments_response = admin_client.get(get_feature_segments_url)
     assert get_feature_segments_response.status_code == status.HTTP_200_OK
     assert get_feature_segments_response.json()["count"] == 0
+
+
+def test_creating_multiple_segment_overrides_in_multiple_versions_sets_correct_priorities(
+    feature: Feature,
+    environment_v2_versioning: Environment,
+    segment: Segment,
+    another_segment: Segment,
+    admin_client_new: APIClient,
+) -> None:
+    """
+    This test is for a specific case where
+    """
+
+    def generate_segment_override_fs_payload(
+        segment: Segment, priority: int
+    ) -> dict[str, typing.Any]:
+        return {
+            "enabled": True,
+            "feature_state_value": {
+                "string_value": f"override for segment {segment.name}",
+                "type": "unicode",
+            },
+            "feature_segment": {
+                "segment": segment.id,
+                "priority": priority,
+            },
+        }
+
+    url = reverse(
+        "api-v1:versioning:environment-feature-versions-list",
+        args=[environment_v2_versioning.id, feature.id],
+    )
+
+    # Now, let's create the first override with priority 0
+    data = {
+        "publish_immediately": True,
+        "feature_states_to_create": [generate_segment_override_fs_payload(segment, 0)],
+    }
+    create_segment_override_1_response = admin_client_new.post(
+        url, data=json.dumps(data), content_type="application/json"
+    )
+    assert create_segment_override_1_response.status_code == status.HTTP_201_CREATED
+    version_1_uuid = create_segment_override_1_response.json()["uuid"]
+    assert (
+        FeatureSegment.objects.get(
+            environment_feature_version__uuid=version_1_uuid,
+            feature=feature,
+            segment=segment,
+        ).priority
+        == 0
+    )
+
+    # Now, let's create the second override with priority 1 and (to mimic the FE behaviour)
+    # update the existing segment override (without actually changing anything).
+    data = {
+        "publish_immediately": True,
+        "feature_states_to_create": [
+            generate_segment_override_fs_payload(another_segment, 1)
+        ],
+        "feature_states_to_update": [generate_segment_override_fs_payload(segment, 0)],
+    }
+    create_segment_override_2_response = admin_client_new.post(
+        url, data=json.dumps(data), content_type="application/json"
+    )
+    assert create_segment_override_2_response.status_code == status.HTTP_201_CREATED
+    version_2_uuid = create_segment_override_2_response.json()["uuid"]
+    assert (
+        FeatureSegment.objects.get(
+            environment_feature_version__uuid=version_2_uuid,
+            feature=feature,
+            segment=another_segment,
+        ).priority
+        == 1
+    )