From 6dde16c636c3668f684e8c34cf6dd9e2e0470755 Mon Sep 17 00:00:00 2001 From: Gagan Trivedi Date: Wed, 3 Dec 2025 15:47:58 +0530 Subject: [PATCH 1/2] feat: add delete-segment-override experimental endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements issue #6234 - adds endpoint to delete segment overrides for a feature in a given environment. Endpoint: POST /api/experiments/environments/{environment_key}/delete-segment-override/ Request body: - feature: {name: "x"} OR {id: 123} - segment: {id: 456} Response: 204 No Content Features: - Works with both V1 and V2 feature versioning - V1: Direct deletion of FeatureSegment - V2: Creates new version, removes segment override, publishes - Reuses UPDATE_FEATURE_STATE permission - Blocked when workflow/change requests are enabled - Returns 400 if segment override doesn't exist Files changed: - versioning_service.py: Add delete_segment_override() with V1/V2 dispatch - serializers.py: Add DeleteSegmentOverrideSerializer - views.py: Add delete_segment_override view - experiments.py: Add URL route - tests: Add 9 test cases covering all scenarios 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- api/api/urls/experiments.py | 11 +- api/features/feature_states/serializers.py | 31 +- api/features/feature_states/views.py | 55 ++- api/features/versioning/versioning_service.py | 63 ++- .../test_unit_feature_states_views.py | 364 ++++++++++++++++++ 5 files changed, 520 insertions(+), 4 deletions(-) diff --git a/api/api/urls/experiments.py b/api/api/urls/experiments.py index 2fbb500ee4f0..984709354663 100644 --- a/api/api/urls/experiments.py +++ b/api/api/urls/experiments.py @@ -7,7 +7,11 @@ from django.urls import path -from features.feature_states.views import update_flag_v1, update_flag_v2 +from features.feature_states.views import ( + delete_segment_override, + update_flag_v1, + update_flag_v2, +) app_name = "experiments" @@ -22,4 +26,9 @@ update_flag_v2, name="update-flag-v2", ), + path( + "environments//delete-segment-override/", + delete_segment_override, + name="delete-segment-override", + ), ] diff --git a/api/features/feature_states/serializers.py b/api/features/feature_states/serializers.py index cbf6f4567587..74b938ff15a6 100644 --- a/api/features/feature_states/serializers.py +++ b/api/features/feature_states/serializers.py @@ -8,8 +8,13 @@ FlagChangeSetV2, SegmentOverrideChangeSet, ) -from features.versioning.versioning_service import update_flag, update_flag_v2 +from features.versioning.versioning_service import ( + delete_segment_override, + update_flag, + update_flag_v2, +) from segments.models import Segment +from users.models import FFAdminUser class BaseFeatureUpdateSerializer(serializers.Serializer): # type: ignore[type-arg] @@ -188,3 +193,27 @@ def change_set_v2(self) -> FlagChangeSetV2: def save(self, **kwargs: object) -> None: feature = self.get_feature() update_flag_v2(self.environment, feature, self.change_set_v2) + + +class SegmentIdentifierSerializer(serializers.Serializer): # type: ignore[type-arg] + id = serializers.IntegerField(required=True) + + +class DeleteSegmentOverrideSerializer(BaseFeatureUpdateSerializer): + feature = FeatureIdentifierSerializer(required=True) + segment = SegmentIdentifierSerializer(required=True) + + def validate_segment(self, value: dict) -> dict: # type: ignore[type-arg] + if value and value.get("id"): + self.validate_segment_id(value["id"]) + return value + + def save(self, **kwargs: object) -> None: + feature = self.get_feature() + segment_id = self.validated_data["segment"]["id"] + + request = self.context["request"] + user = request.user if isinstance(request.user, FFAdminUser) else None + api_key = getattr(request.user, "key", None) if user is None else None + + delete_segment_override(self.environment, feature, segment_id, user, api_key) diff --git a/api/features/feature_states/views.py b/api/features/feature_states/views.py index d992e42a7072..04a608c2b04f 100644 --- a/api/features/feature_states/views.py +++ b/api/features/feature_states/views.py @@ -10,7 +10,11 @@ from environments.models import Environment from .permissions import EnvironmentUpdateFeatureStatePermission -from .serializers import UpdateFlagSerializer, UpdateFlagV2Serializer +from .serializers import ( + DeleteSegmentOverrideSerializer, + UpdateFlagSerializer, + UpdateFlagV2Serializer, +) def _check_workflow_not_enabled(environment: Environment) -> None: @@ -145,3 +149,52 @@ def update_flag_v2(request: Request, environment_key: str) -> Response: serializer.save() return Response(status=status.HTTP_204_NO_CONTENT) + + +@swagger_auto_schema( + method="post", + operation_summary="Delete segment override", + operation_description=""" + **EXPERIMENTAL ENDPOINT** - Subject to change without notice. + + Deletes a segment override for a feature in the given environment. + + **Feature Identification:** + - Use `feature.name` OR `feature.id` (mutually exclusive) + - Feature must belong to the environment's project + + **Segment Identification:** + - Use `segment.id` (required) + + The segment override must exist for the given feature/environment combination. + Returns 400 if the segment override does not exist. + """, + manual_parameters=[ + openapi.Parameter( + "environment_key", + openapi.IN_PATH, + description="The environment API key", + type=openapi.TYPE_STRING, + required=True, + ) + ], + request_body=DeleteSegmentOverrideSerializer, + responses={ + 204: openapi.Response(description="Segment override deleted successfully") + }, + tags=["Experimental - Feature States"], +) # type: ignore[misc] +@api_view(http_method_names=["POST"]) +@permission_classes([IsAuthenticated, EnvironmentUpdateFeatureStatePermission]) +def delete_segment_override(request: Request, environment_key: str) -> Response: + environment = Environment.objects.get(api_key=environment_key) + _check_workflow_not_enabled(environment) + + serializer = DeleteSegmentOverrideSerializer( + data=request.data, + context={"request": request, "environment": environment}, + ) + serializer.is_valid(raise_exception=True) + serializer.save() + + return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/api/features/versioning/versioning_service.py b/api/features/versioning/versioning_service.py index 82093dc48df3..167264fabbe2 100644 --- a/api/features/versioning/versioning_service.py +++ b/api/features/versioning/versioning_service.py @@ -3,10 +3,11 @@ from common.core.utils import using_database_replica from django.db.models import Prefetch, Q, QuerySet from django.utils import timezone +from rest_framework.exceptions import NotFound from environments.models import Environment from features.feature_states.models import FeatureValueType -from features.models import Feature, FeatureState, FeatureStateValue +from features.models import Feature, FeatureSegment, FeatureState, FeatureStateValue from features.versioning.dataclasses import ( FlagChangeSet, FlagChangeSetV2, @@ -395,6 +396,66 @@ def _update_flag_v2_for_versioning_v1( _update_segment_priority(segment_state, override.priority) +def delete_segment_override( + environment: "Environment", + feature: "Feature", + segment_id: int, + user: "typing.Any" = None, + api_key: str | None = None, +) -> None: + if environment.use_v2_feature_versioning: + _delete_segment_override_v2(environment, feature, segment_id, user, api_key) + else: + _delete_segment_override_v1(environment, feature, segment_id) + + +def _delete_segment_override_v1( + environment: "Environment", + feature: "Feature", + segment_id: int, +) -> None: + deleted_count, _ = FeatureSegment.objects.filter( + feature=feature, + segment_id=segment_id, + environment=environment, + ).delete() + if deleted_count == 0: + raise NotFound(f"Segment override for segment {segment_id} does not exist") + + +def _delete_segment_override_v2( + environment: "Environment", + feature: "Feature", + segment_id: int, + user: "typing.Any", + api_key: str | None, +) -> None: + current_version = get_current_live_environment_feature_version( + environment.id, feature.id + ) + if ( + not current_version + or not current_version.feature_states.filter( + feature_segment__segment_id=segment_id + ).exists() + ): + raise NotFound(f"Segment override for segment {segment_id} does not exist") + + new_version = EnvironmentFeatureVersion.objects.create( + environment=environment, + feature=feature, + created_by=user, + created_by_api_key=api_key, + ) + + segment_feature_state = new_version.feature_states.get( + feature_segment__segment_id=segment_id + ) + segment_feature_state.feature_segment.delete() + + new_version.publish(published_by=user, published_by_api_key=api_key) + + def get_updated_feature_states_for_version( version: EnvironmentFeatureVersion, ) -> list[FeatureState]: diff --git a/api/tests/unit/features/feature_states/test_unit_feature_states_views.py b/api/tests/unit/features/feature_states/test_unit_feature_states_views.py index f290abdaed16..6bbeceb365e5 100644 --- a/api/tests/unit/features/feature_states/test_unit_feature_states_views.py +++ b/api/tests/unit/features/feature_states/test_unit_feature_states_views.py @@ -1050,3 +1050,367 @@ def test_update_flag_v1_returns_403_for_nonexistent_environment( # Then assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.parametrize( + "environment_", + (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), +) +def test_delete_segment_override_success( + staff_client: APIClient, + feature: Feature, + environment_: Environment, + project: Project, + with_environment_permissions: WithEnvironmentPermissionsCallable, +) -> None: + # Given + with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] + + segment = Segment.objects.create(name="segment_to_delete", project=project) + + version = None + if environment_.use_v2_feature_versioning: + version = get_current_live_environment_feature_version( + environment_.id, feature.id + ) + + feature_segment = FeatureSegment.objects.create( + feature=feature, + segment=segment, + environment=environment_, + priority=1, + ) + FeatureState.objects.create( + feature=feature, + environment=environment_, + feature_segment=feature_segment, + environment_feature_version=version, + enabled=True, + ) + + url = reverse( + "api-experiments:delete-segment-override", + kwargs={"environment_key": environment_.api_key}, + ) + + data = { + "feature": {"name": feature.name}, + "segment": {"id": segment.id}, + } + + # When + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_204_NO_CONTENT + + latest_flags = get_environment_flags_list( + environment=environment_, feature_name=feature.name + ) + segment_overrides = [ + fs + for fs in latest_flags + if fs.feature_segment and fs.feature_segment.segment_id == segment.id + ] + assert len(segment_overrides) == 0 + + +@pytest.mark.parametrize( + "environment_", + (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), +) +def test_delete_segment_override_by_feature_id( + staff_client: APIClient, + feature: Feature, + environment_: Environment, + project: Project, + with_environment_permissions: WithEnvironmentPermissionsCallable, +) -> None: + # Given + with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] + + segment = Segment.objects.create(name="segment_by_id", project=project) + + version = None + if environment_.use_v2_feature_versioning: + version = get_current_live_environment_feature_version( + environment_.id, feature.id + ) + + feature_segment = FeatureSegment.objects.create( + feature=feature, + segment=segment, + environment=environment_, + priority=1, + ) + FeatureState.objects.create( + feature=feature, + environment=environment_, + feature_segment=feature_segment, + environment_feature_version=version, + enabled=True, + ) + + url = reverse( + "api-experiments:delete-segment-override", + kwargs={"environment_key": environment_.api_key}, + ) + + data = { + "feature": {"id": feature.id}, + "segment": {"id": segment.id}, + } + + # When + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_204_NO_CONTENT + + +@pytest.mark.parametrize( + "environment_", + (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), +) +def test_delete_segment_override_feature_not_found( + staff_client: APIClient, + environment_: Environment, + project: Project, + with_environment_permissions: WithEnvironmentPermissionsCallable, +) -> None: + # Given + with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] + + segment = Segment.objects.create(name="some_segment", project=project) + + url = reverse( + "api-experiments:delete-segment-override", + kwargs={"environment_key": environment_.api_key}, + ) + + data = { + "feature": {"name": "nonexistent_feature"}, + "segment": {"id": segment.id}, + } + + # When + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "not found in project" in str(response.json()) + + +@pytest.mark.parametrize( + "environment_", + (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), +) +def test_delete_segment_override_segment_not_in_project( + staff_client: APIClient, + feature: Feature, + environment_: Environment, + with_environment_permissions: WithEnvironmentPermissionsCallable, +) -> None: + # Given + with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] + + url = reverse( + "api-experiments:delete-segment-override", + kwargs={"environment_key": environment_.api_key}, + ) + + # Segment doesn't exist in project + data = { + "feature": {"name": feature.name}, + "segment": {"id": 999999}, + } + + # When + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "not found in project" in str(response.json()) + + +@pytest.mark.parametrize( + "environment_", + (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), +) +def test_delete_segment_override_not_found( + staff_client: APIClient, + feature: Feature, + environment_: Environment, + project: Project, + with_environment_permissions: WithEnvironmentPermissionsCallable, +) -> None: + # Given + with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] + + # Segment exists in project but has no override + segment = Segment.objects.create(name="segment_no_override", project=project) + + url = reverse( + "api-experiments:delete-segment-override", + kwargs={"environment_key": environment_.api_key}, + ) + + data = { + "feature": {"name": feature.name}, + "segment": {"id": segment.id}, + } + + # When + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_404_NOT_FOUND + assert "does not exist" in str(response.json()) + + +@pytest.mark.parametrize( + "environment_", + (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), +) +def test_delete_segment_override_403_without_permission( + staff_client: APIClient, + feature: Feature, + environment_: Environment, + project: Project, +) -> None: + # Given - no permissions granted + segment = Segment.objects.create(name="no_perm_segment", project=project) + + url = reverse( + "api-experiments:delete-segment-override", + kwargs={"environment_key": environment_.api_key}, + ) + + data = { + "feature": {"name": feature.name}, + "segment": {"id": segment.id}, + } + + # When + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.parametrize( + "environment_", + (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), +) +def test_delete_segment_override_403_when_workflow_enabled( + staff_client: APIClient, + feature: Feature, + environment_: Environment, + project: Project, + with_environment_permissions: WithEnvironmentPermissionsCallable, +) -> None: + # Given + with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] + environment_.minimum_change_request_approvals = 1 + environment_.save() + + segment = Segment.objects.create(name="workflow_segment", project=project) + + url = reverse( + "api-experiments:delete-segment-override", + kwargs={"environment_key": environment_.api_key}, + ) + + data = { + "feature": {"name": feature.name}, + "segment": {"id": segment.id}, + } + + # When + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_403_FORBIDDEN + assert "change requests are enabled" in str(response.json()) + + +def test_delete_segment_override_v2_versioning_creates_new_version( + staff_client: APIClient, + feature: Feature, + environment_v2_versioning: Environment, + project: Project, + with_environment_permissions: WithEnvironmentPermissionsCallable, +) -> None: + # Given + with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] + + segment = Segment.objects.create(name="v2_delete_segment", project=project) + + # Get current version and create segment override associated with it + current_version = get_current_live_environment_feature_version( + environment_v2_versioning.id, feature.id + ) + assert current_version is not None + feature_segment = FeatureSegment.objects.create( + feature=feature, + segment=segment, + environment=environment_v2_versioning, + priority=1, + ) + FeatureState.objects.create( + feature=feature, + environment=environment_v2_versioning, + feature_segment=feature_segment, + environment_feature_version=current_version, + enabled=True, + ) + + url = reverse( + "api-experiments:delete-segment-override", + kwargs={"environment_key": environment_v2_versioning.api_key}, + ) + + data = { + "feature": {"name": feature.name}, + "segment": {"id": segment.id}, + } + + # When + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_204_NO_CONTENT + + # Verify a new version was created and published + new_version = get_current_live_environment_feature_version( + environment_v2_versioning.id, feature.id + ) + assert new_version is not None + assert new_version.uuid != current_version.uuid + + # Verify segment override is not in the new version + latest_flags = get_environment_flags_list( + environment=environment_v2_versioning, feature_name=feature.name + ) + segment_overrides = [ + fs + for fs in latest_flags + if fs.feature_segment and fs.feature_segment.segment_id == segment.id + ] + assert len(segment_overrides) == 0 From 3ed837397528c7775ba62dd2b16321d8d95f3e11 Mon Sep 17 00:00:00 2001 From: Gagan Trivedi Date: Wed, 10 Dec 2025 09:33:48 +0530 Subject: [PATCH 2/2] refactor: use AuthorData in delete_segment_override and fix swagger tag - Update swagger tag from "Experimental - Feature States" to "experimental" to match other experimental endpoints - Refactor delete_segment_override to use AuthorData dataclass instead of separate user/api_key parameters for consistency with other functions --- api/features/feature_states/serializers.py | 8 ++------ api/features/feature_states/views.py | 2 +- api/features/versioning/versioning_service.py | 15 +++++++-------- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/api/features/feature_states/serializers.py b/api/features/feature_states/serializers.py index 74b938ff15a6..1f0e5ab60357 100644 --- a/api/features/feature_states/serializers.py +++ b/api/features/feature_states/serializers.py @@ -14,7 +14,6 @@ update_flag_v2, ) from segments.models import Segment -from users.models import FFAdminUser class BaseFeatureUpdateSerializer(serializers.Serializer): # type: ignore[type-arg] @@ -211,9 +210,6 @@ def validate_segment(self, value: dict) -> dict: # type: ignore[type-arg] def save(self, **kwargs: object) -> None: feature = self.get_feature() segment_id = self.validated_data["segment"]["id"] + author = AuthorData.from_request(self.context["request"]) - request = self.context["request"] - user = request.user if isinstance(request.user, FFAdminUser) else None - api_key = getattr(request.user, "key", None) if user is None else None - - delete_segment_override(self.environment, feature, segment_id, user, api_key) + delete_segment_override(self.environment, feature, segment_id, author) diff --git a/api/features/feature_states/views.py b/api/features/feature_states/views.py index 04a608c2b04f..11a3edd066ce 100644 --- a/api/features/feature_states/views.py +++ b/api/features/feature_states/views.py @@ -182,7 +182,7 @@ def update_flag_v2(request: Request, environment_key: str) -> Response: responses={ 204: openapi.Response(description="Segment override deleted successfully") }, - tags=["Experimental - Feature States"], + tags=["experimental"], ) # type: ignore[misc] @api_view(http_method_names=["POST"]) @permission_classes([IsAuthenticated, EnvironmentUpdateFeatureStatePermission]) diff --git a/api/features/versioning/versioning_service.py b/api/features/versioning/versioning_service.py index 167264fabbe2..21659cb6dbc7 100644 --- a/api/features/versioning/versioning_service.py +++ b/api/features/versioning/versioning_service.py @@ -9,6 +9,7 @@ from features.feature_states.models import FeatureValueType from features.models import Feature, FeatureSegment, FeatureState, FeatureStateValue from features.versioning.dataclasses import ( + AuthorData, FlagChangeSet, FlagChangeSetV2, ) @@ -400,11 +401,10 @@ def delete_segment_override( environment: "Environment", feature: "Feature", segment_id: int, - user: "typing.Any" = None, - api_key: str | None = None, + author: AuthorData, ) -> None: if environment.use_v2_feature_versioning: - _delete_segment_override_v2(environment, feature, segment_id, user, api_key) + _delete_segment_override_v2(environment, feature, segment_id, author) else: _delete_segment_override_v1(environment, feature, segment_id) @@ -427,8 +427,7 @@ def _delete_segment_override_v2( environment: "Environment", feature: "Feature", segment_id: int, - user: "typing.Any", - api_key: str | None, + author: AuthorData, ) -> None: current_version = get_current_live_environment_feature_version( environment.id, feature.id @@ -444,8 +443,8 @@ def _delete_segment_override_v2( new_version = EnvironmentFeatureVersion.objects.create( environment=environment, feature=feature, - created_by=user, - created_by_api_key=api_key, + created_by=author.user, + created_by_api_key=author.api_key, ) segment_feature_state = new_version.feature_states.get( @@ -453,7 +452,7 @@ def _delete_segment_override_v2( ) segment_feature_state.feature_segment.delete() - new_version.publish(published_by=user, published_by_api_key=api_key) + new_version.publish(published_by=author.user, published_by_api_key=author.api_key) def get_updated_feature_states_for_version(