Skip to content

Commit 8d43e11

Browse files
committed
fix: use .to() for priority reordering when creating new segment overrides
Creating a FeatureSegment with a priority value doesn't trigger reordering of existing segments. Use .to() after creation to properly reorder. Fixes both V1 and V2 endpoints.
1 parent 39c4b44 commit 8d43e11

File tree

2 files changed

+134
-7
lines changed

2 files changed

+134
-7
lines changed

api/features/versioning/versioning_service.py

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,11 +137,11 @@ def _update_flag_for_versioning_v2(
137137
feature=feature,
138138
segment_id=change_set.segment_id,
139139
environment=environment,
140-
priority=change_set.segment_priority
141-
if change_set.segment_priority is not None
142-
else 0,
143140
)
144141

142+
if change_set.segment_priority is not None:
143+
feature_segment.to(change_set.segment_priority)
144+
145145
target_feature_state = FeatureState.objects.create(
146146
feature=feature,
147147
environment=environment,
@@ -198,11 +198,11 @@ def _update_flag_for_versioning_v1(
198198
feature=feature,
199199
segment_id=change_set.segment_id,
200200
environment=environment,
201-
priority=change_set.segment_priority
202-
if change_set.segment_priority is not None
203-
else 0,
204201
)
205202

203+
if change_set.segment_priority is not None:
204+
feature_segment.to(change_set.segment_priority)
205+
206206
target_feature_state: FeatureState = FeatureState.objects.create(
207207
feature=feature,
208208
environment=environment,
@@ -257,9 +257,11 @@ def _create_segment_override(
257257
feature=feature,
258258
segment_id=segment_id,
259259
environment=environment,
260-
priority=priority if priority is not None else 0,
261260
)
262261

262+
if priority is not None:
263+
feature_segment.to(priority)
264+
263265
segment_state: FeatureState = FeatureState.objects.create(
264266
feature=feature,
265267
environment=environment,

api/tests/unit/features/feature_states/test_unit_feature_states_views.py

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,65 @@ def test_update_flag_segment_override_creates_feature_segment_if_not_exists(
391391
][0]
392392
assert segment_override.enabled is True
393393
assert segment_override.get_feature_state_value() == "premium_feature"
394+
assert segment_override.feature_segment is not None
395+
assert segment_override.feature_segment.priority == 10
396+
397+
398+
def test_create_new_segment_override_reorders_priorities_v1(
399+
staff_client: APIClient,
400+
feature: Feature,
401+
environment: Environment,
402+
project: Project,
403+
with_environment_permissions: WithEnvironmentPermissionsCallable,
404+
) -> None:
405+
# Given
406+
with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg]
407+
408+
segment1 = Segment.objects.create(name="segment1", project=project)
409+
segment2 = Segment.objects.create(name="segment2", project=project)
410+
411+
# Create existing segment override at priority 0
412+
feature_segment1 = FeatureSegment.objects.create(
413+
feature=feature,
414+
segment=segment1,
415+
environment=environment,
416+
priority=0,
417+
)
418+
FeatureState.objects.create(
419+
feature=feature,
420+
environment=environment,
421+
feature_segment=feature_segment1,
422+
enabled=True,
423+
)
424+
425+
url = reverse(
426+
"api-experiments:update-flag-v1",
427+
kwargs={"environment_key": environment.api_key},
428+
)
429+
430+
# When - Create new segment override at priority 0 (should push segment1 to priority 1)
431+
data = {
432+
"feature": {"name": feature.name},
433+
"segment": {"id": segment2.id, "priority": 0},
434+
"enabled": True,
435+
"value": {"type": "string", "string_value": "new_segment_value"},
436+
}
437+
response = staff_client.post(
438+
url, data=json.dumps(data), content_type="application/json"
439+
)
440+
441+
# Then
442+
assert response.status_code == status.HTTP_204_NO_CONTENT
443+
444+
# Refresh from DB
445+
feature_segment1.refresh_from_db()
446+
feature_segment2 = FeatureSegment.objects.get(
447+
feature=feature, segment=segment2, environment=environment
448+
)
449+
450+
# segment2 should be at priority 0, segment1 should have been pushed to priority 1
451+
assert feature_segment2.priority == 0
452+
assert feature_segment1.priority == 1
394453

395454

396455
# Update Multiple Feature States Tests
@@ -709,6 +768,72 @@ def test_update_existing_segment_override_with_priority_v2(
709768
assert feature_segment.priority == 2
710769

711770

771+
def test_create_new_segment_override_reorders_priorities_v2(
772+
staff_client: APIClient,
773+
feature: Feature,
774+
environment: Environment,
775+
project: Project,
776+
with_environment_permissions: WithEnvironmentPermissionsCallable,
777+
) -> None:
778+
# Given
779+
with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg]
780+
781+
segment1 = Segment.objects.create(name="segment1_v2", project=project)
782+
segment2 = Segment.objects.create(name="segment2_v2", project=project)
783+
784+
# Create existing segment override at priority 0
785+
feature_segment1 = FeatureSegment.objects.create(
786+
feature=feature,
787+
segment=segment1,
788+
environment=environment,
789+
priority=0,
790+
)
791+
FeatureState.objects.create(
792+
feature=feature,
793+
environment=environment,
794+
feature_segment=feature_segment1,
795+
enabled=True,
796+
)
797+
798+
url = reverse(
799+
"api-experiments:update-flag-v2",
800+
kwargs={"environment_key": environment.api_key},
801+
)
802+
803+
# When - Create new segment override at priority 0 (should push segment1 to priority 1)
804+
data = {
805+
"feature": {"name": feature.name},
806+
"environment_default": {
807+
"enabled": True,
808+
"value": {"type": "string", "string_value": "default"},
809+
},
810+
"segment_overrides": [
811+
{
812+
"segment_id": segment2.id,
813+
"priority": 0,
814+
"enabled": True,
815+
"value": {"type": "string", "string_value": "new_segment_value"},
816+
},
817+
],
818+
}
819+
response = staff_client.post(
820+
url, data=json.dumps(data), content_type="application/json"
821+
)
822+
823+
# Then
824+
assert response.status_code == status.HTTP_204_NO_CONTENT
825+
826+
# Refresh from DB
827+
feature_segment1.refresh_from_db()
828+
feature_segment2 = FeatureSegment.objects.get(
829+
feature=feature, segment=segment2, environment=environment
830+
)
831+
832+
# segment2 should be at priority 0, segment1 should have been pushed to priority 1
833+
assert feature_segment2.priority == 0
834+
assert feature_segment1.priority == 1
835+
836+
712837
def test_update_flag_v1_returns_403_when_workflow_enabled(
713838
staff_client: APIClient,
714839
feature: Feature,

0 commit comments

Comments
 (0)