From c60cf8283fe080000526f3d61d5079016876dfd6 Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Fri, 14 Aug 2026 19:15:14 -0300 Subject: [PATCH] get priorities straight v3 --- api/features/future/exceptions.py | 16 ++++++++++++++- api/features/future/services.py | 20 +++++++++---------- .../features/future/test_flag_endpoint.py | 16 +++++++++++++-- 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/api/features/future/exceptions.py b/api/features/future/exceptions.py index 43717e2fcf53..0aaa733f060c 100644 --- a/api/features/future/exceptions.py +++ b/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." + ) diff --git a/api/features/future/services.py b/api/features/future/services.py index 30bb62eaa6c2..5fe163008530 100644 --- a/api/features/future/services.py +++ b/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): diff --git a/api/tests/integration/features/future/test_flag_endpoint.py b/api/tests/integration/features/future/test_flag_endpoint.py index bb4cfee59408..1be3bee3f931 100644 --- a/api/tests/integration/features/future/test_flag_endpoint.py +++ b/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,