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..1f0e5ab60357 100644 --- a/api/features/feature_states/serializers.py +++ b/api/features/feature_states/serializers.py @@ -8,7 +8,11 @@ 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 @@ -188,3 +192,24 @@ 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"] + author = AuthorData.from_request(self.context["request"]) + + 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 d992e42a7072..11a3edd066ce 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"], +) # 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..21659cb6dbc7 100644 --- a/api/features/versioning/versioning_service.py +++ b/api/features/versioning/versioning_service.py @@ -3,11 +3,13 @@ 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 ( + AuthorData, FlagChangeSet, FlagChangeSetV2, ) @@ -395,6 +397,64 @@ 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, + author: AuthorData, +) -> None: + if environment.use_v2_feature_versioning: + _delete_segment_override_v2(environment, feature, segment_id, author) + 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, + author: AuthorData, +) -> 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=author.user, + created_by_api_key=author.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=author.user, published_by_api_key=author.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