feat(__future__): Actionable error message for duplicate priorities in update-flag

api/features/future/exceptions.py

@@ -1,8 +1,13 @@
 """https://docs.flagsmith.com/managing-flags/updating-flags"""
 
+from collections.abc import Sequence
+
+from django.utils.text import get_text_list
 from rest_framework import status
 from rest_framework.exceptions import APIException
 
+from segments.models import Segment
+
 
 class ChangeRequestsEnabledError(APIException):
     """Raised where a flag can only be changed by going through a change request."""
@@ -22,4 +27,13 @@ class DuplicatePriorityError(APIException):
     """Raised where a flag's segment overrides would end up sharing a priority."""
 
     status_code = status.HTTP_400_BAD_REQUEST
-    default_detail = "Segment overrides must not share a priority."
+
+    def __init__(self, segments: Sequence[Segment]) -> None:
+        conflicted = get_text_list(
+            [f"{segment.id} ({segment.name})" for segment in segments],
+            "and",
+        )
+        super().__init__(
+            f"The overrides for segments {conflicted} are in conflict; "
+            "provide explicit priority values."
+        )

api/features/future/services.py

@@ -1,11 +1,13 @@
 """https://docs.flagsmith.com/managing-flags/updating-flags"""
 
 from collections.abc import Collection, Sequence
+from itertools import groupby
+from operator import attrgetter
 from typing import NamedTuple
 
 import structlog
 from django.db import transaction
-from django.db.models import Count, Q
+from django.db.models import Q
 
 from api_keys.user import APIKeyUser
 from environments.models import Environment
@@ -193,21 +195,19 @@ def _check_priorities(
     version: EnvironmentFeatureVersion | None,
 ) -> None:
     """Precedence between two segment overrides sharing a priority is undefined."""
-    duplicate = (
+    feature_segments = (
         FeatureSegment.objects.filter(
             environment=environment,
             feature=feature,
             environment_feature_version=version,
         )
-        .values("priority")
-        .annotate(count=Count("priority"))
-        .filter(count__gt=1)
-        .order_by("priority")
-        .values_list("priority", flat=True)
-        .first()
+        .select_related("segment")
+        .order_by("priority", "segment_id")
     )
-    if duplicate is not None:
-        raise DuplicatePriorityError(f"Duplicate priority: {duplicate}.")
+    for _priority, sharing in groupby(feature_segments, attrgetter("priority")):
+        segments = [feature_segment.segment for feature_segment in sharing]
+        if len(segments) > 1:
+            raise DuplicatePriorityError(segments)
 
 
 class WrittenSegmentOverrides(NamedTuple):

api/tests/integration/features/future/test_flag_endpoint.py

@@ -816,7 +816,13 @@ def test_update_flag__patch_segment_override_priority_in_use__responds_400(
 
     # Then
     assert response.status_code == 400
-    assert response.json() == {"detail": "Duplicate priority: 1."}
+    assert response.json() == {
+        "detail": (
+            f"The overrides for segments {segment} (Test Segment) and "
+            f"{segment_2} (Test Segment 2) are in conflict; "
+            "provide explicit priority values."
+        ),
+    }
     assert dict(
         FeatureState.objects.get_live_feature_states(
             environment=versioned_environment,
@@ -859,7 +865,13 @@ def test_update_flag__patch_second_segment_override_without_priority__responds_4
 
     # Then
     assert response.status_code == 400
-    assert response.json() == {"detail": "Duplicate priority: 0."}
+    assert response.json() == {
+        "detail": (
+            f"The overrides for segments {segment} (Test Segment) and "
+            f"{segment_2} (Test Segment 2) are in conflict; "
+            "provide explicit priority values."
+        ),
+    }
     assert dict(
         FeatureState.objects.get_live_feature_states(
             environment=versioned_environment,