diff --git a/api/api/urls/experiments.py b/api/api/urls/experiments.py new file mode 100644 index 000000000000..2fbb500ee4f0 --- /dev/null +++ b/api/api/urls/experiments.py @@ -0,0 +1,25 @@ +""" +Experimental API endpoints. + +These endpoints are subject to change and should not be considered stable. +Use at your own risk - breaking changes may occur without prior notice. +""" + +from django.urls import path + +from features.feature_states.views import update_flag_v1, update_flag_v2 + +app_name = "experiments" + +urlpatterns = [ + path( + "environments//update-flag-v1/", + update_flag_v1, + name="update-flag-v1", + ), + path( + "environments//update-flag-v2/", + update_flag_v2, + name="update-flag-v2", + ), +] diff --git a/api/app/urls.py b/api/app/urls.py index f936b8d70953..69fd350419eb 100644 --- a/api/app/urls.py +++ b/api/app/urls.py @@ -22,6 +22,10 @@ ), re_path(r"^api/v1/", include("api.urls.v1", namespace="api-v1")), re_path(r"^api/v2/", include("api.urls.v2", namespace="api-v2")), + re_path( + r"^api/experiments/", + include("api.urls.experiments", namespace="api-experiments"), + ), re_path(r"^admin/", admin.site.urls), re_path( r"^sales-dashboard/", diff --git a/api/features/feature_states/models.py b/api/features/feature_states/models.py index 9cbf1bb65e5e..05486661e97e 100644 --- a/api/features/feature_states/models.py +++ b/api/features/feature_states/models.py @@ -1,4 +1,5 @@ import typing +from typing import Literal from django.conf import settings from django.db import models @@ -10,6 +11,9 @@ STRING, ) +# TODO: use Pydantic TypeAdapter to map serializer data to DTOs +FeatureValueType = Literal["string", "integer", "boolean"] + class AbstractBaseFeatureValueModel(models.Model): class Meta: @@ -38,3 +42,36 @@ def value(self) -> typing.Union[str, int, bool]: self.type, # type: ignore[arg-type] self.string_value, ) + + def set_value(self, value: str, type_: FeatureValueType) -> None: + typed_value: str | int | bool + match type_: + case "string": + typed_value = value + field = "string_value" + type_const = STRING + case "integer": + try: + typed_value = int(value) + except ValueError: + raise ValueError(f"'{value}' is not a valid integer") + field = "integer_value" + type_const = INTEGER + case "boolean": + if value.lower() not in ("true", "false"): + raise ValueError( + f"'{value}' is not a valid boolean (use 'true' or 'false')" + ) + typed_value = value.lower() == "true" + field = "boolean_value" + type_const = BOOLEAN + case _: + raise ValueError( + f"'{type_}' is not a valid type (use 'string', 'integer', or 'boolean')" + ) + + self.string_value = None + self.integer_value = None + self.boolean_value = None + setattr(self, field, typed_value) + self.type = type_const diff --git a/api/features/feature_states/permissions.py b/api/features/feature_states/permissions.py new file mode 100644 index 000000000000..d7bf46e80d9f --- /dev/null +++ b/api/features/feature_states/permissions.py @@ -0,0 +1,19 @@ +from common.environments.permissions import UPDATE_FEATURE_STATE +from rest_framework.permissions import BasePermission +from rest_framework.request import Request +from rest_framework.views import APIView + +from environments.models import Environment + + +class EnvironmentUpdateFeatureStatePermission(BasePermission): + def has_permission(self, request: Request, view: APIView) -> bool: + environment_key = view.kwargs.get("environment_key") + try: + environment = Environment.objects.get(api_key=environment_key) + except Environment.DoesNotExist: + return False + + return request.user.has_environment_permission( # type: ignore[union-attr] + UPDATE_FEATURE_STATE, environment + ) diff --git a/api/features/feature_states/serializers.py b/api/features/feature_states/serializers.py new file mode 100644 index 000000000000..cbf6f4567587 --- /dev/null +++ b/api/features/feature_states/serializers.py @@ -0,0 +1,190 @@ +from rest_framework import serializers + +from environments.models import Environment +from features.models import Feature, FeatureState +from features.versioning.dataclasses import ( + AuthorData, + FlagChangeSet, + FlagChangeSetV2, + SegmentOverrideChangeSet, +) +from features.versioning.versioning_service import update_flag, update_flag_v2 +from segments.models import Segment + + +class BaseFeatureUpdateSerializer(serializers.Serializer): # type: ignore[type-arg] + @property + def environment(self) -> Environment: + environment: Environment | None = self.context.get("environment") + if not environment: + raise serializers.ValidationError("Environment context is required") + return environment + + def get_feature(self) -> Feature: + feature_data = self.validated_data["feature"] + try: + feature: Feature = Feature.objects.get( + project_id=self.environment.project_id, **feature_data + ) + return feature + except Feature.DoesNotExist: + raise serializers.ValidationError( + f"Feature '{feature_data}' not found in project" + ) + + def validate_segment_id(self, segment_id: int) -> None: + if not Segment.objects.filter( + id=segment_id, project_id=self.environment.project_id + ).exists(): + raise serializers.ValidationError( + f"Segment with id {segment_id} not found in project" + ) + + +class FeatureIdentifierSerializer(serializers.Serializer): # type: ignore[type-arg] + name = serializers.CharField(required=False, allow_blank=False) + id = serializers.IntegerField(required=False) + + def validate(self, data: dict) -> dict: # type: ignore[type-arg] + has_name = "name" in data + has_id = "id" in data + if not has_name and not has_id: + raise serializers.ValidationError( + "Either 'name' or 'id' is required for feature identification" + ) + if has_name and has_id: + raise serializers.ValidationError("Provide either 'name' or 'id', not both") + return data + + +class FeatureUpdateSegmentDataSerializer(serializers.Serializer): # type: ignore[type-arg] + id = serializers.IntegerField(required=True) + priority = serializers.IntegerField(required=False, allow_null=True) + + +class FeatureValueSerializer(serializers.Serializer): # type: ignore[type-arg] + type = serializers.ChoiceField( + choices=["integer", "string", "boolean"], required=True + ) + value = serializers.CharField(required=True, allow_blank=True) + + def validate(self, data: dict) -> dict: # type: ignore[type-arg] + value_type = data["type"] + string_val = data["value"] + + if value_type == "integer": + try: + int(string_val) + except ValueError: + raise serializers.ValidationError( + f"'{string_val}' is not a valid integer" + ) + elif value_type == "boolean": + if string_val.lower() not in ("true", "false"): + raise serializers.ValidationError( + f"'{string_val}' is not a valid boolean (use 'true' or 'false')" + ) + + return data + + +class UpdateFlagSerializer(BaseFeatureUpdateSerializer): + feature = FeatureIdentifierSerializer(required=True) + segment = FeatureUpdateSegmentDataSerializer(required=False) + enabled = serializers.BooleanField(required=True) + value = FeatureValueSerializer(required=True) + + def validate_segment(self, value: dict) -> dict: # type: ignore[type-arg] + if value and "id" in value: + self.validate_segment_id(value["id"]) + return value + + @property + def flag_change_set(self) -> FlagChangeSet: + validated_data = self.validated_data + value_data = validated_data["value"] + segment_data = validated_data.get("segment") + + return FlagChangeSet( + author=AuthorData.from_request(self.context["request"]), + enabled=validated_data["enabled"], + feature_state_value=value_data["value"], + type_=value_data["type"], + segment_id=segment_data.get("id") if segment_data else None, + segment_priority=segment_data.get("priority") if segment_data else None, + ) + + def save(self, **kwargs: object) -> FeatureState: + feature = self.get_feature() + return update_flag(self.environment, feature, self.flag_change_set) + + +class EnvironmentDefaultSerializer(serializers.Serializer): # type: ignore[type-arg] + enabled = serializers.BooleanField(required=True) + value = FeatureValueSerializer(required=True) + + +class SegmentOverrideSerializer(serializers.Serializer): # type: ignore[type-arg] + segment_id = serializers.IntegerField(required=True) + priority = serializers.IntegerField(required=False, allow_null=True) + enabled = serializers.BooleanField(required=True) + value = FeatureValueSerializer(required=True) + + +class UpdateFlagV2Serializer(BaseFeatureUpdateSerializer): + feature = FeatureIdentifierSerializer(required=True) + environment_default = EnvironmentDefaultSerializer(required=True) + segment_overrides = SegmentOverrideSerializer(many=True, required=False) + + def validate_segment_overrides( + self, + value: list[dict], # type: ignore[type-arg] + ) -> list[dict]: # type: ignore[type-arg] + if not value: + return value + + segment_ids = [override["segment_id"] for override in value] + if len(segment_ids) != len(set(segment_ids)): + raise serializers.ValidationError( + "Duplicate segment_id values are not allowed" + ) + + # TODO: optimise this once out of experimentation + for segment_id in segment_ids: + self.validate_segment_id(segment_id) + + return value + + @property + def change_set_v2(self) -> FlagChangeSetV2: + validated_data = self.validated_data + + env_default = validated_data["environment_default"] + env_value_data = env_default["value"] + + segment_overrides_data = validated_data.get("segment_overrides", []) + segment_overrides = [] + + for override_data in segment_overrides_data: + value_data = override_data["value"] + + segment_override = SegmentOverrideChangeSet( + segment_id=override_data["segment_id"], + enabled=override_data["enabled"], + feature_state_value=value_data["value"], + type_=value_data["type"], + priority=override_data.get("priority"), + ) + segment_overrides.append(segment_override) + + return FlagChangeSetV2( + author=AuthorData.from_request(self.context["request"]), + environment_default_enabled=env_default["enabled"], + environment_default_value=env_value_data["value"], + environment_default_type=env_value_data["type"], + segment_overrides=segment_overrides, + ) + + def save(self, **kwargs: object) -> None: + feature = self.get_feature() + update_flag_v2(self.environment, feature, self.change_set_v2) diff --git a/api/features/feature_states/views.py b/api/features/feature_states/views.py new file mode 100644 index 000000000000..d992e42a7072 --- /dev/null +++ b/api/features/feature_states/views.py @@ -0,0 +1,147 @@ +from drf_yasg import openapi # type: ignore[import-untyped] +from drf_yasg.utils import swagger_auto_schema # type: ignore[import-untyped] +from rest_framework import status +from rest_framework.decorators import api_view, permission_classes +from rest_framework.exceptions import PermissionDenied +from rest_framework.permissions import IsAuthenticated +from rest_framework.request import Request +from rest_framework.response import Response + +from environments.models import Environment + +from .permissions import EnvironmentUpdateFeatureStatePermission +from .serializers import UpdateFlagSerializer, UpdateFlagV2Serializer + + +def _check_workflow_not_enabled(environment: Environment) -> None: + if environment.is_workflow_enabled: + raise PermissionDenied( + "This endpoint cannot be used when change requests are enabled. " + "Use the change request workflow instead." + ) + + +@swagger_auto_schema( + method="post", + operation_summary="Update single feature state", + operation_description=""" + **EXPERIMENTAL ENDPOINT** - Subject to change without notice. + + Updates a single feature state within an environment. You can update either: + - The environment default state (when no segment is specified) + - A specific segment override (when segment.id is provided) + + **Feature Identification:** + - Use `feature.name` OR `feature.id` (mutually exclusive) + + **Value Format:** + - The `value` field is always a string representation + - The `type` field tells the server how to parse it + - Available types: integer, string, boolean + - Examples: + - `{"type": "integer", "value": "42"}` + - `{"type": "boolean", "value": "true"}` + - `{"type": "string", "value": "hello"}` + + **Segment Priority:** + - Optional `segment.priority` field controls ordering + - If field value is null or the field is omitted, lowest priority is assumed + """, + manual_parameters=[ + openapi.Parameter( + "environment_key", + openapi.IN_PATH, + description="The environment API key", + type=openapi.TYPE_STRING, + required=True, + ) + ], + request_body=UpdateFlagSerializer, + responses={ + 204: openapi.Response( + description="Feature state updated successfully (no content returned)" + ) + }, + tags=["experimental"], +) # type: ignore[misc] +@api_view(http_method_names=["POST"]) +@permission_classes([IsAuthenticated, EnvironmentUpdateFeatureStatePermission]) +def update_flag_v1(request: Request, environment_key: str) -> Response: + environment = Environment.objects.get(api_key=environment_key) + _check_workflow_not_enabled(environment) + + serializer = UpdateFlagSerializer( + data=request.data, + context={"request": request, "environment": environment}, + ) + serializer.is_valid(raise_exception=True) + serializer.save() + + return Response(status=status.HTTP_204_NO_CONTENT) + + +@swagger_auto_schema( + method="post", + operation_summary="Update multiple feature states", + operation_description=""" + **EXPERIMENTAL ENDPOINT** - Subject to change without notice. + + Updates multiple feature states in a single request. This endpoint allows + you to configure an entire feature (environment default + all segment overrides) + in one operation. + + **What You Can Update:** + - Environment default state (required) + - Multiple segment overrides (optional) + - Priority ordering for each segment + + **Feature Identification:** + - Use `feature.name` OR `feature.id` (mutually exclusive) + + **Value Format:** + - The `value` field is always a string representation + - The `type` field tells the server how to parse it + - Available types: integer, string, boolean + - Examples: + - `{"type": "string", "value": "production"}` + - `{"type": "integer", "value": "100"}` + - `{"type": "boolean", "value": "false"}` + + **Segment Overrides:** + - Provide array of segment override configurations + - Each override must specify: segment_id, enabled, value + - Optional priority field controls ordering + - Duplicate segment_id values are not allowed + + """, + manual_parameters=[ + openapi.Parameter( + "environment_key", + openapi.IN_PATH, + description="The environment API key", + type=openapi.TYPE_STRING, + required=True, + ) + ], + request_body=UpdateFlagV2Serializer, + responses={ + 204: openapi.Response( + description="Feature states updated successfully (no content returned)" + ) + }, + tags=["experimental"], +) # type: ignore[misc] +@api_view(http_method_names=["POST"]) +@permission_classes([IsAuthenticated, EnvironmentUpdateFeatureStatePermission]) +def update_flag_v2(request: Request, environment_key: str) -> Response: + environment = Environment.objects.get(api_key=environment_key) + _check_workflow_not_enabled(environment) + + serializer = UpdateFlagV2Serializer( + 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/dataclasses.py b/api/features/versioning/dataclasses.py index 0c4476a4f51a..029cfc2e75c2 100644 --- a/api/features/versioning/dataclasses.py +++ b/api/features/versioning/dataclasses.py @@ -1,7 +1,36 @@ +from __future__ import annotations + +from dataclasses import dataclass, field from datetime import datetime +from typing import TYPE_CHECKING from pydantic import BaseModel, computed_field +from features.feature_states.models import FeatureValueType + +if TYPE_CHECKING: + from rest_framework.request import Request + + from api_keys.models import MasterAPIKey + from users.models import FFAdminUser + + +@dataclass +class AuthorData: + user: FFAdminUser | None = None + api_key: MasterAPIKey | None = None + + @classmethod + def from_request(cls, request: Request) -> AuthorData: + from users.models import FFAdminUser + + if type(request.user) is FFAdminUser: + return cls(user=request.user) + elif hasattr(request.user, "key"): + return cls(api_key=request.user.key) + else: + raise ValueError("Request user must be FFAdminUser or have an API key") + class Conflict(BaseModel): segment_id: int | None = None @@ -12,3 +41,33 @@ class Conflict(BaseModel): @property def is_environment_default(self) -> bool: return self.segment_id is None + + +@dataclass +class FlagChangeSet: + author: AuthorData + enabled: bool + feature_state_value: str + type_: FeatureValueType + + segment_id: int | None = None + segment_priority: int | None = None + + +@dataclass +class SegmentOverrideChangeSet: + segment_id: int + enabled: bool + feature_state_value: str + type_: FeatureValueType + priority: int | None = None + + +@dataclass +class FlagChangeSetV2: + author: AuthorData + environment_default_enabled: bool + environment_default_value: str + environment_default_type: FeatureValueType + + segment_overrides: list[SegmentOverrideChangeSet] = field(default_factory=list) diff --git a/api/features/versioning/versioning_service.py b/api/features/versioning/versioning_service.py index 6cb96960cc21..82093dc48df3 100644 --- a/api/features/versioning/versioning_service.py +++ b/api/features/versioning/versioning_service.py @@ -5,7 +5,12 @@ from django.utils import timezone from environments.models import Environment -from features.models import FeatureState +from features.feature_states.models import FeatureValueType +from features.models import Feature, FeatureState, FeatureStateValue +from features.versioning.dataclasses import ( + FlagChangeSet, + FlagChangeSetV2, +) from features.versioning.models import EnvironmentFeatureVersion @@ -101,6 +106,295 @@ def get_current_live_environment_feature_version( ) +def update_flag( + environment: Environment, feature: Feature, change_set: FlagChangeSet +) -> FeatureState: + if environment.use_v2_feature_versioning: + return _update_flag_for_versioning_v2(environment, feature, change_set) + else: + return _update_flag_for_versioning_v1(environment, feature, change_set) + + +def _update_flag_for_versioning_v2( + environment: Environment, feature: Feature, change_set: FlagChangeSet +) -> FeatureState: + from features.models import FeatureSegment, FeatureState + + new_version = EnvironmentFeatureVersion.objects.create( + environment=environment, + feature=feature, + created_by=change_set.author.user, + created_by_api_key=change_set.author.api_key, + ) + + if change_set.segment_id is not None: + # Segment override - may or may not exist + try: + target_feature_state: FeatureState = new_version.feature_states.get( + feature_segment__segment_id=change_set.segment_id, + ) + except FeatureState.DoesNotExist: + feature_segment = FeatureSegment.objects.create( + feature=feature, + segment_id=change_set.segment_id, + environment=environment, + environment_feature_version=new_version, + ) + + target_feature_state = FeatureState.objects.create( + feature=feature, + environment=environment, + feature_segment=feature_segment, + environment_feature_version=new_version, + enabled=change_set.enabled, + ) + else: + # Environment default - always exists + target_feature_state = new_version.feature_states.get( + feature_segment__isnull=True, + identity_id=None, + ) + + target_feature_state.enabled = change_set.enabled + target_feature_state.save() + + _update_feature_state_value( + target_feature_state.feature_state_value, + change_set.feature_state_value, + change_set.type_, + ) + + if change_set.segment_id is not None and change_set.segment_priority is not None: + _update_segment_priority(target_feature_state, change_set.segment_priority) + + new_version.publish( + published_by=change_set.author.user, + published_by_api_key=change_set.author.api_key, + ) + + return target_feature_state + + +def _update_flag_for_versioning_v1( + environment: Environment, feature: Feature, change_set: FlagChangeSet +) -> FeatureState: + from features.models import FeatureSegment, FeatureState + + if change_set.segment_id is not None: + additional_filters = Q(feature_segment__segment_id=change_set.segment_id) + else: + additional_filters = Q(feature_segment__isnull=True, identity_id__isnull=True) + + latest_feature_states = get_environment_flags_dict( + environment=environment, + feature_name=feature.name, + additional_filters=additional_filters, + ) + + if len(latest_feature_states) == 0 and change_set.segment_id is not None: + feature_segment = FeatureSegment.objects.create( + feature=feature, + segment_id=change_set.segment_id, + environment=environment, + ) + + target_feature_state: FeatureState = FeatureState.objects.create( + feature=feature, + environment=environment, + feature_segment=feature_segment, + enabled=change_set.enabled, + ) + else: + assert len(latest_feature_states) == 1 + target_feature_state = list(latest_feature_states.values())[0] + target_feature_state.enabled = change_set.enabled + target_feature_state.save() + + _update_feature_state_value( + target_feature_state.feature_state_value, + change_set.feature_state_value, + change_set.type_, + ) + + if change_set.segment_id is not None and change_set.segment_priority is not None: + _update_segment_priority(target_feature_state, change_set.segment_priority) + + return target_feature_state + + +def _update_feature_state_value( + fsv: FeatureStateValue, value: str, type_: FeatureValueType +) -> None: + fsv.set_value(value, type_) + fsv.save() + + +def _create_segment_override( + feature: Feature, + environment: Environment, + segment_id: int, + enabled: bool, + priority: int | None, + version: EnvironmentFeatureVersion | None = None, +) -> FeatureState: + from features.models import FeatureSegment + + feature_segment = FeatureSegment.objects.create( + feature=feature, + segment_id=segment_id, + environment=environment, + environment_feature_version=version, + ) + + if priority is not None: + feature_segment.to(priority) + + segment_state: FeatureState = FeatureState.objects.create( + feature=feature, + environment=environment, + feature_segment=feature_segment, + environment_feature_version=version, + enabled=enabled, + ) + + return segment_state + + +def _update_segment_priority(feature_state: FeatureState, priority: int) -> None: + feature_segment = feature_state.feature_segment + if feature_segment: + feature_segment.to(priority) + + +def update_flag_v2( + environment: Environment, feature: Feature, change_set: FlagChangeSetV2 +) -> None: + if environment.use_v2_feature_versioning: + _update_flag_v2_for_versioning_v2(environment, feature, change_set) + else: + _update_flag_v2_for_versioning_v1(environment, feature, change_set) + + +def _update_flag_v2_for_versioning_v2( + environment: Environment, feature: Feature, change_set: FlagChangeSetV2 +) -> None: + new_version = EnvironmentFeatureVersion.objects.create( + environment=environment, + feature=feature, + created_by=change_set.author.user, + created_by_api_key=change_set.author.api_key, + ) + + env_default_state = new_version.feature_states.get( + feature_segment__isnull=True, identity_id=None + ) + env_default_state.enabled = change_set.environment_default_enabled + env_default_state.save() + + _update_feature_state_value( + env_default_state.feature_state_value, + change_set.environment_default_value, + change_set.environment_default_type, + ) + + for override in change_set.segment_overrides: + try: + segment_state = new_version.feature_states.get( + feature_segment__segment_id=override.segment_id + ) + segment_state.enabled = override.enabled + segment_state.save() + + _update_feature_state_value( + segment_state.feature_state_value, + override.feature_state_value, + override.type_, + ) + + if override.priority is not None: + _update_segment_priority(segment_state, override.priority) + except FeatureState.DoesNotExist: + segment_state = _create_segment_override( + feature=feature, + environment=environment, + segment_id=override.segment_id, + enabled=override.enabled, + priority=override.priority, + version=new_version, + ) + + _update_feature_state_value( + segment_state.feature_state_value, + override.feature_state_value, + override.type_, + ) + + new_version.publish( + published_by=change_set.author.user, + published_by_api_key=change_set.author.api_key, + ) + + +def _update_flag_v2_for_versioning_v1( + environment: Environment, feature: Feature, change_set: FlagChangeSetV2 +) -> None: + env_default_states = get_environment_flags_dict( + environment=environment, + feature_name=feature.name, + additional_filters=Q(feature_segment__isnull=True, identity_id__isnull=True), + ) + assert len(env_default_states) == 1 + + env_default_state = list(env_default_states.values())[0] + env_default_state.enabled = change_set.environment_default_enabled + env_default_state.save() + + _update_feature_state_value( + env_default_state.feature_state_value, + change_set.environment_default_value, + change_set.environment_default_type, + ) + + for override in change_set.segment_overrides: + # TODO: optimise this once this is out of the + # experimentation stage + segment_states = get_environment_flags_dict( + environment=environment, + feature_name=feature.name, + additional_filters=Q(feature_segment__segment_id=override.segment_id), + ) + + if len(segment_states) == 0: + segment_state = _create_segment_override( + feature=feature, + environment=environment, + segment_id=override.segment_id, + enabled=override.enabled, + priority=override.priority, + version=None, # V1 versioning doesn't use versions + ) + + _update_feature_state_value( + segment_state.feature_state_value, + override.feature_state_value, + override.type_, + ) + else: + assert len(segment_states) == 1 + segment_state = list(segment_states.values())[0] + segment_state.enabled = override.enabled + segment_state.save() + + _update_feature_state_value( + segment_state.feature_state_value, + override.feature_state_value, + override.type_, + ) + + if override.priority is not None: + _update_segment_priority(segment_state, override.priority) + + def get_updated_feature_states_for_version( version: EnvironmentFeatureVersion, ) -> list[FeatureState]: diff --git a/api/tests/unit/features/feature_states/__init__.py b/api/tests/unit/features/feature_states/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/api/tests/unit/features/feature_states/test_models.py b/api/tests/unit/features/feature_states/test_models.py new file mode 100644 index 000000000000..90b763360177 --- /dev/null +++ b/api/tests/unit/features/feature_states/test_models.py @@ -0,0 +1,110 @@ +from typing import cast + +import pytest + +from features.feature_states.models import FeatureValueType +from features.models import FeatureState +from features.value_types import BOOLEAN, INTEGER, STRING + + +@pytest.mark.parametrize( + "value,type_,expected_value,expected_type", + [ + ("hello", "string", "hello", STRING), + ("42", "integer", 42, INTEGER), + ("true", "boolean", True, BOOLEAN), + ("false", "boolean", False, BOOLEAN), + ("TRUE", "boolean", True, BOOLEAN), + ], +) +def test_set_value( + feature_state: FeatureState, + value: str, + type_: FeatureValueType, + expected_value: str | int | bool, + expected_type: str, +) -> None: + # Given + fsv = feature_state.feature_state_value + + # When + fsv.set_value(value, type_) + + # Then + assert fsv.value == expected_value + assert fsv.type == expected_type + + +@pytest.mark.parametrize( + "value,type_,expected_error", + [ + ("not_a_number", "integer", "'not_a_number' is not a valid integer"), + ("yes", "boolean", "'yes' is not a valid boolean"), + ("hello", "invalid", "'invalid' is not a valid type"), + ], +) +def test_set_value_invalid_raises_error( + feature_state: FeatureState, + value: str, + type_: str, + expected_error: str, +) -> None: + # Given + fsv = feature_state.feature_state_value + + # When / Then + with pytest.raises(ValueError) as exc_info: + fsv.set_value(value, cast(FeatureValueType, type_)) + + assert expected_error in str(exc_info.value) + + +def test_set_value_clears_old_fields_when_changing_type( + feature_state: FeatureState, +) -> None: + # Given + fsv = feature_state.feature_state_value + fsv.set_value("42", "integer") + fsv.save() + + assert fsv.integer_value == 42 + + # When - change from integer to string + fsv.set_value("hello", "string") + + # Then - integer_value should be cleared + assert fsv.string_value == "hello" + assert fsv.integer_value is None + assert fsv.boolean_value is None + assert fsv.type == STRING + + +@pytest.mark.parametrize( + "invalid_value,invalid_type", + [ + ("not_a_number", "integer"), + ("yes", "boolean"), + ("hello", "invalid_type"), + ], +) +def test_set_value_preserves_original_value_on_error( + feature_state: FeatureState, + invalid_value: str, + invalid_type: str, +) -> None: + # Given + fsv = feature_state.feature_state_value + fsv.set_value("42", "integer") + fsv.save() + + assert fsv.integer_value == 42 + assert fsv.type == INTEGER + + # When - try to set invalid value + with pytest.raises(ValueError): + fsv.set_value(invalid_value, cast(FeatureValueType, invalid_type)) + + # Then - original value should be preserved + assert fsv.integer_value == 42 + assert fsv.type == INTEGER + assert fsv.value == 42 diff --git a/api/tests/unit/features/feature_states/test_serializers.py b/api/tests/unit/features/feature_states/test_serializers.py new file mode 100644 index 000000000000..3b31a7fa9c7f --- /dev/null +++ b/api/tests/unit/features/feature_states/test_serializers.py @@ -0,0 +1,181 @@ +import typing + +import pytest +from rest_framework import serializers + +from environments.models import Environment +from features.feature_states.serializers import ( + FeatureValueSerializer, + UpdateFlagSerializer, + UpdateFlagV2Serializer, +) +from features.models import Feature +from projects.models import Project +from segments.models import Segment + + +def test_get_feature_raises_error_when_environment_not_in_context( + feature: Feature, +) -> None: + # Given + serializer = UpdateFlagSerializer( + data={ + "feature": {"name": feature.name}, + "enabled": True, + "value": {"type": "string", "value": "test"}, + }, + context={}, # No environment + ) + serializer.is_valid() + + # When + with pytest.raises(serializers.ValidationError) as exc_info: + serializer.get_feature() + + # Then + assert "Environment context is required" in str(exc_info.value) + + +def test_validate_segment_overrides_returns_empty_list() -> None: + # Given + serializer = UpdateFlagV2Serializer() + + # When + result = serializer.validate_segment_overrides([]) + + # Then + assert result == [] + + +def test_feature_value_serializer_rejects_invalid_integer() -> None: + # Given + serializer = FeatureValueSerializer( + data={"type": "integer", "value": "not_a_number"} + ) + + # When + is_valid = serializer.is_valid() + + # Then + assert is_valid is False + assert "not a valid integer" in str(serializer.errors) + + +def test_feature_value_serializer_rejects_invalid_boolean() -> None: + # Given + serializer = FeatureValueSerializer(data={"type": "boolean", "value": "yes"}) + + # When + is_valid = serializer.is_valid() + + # Then + assert is_valid is False + assert "not a valid boolean" in str(serializer.errors) + + +@pytest.mark.parametrize( + "serializer_class,data_factory", + [ + ( + UpdateFlagSerializer, + lambda feature, segment_id: { + "feature": {"name": feature.name}, + "segment": {"id": segment_id}, + "enabled": True, + "value": {"type": "string", "value": "test"}, + }, + ), + ( + UpdateFlagV2Serializer, + lambda feature, segment_id: { + "feature": {"name": feature.name}, + "environment_default": { + "enabled": True, + "value": {"type": "string", "value": "default"}, + }, + "segment_overrides": [ + { + "segment_id": segment_id, + "enabled": True, + "value": {"type": "string", "value": "test"}, + }, + ], + }, + ), + ], +) +def test_serializer_rejects_nonexistent_segment( + feature: Feature, + environment: Environment, + serializer_class: type, + data_factory: typing.Callable[[Feature, int], dict], # type: ignore[type-arg] +) -> None: + # Given + serializer = serializer_class( + data=data_factory(feature, 999999), + context={"environment": environment}, + ) + + # When + is_valid = serializer.is_valid() + + # Then + assert is_valid is False + assert "not found in project" in str(serializer.errors) + + +@pytest.mark.parametrize( + "serializer_class,data_factory", + [ + ( + UpdateFlagSerializer, + lambda feature, segment_id: { + "feature": {"name": feature.name}, + "segment": {"id": segment_id}, + "enabled": True, + "value": {"type": "string", "value": "test"}, + }, + ), + ( + UpdateFlagV2Serializer, + lambda feature, segment_id: { + "feature": {"name": feature.name}, + "environment_default": { + "enabled": True, + "value": {"type": "string", "value": "default"}, + }, + "segment_overrides": [ + { + "segment_id": segment_id, + "enabled": True, + "value": {"type": "string", "value": "test"}, + }, + ], + }, + ), + ], +) +def test_serializer_rejects_cross_project_segment( + feature: Feature, + environment: Environment, + organisation: object, + serializer_class: type, + data_factory: typing.Callable[[Feature, int], dict], # type: ignore[type-arg] +) -> None: + # Given + other_project = Project.objects.create( + name="Other Project", + organisation=organisation, + ) + other_segment = Segment.objects.create(name="other_segment", project=other_project) + serializer = serializer_class( + data=data_factory(feature, other_segment.id), + context={"environment": environment}, + ) + + # When + is_valid = serializer.is_valid() + + # Then + assert is_valid is False + assert "not found in project" in str(serializer.errors) 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 new file mode 100644 index 000000000000..f290abdaed16 --- /dev/null +++ b/api/tests/unit/features/feature_states/test_unit_feature_states_views.py @@ -0,0 +1,1052 @@ +import json +import typing + +import pytest +from common.environments.permissions import UPDATE_FEATURE_STATE +from django.urls import reverse +from pytest_lazyfixture import lazy_fixture # type: ignore[import-untyped] +from rest_framework import status +from rest_framework.test import APIClient + +from environments.models import Environment +from features.models import Feature, FeatureSegment, FeatureState +from features.versioning.versioning_service import ( + get_current_live_environment_feature_version, + get_environment_flags_list, +) +from projects.models import Project +from segments.models import Segment +from tests.types import WithEnvironmentPermissionsCallable + + +@pytest.mark.parametrize( + "value_type,string_value,expected_value", + [ + ("integer", "42", 42), + ("string", "hello", "hello"), + ("boolean", "true", True), + ], +) +@pytest.mark.parametrize( + "environment_", + (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), +) +def test_update_flag_by_name( + staff_client: APIClient, + feature: Feature, + environment_: Environment, + with_environment_permissions: WithEnvironmentPermissionsCallable, + value_type: str, + string_value: str, + expected_value: typing.Any, +) -> None: + # Given + with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] + url = reverse( + "api-experiments:update-flag-v1", + kwargs={"environment_key": environment_.api_key}, + ) + + data = { + "feature": {"name": feature.name}, + "enabled": True, + "value": {"type": value_type, "value": string_value}, + } + + # 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 + ) + + assert latest_flags[0].enabled is True + assert latest_flags[0].get_feature_state_value() == expected_value + + +@pytest.mark.parametrize( + "environment_", + (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), +) +def test_update_flag_by_id( + 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:update-flag-v1", + kwargs={"environment_key": environment_.api_key}, + ) + + data = { + "feature": {"id": feature.id}, + "enabled": False, + "value": {"type": "string", "value": "test_value"}, + } + + # 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 + ) + + assert latest_flags[0].enabled is False + assert latest_flags[0].get_feature_state_value() == "test_value" + + +@pytest.mark.parametrize( + "environment_", + (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), +) +def test_update_flag_error_when_both_name_and_id_provided( + 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:update-flag-v1", + kwargs={"environment_key": environment_.api_key}, + ) + + # Providing both name and ID (even if they match the same feature) + data = { + "feature": {"name": feature.name, "id": feature.id}, + "enabled": True, + "value": {"type": "integer", "value": "42"}, + } + + # 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 "feature" in response.json() + assert "Provide either 'name' or 'id', not both" in str(response.json()["feature"]) + + +@pytest.mark.parametrize( + "environment_", + (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), +) +def test_update_flag_error_when_both_name_and_id_provided_for_different_features( + 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] + + # Create another feature in the same project + another_feature = Feature.objects.create( + name="another_feature", project=project, type="STANDARD" + ) + + url = reverse( + "api-experiments:update-flag-v1", + kwargs={"environment_key": environment_.api_key}, + ) + + # Providing both name and ID for DIFFERENT features (worst case) + data = { + "feature": {"name": feature.name, "id": another_feature.id}, + "enabled": True, + "value": {"type": "integer", "value": "42"}, + } + + # When + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then - Should fail at validation level (mutual exclusion) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "feature" in response.json() + assert "Provide either 'name' or 'id', not both" in str(response.json()["feature"]) + + +@pytest.mark.parametrize( + "environment_", + (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), +) +def test_update_flag_error_when_neither_name_nor_id_provided( + 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:update-flag-v1", + kwargs={"environment_key": environment_.api_key}, + ) + + # Providing empty feature object + data = { + "feature": {}, + "enabled": True, + "value": {"type": "integer", "value": "42"}, + } + + # 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 "feature" in response.json() + assert "Either 'name' or 'id' is required" in str(response.json()["feature"]) + + +@pytest.mark.parametrize( + "environment_", + (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), +) +def test_update_flag_error_when_feature_not_found_by_name( + staff_client: APIClient, + environment_: Environment, + with_environment_permissions: WithEnvironmentPermissionsCallable, +) -> None: + # Given + with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] + url = reverse( + "api-experiments:update-flag-v1", + kwargs={"environment_key": environment_.api_key}, + ) + + data = { + "feature": {"name": "non_existent_feature"}, + "enabled": True, + "value": {"type": "integer", "value": "42"}, + } + + # 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_update_flag_error_when_feature_not_found_by_id( + staff_client: APIClient, + environment_: Environment, + with_environment_permissions: WithEnvironmentPermissionsCallable, +) -> None: + # Given + with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] + url = reverse( + "api-experiments:update-flag-v1", + kwargs={"environment_key": environment_.api_key}, + ) + + data = { + "feature": {"id": 999999}, # Non-existent ID + "enabled": True, + "value": {"type": "integer", "value": "42"}, + } + + # 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_update_flag_segment_override_by_name( + 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="test_segment", project=project, description="Test segment" + ) + + url = reverse( + "api-experiments:update-flag-v1", + kwargs={"environment_key": environment_.api_key}, + ) + + data = { + "feature": {"name": feature.name}, + "segment": {"id": segment.id, "priority": 1}, + "enabled": False, + "value": {"type": "integer", "value": "999"}, + } + + # 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 the segment override was created + latest_flags = get_environment_flags_list( + environment=environment_, feature_name=feature.name + ) + segment_override = [ + fs + for fs in latest_flags + if fs.feature_segment and fs.feature_segment.segment_id == segment.id + ][0] + assert segment_override.enabled is False + assert segment_override.get_feature_state_value() == 999 + + +@pytest.mark.parametrize( + "environment_", + (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), +) +def test_update_flag_segment_override_creates_feature_segment_if_not_exists( + 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="new_segment", + project=project, + description="New segment for testing creation", + ) + + # Verify FeatureSegment doesn't exist yet + assert not FeatureSegment.objects.filter( + feature=feature, segment=segment, environment=environment_ + ).exists() + + url = reverse( + "api-experiments:update-flag-v1", + kwargs={"environment_key": environment_.api_key}, + ) + + data = { + "feature": {"name": feature.name}, + "segment": {"id": segment.id, "priority": 10}, + "enabled": True, + "value": {"type": "string", "value": "premium_feature"}, + } + + # When + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then - Should succeed and CREATE the FeatureSegment + assert response.status_code == status.HTTP_204_NO_CONTENT + + # Verify the segment override was created + latest_flags = get_environment_flags_list( + environment=environment_, feature_name=feature.name + ) + segment_override = [ + fs + for fs in latest_flags + if fs.feature_segment and fs.feature_segment.segment_id == segment.id + ][0] + assert segment_override.enabled is True + assert segment_override.get_feature_state_value() == "premium_feature" + assert segment_override.feature_segment is not None + assert segment_override.feature_segment.priority == 10 + + +def test_create_new_segment_override_reorders_priorities_v1( + 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] + + segment1 = Segment.objects.create(name="segment1", project=project) + segment2 = Segment.objects.create(name="segment2", project=project) + + # Create existing segment override at priority 0 + feature_segment1 = FeatureSegment.objects.create( + feature=feature, + segment=segment1, + environment=environment, + priority=0, + ) + FeatureState.objects.create( + feature=feature, + environment=environment, + feature_segment=feature_segment1, + enabled=True, + ) + + url = reverse( + "api-experiments:update-flag-v1", + kwargs={"environment_key": environment.api_key}, + ) + + # When - Create new segment override at priority 0 (should push segment1 to priority 1) + data = { + "feature": {"name": feature.name}, + "segment": {"id": segment2.id, "priority": 0}, + "enabled": True, + "value": {"type": "string", "value": "new_segment_value"}, + } + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_204_NO_CONTENT + + # Refresh from DB + feature_segment1.refresh_from_db() + feature_segment2 = FeatureSegment.objects.get( + feature=feature, segment=segment2, environment=environment + ) + + # segment2 should be at priority 0, segment1 should have been pushed to priority 1 + assert feature_segment2.priority == 0 + assert feature_segment1.priority == 1 + + +# Update Multiple Feature States Tests + + +@pytest.mark.parametrize( + "environment_", + (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), +) +def test_update_feature_states_creates_new_segment_overrides( + 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] + + # Create two segments that don't have feature overrides yet + segment1 = Segment.objects.create( + name="vip_segment", project=project, description="VIP users" + ) + segment2 = Segment.objects.create( + name="beta_segment", project=project, description="Beta testers" + ) + + # Verify neither FeatureSegment exists yet + assert not FeatureSegment.objects.filter( + feature=feature, segment=segment1, environment=environment_ + ).exists() + assert not FeatureSegment.objects.filter( + feature=feature, segment=segment2, environment=environment_ + ).exists() + + url = reverse( + "api-experiments:update-flag-v2", + kwargs={"environment_key": environment_.api_key}, + ) + + # Batch update with environment default + 2 new segment overrides + data = { + "feature": {"name": feature.name}, + "environment_default": { + "enabled": False, + "value": {"type": "string", "value": "default"}, + }, + "segment_overrides": [ + { + "segment_id": segment1.id, + "priority": 1, + "enabled": True, + "value": {"type": "string", "value": "vip_feature"}, + }, + { + "segment_id": segment2.id, + "priority": 2, + "enabled": True, + "value": {"type": "string", "value": "beta_feature"}, + }, + ], + } + + # 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 both segment overrides were created + latest_flags = get_environment_flags_list( + environment=environment_, feature_name=feature.name + ) + + vip_override = [ + fs + for fs in latest_flags + if fs.feature_segment and fs.feature_segment.segment_id == segment1.id + ][0] + assert vip_override.enabled is True + assert vip_override.get_feature_state_value() == "vip_feature" + + beta_override = [ + fs + for fs in latest_flags + if fs.feature_segment and fs.feature_segment.segment_id == segment2.id + ][0] + assert beta_override.enabled is True + assert beta_override.get_feature_state_value() == "beta_feature" + + +@pytest.mark.parametrize( + "environment_", + (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), +) +def test_update_feature_states_environment_default_only( + 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:update-flag-v2", + kwargs={"environment_key": environment_.api_key}, + ) + + # Test with just environment default (no segment overrides) + data = { + "feature": {"name": feature.name}, + "environment_default": { + "enabled": True, + "value": {"type": "integer", "value": "100"}, + }, + } + + # 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 the actual state in the database + latest_flags = get_environment_flags_list( + environment=environment_, feature_name=feature.name + ) + + # Find environment default + env_default = [fs for fs in latest_flags if fs.feature_segment is None][0] + assert env_default.enabled is True + assert env_default.get_feature_state_value() == 100 + + +def test_update_feature_states_rejects_duplicate_segment_ids( + 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] + + segment1 = Segment.objects.create(name="segment1", project=project) + + url = reverse( + "api-experiments:update-flag-v2", + kwargs={"environment_key": environment.api_key}, + ) + + # Request with duplicate segment_id + data = { + "feature": {"name": feature.name}, + "environment_default": { + "enabled": True, + "value": {"type": "string", "value": "default"}, + }, + "segment_overrides": [ + { + "segment_id": segment1.id, + "enabled": True, + "value": {"type": "string", "value": "value1"}, + }, + { + "segment_id": segment1.id, # Duplicate! + "enabled": False, + "value": {"type": "string", "value": "value2"}, + }, + ], + } + + # 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 "Duplicate segment_id values are not allowed" in str(response.json()) + + +@pytest.mark.parametrize( + "environment_", + (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), +) +def test_update_existing_segment_override_with_priority_v1( + staff_client: APIClient, + feature: Feature, + environment_: Environment, + project: Project, + with_environment_permissions: WithEnvironmentPermissionsCallable, +) -> None: + """Test updating an existing segment override with a new priority using V1 endpoint.""" + # Given + with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] + + segment = Segment.objects.create(name="priority_segment", project=project) + + # Create the segment override manually + feature_segment = FeatureSegment.objects.create( + feature=feature, + segment=segment, + environment=environment_, + priority=5, + ) + FeatureState.objects.create( + feature=feature, + environment=environment_, + feature_segment=feature_segment, + enabled=True, + ) + + url = reverse( + "api-experiments:update-flag-v1", + kwargs={"environment_key": environment_.api_key}, + ) + + # When - Update the same segment override with new priority + update_data = { + "feature": {"name": feature.name}, + "segment": {"id": segment.id, "priority": 1}, # Changed priority + "enabled": False, # Changed enabled + "value": {"type": "integer", "value": "200"}, # Changed value + } + response = staff_client.post( + url, data=json.dumps(update_data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_204_NO_CONTENT + + # Verify the update - get the latest feature segment (V2 versioning creates new ones) + feature_segment = ( + FeatureSegment.objects.filter( + feature=feature, segment=segment, environment=environment_ + ) + .order_by("-id") + .first() + ) + assert feature_segment is not None + assert feature_segment.priority == 1 + + +@pytest.mark.parametrize( + "environment_", + (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), +) +def test_update_existing_segment_override_with_priority_v2( + staff_client: APIClient, + feature: Feature, + environment_: Environment, + project: Project, + with_environment_permissions: WithEnvironmentPermissionsCallable, +) -> None: + """Test updating an existing segment override with a new priority using V2 endpoint.""" + # Given + with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] + + segment = Segment.objects.create(name="priority_segment_v2", project=project) + + # Create the segment override manually + feature_segment = FeatureSegment.objects.create( + feature=feature, + segment=segment, + environment=environment_, + priority=5, + ) + FeatureState.objects.create( + feature=feature, + environment=environment_, + feature_segment=feature_segment, + enabled=True, + ) + + # When - Update the existing segment override using V2 endpoint + v2_url = reverse( + "api-experiments:update-flag-v2", + kwargs={"environment_key": environment_.api_key}, + ) + update_data = { + "feature": {"name": feature.name}, + "environment_default": { + "enabled": True, + "value": {"type": "string", "value": "default"}, + }, + "segment_overrides": [ + { + "segment_id": segment.id, + "priority": 2, # Changed priority + "enabled": False, # Changed enabled + "value": {"type": "string", "value": "updated"}, + }, + ], + } + response = staff_client.post( + v2_url, data=json.dumps(update_data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_204_NO_CONTENT + + # Verify the priority was updated - get the latest feature segment + feature_segment = ( + FeatureSegment.objects.filter( + feature=feature, segment=segment, environment=environment_ + ) + .order_by("-id") + .first() + ) + assert feature_segment is not None + assert feature_segment.priority == 2 + + +def test_create_new_segment_override_reorders_priorities_v2( + 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] + + segment1 = Segment.objects.create(name="segment1_v2", project=project) + segment2 = Segment.objects.create(name="segment2_v2", project=project) + + # Create existing segment override at priority 0 + feature_segment1 = FeatureSegment.objects.create( + feature=feature, + segment=segment1, + environment=environment, + priority=0, + ) + FeatureState.objects.create( + feature=feature, + environment=environment, + feature_segment=feature_segment1, + enabled=True, + ) + + url = reverse( + "api-experiments:update-flag-v2", + kwargs={"environment_key": environment.api_key}, + ) + + # When - Create new segment override at priority 0 (should push segment1 to priority 1) + data = { + "feature": {"name": feature.name}, + "environment_default": { + "enabled": True, + "value": {"type": "string", "value": "default"}, + }, + "segment_overrides": [ + { + "segment_id": segment2.id, + "priority": 0, + "enabled": True, + "value": {"type": "string", "value": "new_segment_value"}, + }, + ], + } + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_204_NO_CONTENT + + # Refresh from DB + feature_segment1.refresh_from_db() + feature_segment2 = FeatureSegment.objects.get( + feature=feature, segment=segment2, environment=environment + ) + + # segment2 should be at priority 0, segment1 should have been pushed to priority 1 + assert feature_segment2.priority == 0 + assert feature_segment1.priority == 1 + + +def test_update_flag_v1_returns_403_when_workflow_enabled( + staff_client: APIClient, + feature: Feature, + environment: Environment, + with_environment_permissions: WithEnvironmentPermissionsCallable, +) -> None: + # Given + with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] + environment.minimum_change_request_approvals = 1 + environment.save() + + url = reverse( + "api-experiments:update-flag-v1", + kwargs={"environment_key": environment.api_key}, + ) + + data = { + "feature": {"name": feature.name}, + "enabled": True, + "value": {"type": "string", "value": "test"}, + } + + # 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_update_flag_v2_returns_403_when_workflow_enabled( + staff_client: APIClient, + feature: Feature, + environment: Environment, + with_environment_permissions: WithEnvironmentPermissionsCallable, +) -> None: + # Given + with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] + environment.minimum_change_request_approvals = 1 + environment.save() + + url = reverse( + "api-experiments:update-flag-v2", + kwargs={"environment_key": environment.api_key}, + ) + + data = { + "feature": {"name": feature.name}, + "environment_default": { + "enabled": True, + "value": {"type": "string", "value": "test"}, + }, + } + + # 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_update_existing_segment_override_v2_versioning( + 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="existing_segment", project=project) + + # Get the current live version and create segment override associated with it + current_version = get_current_live_environment_feature_version( + environment_v2_versioning.id, feature.id + ) + feature_segment = FeatureSegment.objects.create( + feature=feature, + segment=segment, + environment=environment_v2_versioning, + priority=5, + ) + FeatureState.objects.create( + feature=feature, + environment=environment_v2_versioning, + feature_segment=feature_segment, + environment_feature_version=current_version, + enabled=True, + ) + + url = reverse( + "api-experiments:update-flag-v2", + kwargs={"environment_key": environment_v2_versioning.api_key}, + ) + + # When - Update the existing segment override + data = { + "feature": {"name": feature.name}, + "environment_default": { + "enabled": True, + "value": {"type": "string", "value": "default"}, + }, + "segment_overrides": [ + { + "segment_id": segment.id, + "priority": 1, # Changed priority from 5 to 1 + "enabled": False, # Changed enabled from True to False + "value": {"type": "integer", "value": "999"}, + }, + ], + } + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_204_NO_CONTENT + + # Verify the segment override was updated + latest_flags = get_environment_flags_list( + environment=environment_v2_versioning, feature_name=feature.name + ) + segment_override = [ + fs + for fs in latest_flags + if fs.feature_segment and fs.feature_segment.segment_id == segment.id + ][0] + + assert segment_override.enabled is False + assert segment_override.get_feature_state_value() == 999 + assert segment_override.feature_segment is not None + assert segment_override.feature_segment.priority == 1 + + +def test_update_flag_v1_returns_403_without_permission( + staff_client: APIClient, + feature: Feature, + environment: Environment, +) -> None: + # Given - no permissions granted + url = reverse( + "api-experiments:update-flag-v1", + kwargs={"environment_key": environment.api_key}, + ) + + data = { + "feature": {"name": feature.name}, + "enabled": True, + "value": {"type": "string", "value": "test"}, + } + + # When + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_403_FORBIDDEN + + +def test_update_flag_v2_returns_403_without_permission( + staff_client: APIClient, + feature: Feature, + environment: Environment, +) -> None: + # Given - no permissions granted + url = reverse( + "api-experiments:update-flag-v2", + kwargs={"environment_key": environment.api_key}, + ) + + data = { + "feature": {"name": feature.name}, + "environment_default": { + "enabled": True, + "value": {"type": "string", "value": "test"}, + }, + } + + # When + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_403_FORBIDDEN + + +def test_update_flag_v1_returns_403_for_nonexistent_environment( + staff_client: APIClient, +) -> None: + # Given + url = reverse( + "api-experiments:update-flag-v1", + kwargs={"environment_key": "nonexistent_key"}, + ) + + data = { + "feature": {"name": "any_feature"}, + "enabled": True, + "value": {"type": "string", "value": "test"}, + } + + # When + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_403_FORBIDDEN diff --git a/api/tests/unit/features/test_unit_features_views.py b/api/tests/unit/features/test_unit_features_views.py index c53a59e1234a..23b79dc59565 100644 --- a/api/tests/unit/features/test_unit_features_views.py +++ b/api/tests/unit/features/test_unit_features_views.py @@ -47,7 +47,7 @@ from features.feature_types import MULTIVARIATE from features.models import Feature, FeatureSegment, FeatureState from features.multivariate.models import MultivariateFeatureOption -from features.value_types import BOOLEAN, INTEGER, STRING +from features.value_types import STRING from features.versioning.models import EnvironmentFeatureVersion from metadata.models import MetadataModelField from organisations.models import Organisation, OrganisationRole @@ -3341,9 +3341,7 @@ def test_list_features_with_feature_state( version=100, ) feature_state_value_versioned = feature_state_versioned.feature_state_value - feature_state_value_versioned.string_value = None - feature_state_value_versioned.integer_value = 2005 - feature_state_value_versioned.type = INTEGER + feature_state_value_versioned.set_value("2005", "integer") feature_state_value_versioned.save() feature_state2 = feature2.feature_states.filter(environment=environment).first() @@ -3351,9 +3349,7 @@ def test_list_features_with_feature_state( feature_state2.save() feature_state_value2 = feature_state2.feature_state_value - feature_state_value2.string_value = None - feature_state_value2.boolean_value = True - feature_state_value2.type = BOOLEAN + feature_state_value2.set_value("true", "boolean") feature_state_value2.save() feature_state_value3 = ( @@ -3361,7 +3357,7 @@ def test_list_features_with_feature_state( .first() .feature_state_value ) - feature_state_value3.string_value = "present" + feature_state_value3.set_value("present", "string") feature_state_value3.save() # This should be ignored due to identity being set. @@ -3470,9 +3466,7 @@ def test_list_features_with_filter_by_value_search_string_and_int( environment_feature_version1.publish(staff_user) feature_state_value1 = feature_state1.feature_state_value - feature_state_value1.string_value = None - feature_state_value1.integer_value = 1945 - feature_state_value1.type = INTEGER + feature_state_value1.set_value("1945", "integer") feature_state_value1.save() feature_state2 = feature2.feature_states.filter(environment=environment).first() @@ -3480,9 +3474,7 @@ def test_list_features_with_filter_by_value_search_string_and_int( feature_state2.save() feature_state_value2 = feature_state2.feature_state_value - feature_state_value2.string_value = None - feature_state_value2.boolean_value = True - feature_state_value2.type = BOOLEAN + feature_state_value2.set_value("true", "boolean") feature_state_value2.save() feature_state_value3 = ( @@ -3490,8 +3482,7 @@ def test_list_features_with_filter_by_value_search_string_and_int( .first() .feature_state_value ) - feature_state_value3.string_value = "present" - feature_state_value3.type = STRING + feature_state_value3.set_value("present", "string") feature_state_value3.save() feature_state4 = feature4.feature_states.filter(environment=environment).first() @@ -3499,8 +3490,7 @@ def test_list_features_with_filter_by_value_search_string_and_int( feature_state4.save() feature_state_value4 = feature_state4.feature_state_value - feature_state_value4.string_value = "year 1945" - feature_state_value4.type = STRING + feature_state_value4.set_value("year 1945", "string") feature_state_value4.save() base_url = reverse("api-v1:projects:project-features-list", args=[project.id]) @@ -3548,9 +3538,7 @@ def test_list_features_with_filter_by_search_value_boolean( feature_state1.save() # type: ignore[union-attr] feature_state_value1 = feature_state1.feature_state_value # type: ignore[union-attr] - feature_state_value1.string_value = None - feature_state_value1.integer_value = 1945 - feature_state_value1.type = INTEGER + feature_state_value1.set_value("1945", "integer") feature_state_value1.save() feature_state2 = feature2.feature_states.filter(environment=environment).first() @@ -3558,9 +3546,7 @@ def test_list_features_with_filter_by_search_value_boolean( feature_state2.save() feature_state_value2 = feature_state2.feature_state_value - feature_state_value2.string_value = None - feature_state_value2.boolean_value = True - feature_state_value2.type = BOOLEAN + feature_state_value2.set_value("true", "boolean") feature_state_value2.save() feature_state_value3 = ( @@ -3568,8 +3554,7 @@ def test_list_features_with_filter_by_search_value_boolean( .first() .feature_state_value ) - feature_state_value3.string_value = "present" - feature_state_value3.type = STRING + feature_state_value3.set_value("present", "string") feature_state_value3.save() feature_state4 = feature4.feature_states.filter(environment=environment).first() @@ -3577,8 +3562,7 @@ def test_list_features_with_filter_by_search_value_boolean( feature_state4.save() feature_state_value4 = feature_state4.feature_state_value - feature_state_value4.string_value = "year 1945" - feature_state_value4.type = STRING + feature_state_value4.set_value("year 1945", "string") feature_state_value4.save() base_url = reverse("api-v1:projects:project-features-list", args=[project.id]) @@ -4094,7 +4078,7 @@ def test_list_features_segment_query_param_with_valid_segment( environment=environment, enabled=True, ) - segment_override.feature_state_value.string_value = "segment_value" + segment_override.feature_state_value.set_value("segment_value", "string") segment_override.feature_state_value.save() base_url = reverse("api-v1:projects:project-features-list", args=[project.id]) diff --git a/api/tests/unit/features/versioning/test_unit_versioning_dataclasses.py b/api/tests/unit/features/versioning/test_unit_versioning_dataclasses.py index f7c89fd88efc..d169ab28cc13 100644 --- a/api/tests/unit/features/versioning/test_unit_versioning_dataclasses.py +++ b/api/tests/unit/features/versioning/test_unit_versioning_dataclasses.py @@ -1,6 +1,8 @@ +from unittest.mock import Mock + import pytest -from features.versioning.dataclasses import Conflict +from features.versioning.dataclasses import AuthorData, Conflict @pytest.mark.parametrize("segment_id, expected_result", ((None, True), (1, False))) @@ -8,3 +10,26 @@ def test_conflict_is_environment_default( segment_id: int | None, expected_result: bool ) -> None: assert Conflict(segment_id=segment_id).is_environment_default is expected_result + + +def test_author_data_from_request_sets_api_key_for_non_user() -> None: + mock_master_api_key = Mock(spec_set=["id", "name"]) + mock_api_key_user = Mock() + mock_api_key_user.key = mock_master_api_key + mock_request = Mock() + mock_request.user = mock_api_key_user + + author = AuthorData.from_request(mock_request) + + assert author.api_key is mock_master_api_key + assert author.user is None + + +def test_author_data_from_request_raises_for_invalid_user() -> None: + mock_request = Mock() + mock_request.user = object() # No .key attribute + + with pytest.raises(ValueError) as exc_info: + AuthorData.from_request(mock_request) + + assert "must be FFAdminUser or have an API key" in str(exc_info.value) diff --git a/api/tests/unit/features/versioning/test_unit_versioning_versioning_service.py b/api/tests/unit/features/versioning/test_unit_versioning_versioning_service.py index 62b591a1c95f..f6adb5238f62 100644 --- a/api/tests/unit/features/versioning/test_unit_versioning_versioning_service.py +++ b/api/tests/unit/features/versioning/test_unit_versioning_versioning_service.py @@ -600,3 +600,18 @@ def test_get_updated_feature_states_for_version_detects_segment_override_multiva assert updated_feature_states[0].feature_segment is not None assert updated_feature_states[0].feature_segment.segment == segment assert updated_feature_states[0].get_feature_state_value() == "new_control_value" + + +def test_get_environment_flags_list_with_replica( + feature: Feature, + environment: Environment, +) -> None: + # This just verifies the code path works - actual replica behavior + # depends on database configuration + result = get_environment_flags_list( + environment=environment, + from_replica=True, + ) + + assert len(result) >= 1 + assert result[0].feature == feature