diff --git a/api/api/urls/experiments.py b/api/api/urls/experiments.py index 984709354663..92efd49cc9c9 100644 --- a/api/api/urls/experiments.py +++ b/api/api/urls/experiments.py @@ -9,8 +9,8 @@ from features.feature_states.views import ( delete_segment_override, - update_flag_v1, - update_flag_v2, + update_flag_option_a, + update_flag_option_b, ) app_name = "experiments" @@ -18,13 +18,13 @@ urlpatterns = [ path( "environments//update-flag-v1/", - update_flag_v1, - name="update-flag-v1", + update_flag_option_a, + name="update-flag-option-a", ), path( "environments//update-flag-v2/", - update_flag_v2, - name="update-flag-v2", + update_flag_option_b, + name="update-flag-option-b", ), path( "environments//delete-segment-override/", diff --git a/api/experimentation/serializers.py b/api/experimentation/serializers.py index 92ca8cd0ace4..4a1556300a00 100644 --- a/api/experimentation/serializers.py +++ b/api/experimentation/serializers.py @@ -30,7 +30,7 @@ ) from features.feature_states.serializers import ( FeatureValueSerializer, - MultivariateValueSerializer, + SegmentOverrideMultivariateValueSerializer, ) from features.feature_types import MULTIVARIATE from features.models import Feature @@ -223,7 +223,7 @@ class ExperimentRolloutSerializer(serializers.Serializer): # type: ignore[type- required=True, min_value=0, max_value=100 ) feature_state_value = FeatureValueSerializer(required=True) - multivariate_feature_state_values = MultivariateValueSerializer( + multivariate_feature_state_values = SegmentOverrideMultivariateValueSerializer( many=True, required=False ) diff --git a/api/experimentation/services.py b/api/experimentation/services.py index abddebe6dc86..566eb1cddece 100644 --- a/api/experimentation/services.py +++ b/api/experimentation/services.py @@ -57,9 +57,9 @@ ) from features.models import FeatureState from features.value_types import BOOLEAN, INTEGER, STRING -from features.versioning.dataclasses import FlagChangeSet +from features.versioning.dataclasses import FeatureValue, FlagChangeSetOptionA from features.versioning.versioning_service import ( - update_flag, + update_flag_option_a, update_multivariate_values, ) from integrations.flagsmith.client import get_openfeature_client @@ -594,26 +594,31 @@ def _get_live_rollout_override(experiment: Experiment) -> FeatureState | None: def _update_live_feature_state( - feature_state: FeatureState, change_set: FlagChangeSet + feature_state: FeatureState, change_set: FlagChangeSetOptionA ) -> None: - feature_state.enabled = change_set.enabled - feature_state.save() - feature_state.feature_state_value.set_value( - change_set.feature_state_value, change_set.type_ - ) - feature_state.feature_state_value.save() - update_multivariate_values(feature_state, change_set.multivariate_values) + with transaction.atomic(): + if change_set.enabled is not None: + feature_state.enabled = change_set.enabled + feature_state.save() + if change_set.value is not None: + feature_state.feature_state_value.set_value( + change_set.value.value, change_set.value.type_ + ) + feature_state.feature_state_value.save() + update_multivariate_values(feature_state, change_set.multivariate_values) -def _update_rollout_in_place(experiment: Experiment, change_set: FlagChangeSet) -> None: +def _update_rollout_in_place( + experiment: Experiment, change_set: FlagChangeSetOptionA +) -> None: """Write the rollout-segment override, keeping variant assignment stable. - Under v2 versioning, ``update_flag`` clones the override into a fresh feature + Under v2 versioning, ``update_flag_option_a`` clones the override into a fresh feature state on every call. Since the multivariate split is salted on the feature state id, that would re-randomise control/variant for already-enrolled identities on each rollout update. Once the override exists, mutate it in place instead (no version is published). Creating the override, and v1 - versioning, still go through ``update_flag``, which already reuses the + versioning, still go through ``update_flag_option_a``, which already reuses the feature state. This is a temporary solution until we find a permanent fix for the @@ -624,7 +629,7 @@ def _update_rollout_in_place(experiment: Experiment, change_set: FlagChangeSet) ): _update_live_feature_state(override, change_set) return - update_flag(experiment.environment, experiment.feature, change_set) + update_flag_option_a(experiment.environment, experiment.feature, change_set) def apply_experiment_rollout(experiment: Experiment, spec: RolloutSpec) -> None: @@ -638,11 +643,12 @@ def apply_experiment_rollout(experiment: Experiment, spec: RolloutSpec) -> None: segment = _sync_rollout_segment(experiment, spec.rollout_percentage) _update_rollout_in_place( experiment, - FlagChangeSet( + FlagChangeSetOptionA( author=spec.author, enabled=spec.enabled, - feature_state_value=spec.feature_state_value, - type_=spec.value_type, + value=FeatureValue( + type_=spec.value_type, value=spec.feature_state_value + ), segment_id=segment.id, multivariate_values=spec.multivariate_values, ), @@ -699,11 +705,10 @@ def enable_experiment_rollout(experiment: Experiment, author: AuthorData) -> Non value = rollout["feature_state_value"] _update_rollout_in_place( experiment, - FlagChangeSet( + FlagChangeSetOptionA( author=author, enabled=True, - feature_state_value=value["value"], - type_=value["type"], + value=FeatureValue(type_=value["type"], value=value["value"]), segment_id=experiment.rollout_segment_id, ), ) diff --git a/api/features/feature_states/serializers.py b/api/features/feature_states/serializers.py index df20853d176a..a908924c17ee 100644 --- a/api/features/feature_states/serializers.py +++ b/api/features/feature_states/serializers.py @@ -1,25 +1,46 @@ -from typing import Any +from collections.abc import Sequence +from typing import TypeVar, cast from rest_framework import serializers from core.dataclasses import AuthorData from environments.models import Environment +from features.feature_states.types import ( + DeleteSegmentOverridePayload, + EnvironmentDefaultPayload, + EnvironmentMultivariateValuePayload, + FeatureIdentifierPayload, + FeatureValuePayload, + SegmentIdentifierPayload, + SegmentOverrideMultivariateValuePayload, + SegmentOverridePayload, + SegmentReferencePayload, + UpdateFlagOptionAPayload, + UpdateFlagOptionBPayload, +) from features.models import Feature, FeatureState from features.versioning.dataclasses import ( - FlagChangeSet, - FlagChangeSetV2, + EnvironmentDefaultChangeSet, + EnvironmentMultivariateValueChangeSet, + FeatureValue, + FlagChangeSetOptionA, + FlagChangeSetOptionB, + MultivariateOptionUpdateChangeSet, MultivariateValueChangeSet, + NewMultivariateOptionChangeSet, SegmentOverrideChangeSet, ) from features.versioning.versioning_service import ( delete_segment_override, - update_flag, - update_flag_v2, + update_flag_option_a, + update_flag_option_b, ) from segments.models import Segment +_IN = TypeVar("_IN") + -class BaseFeatureUpdateSerializer(serializers.Serializer): # type: ignore[type-arg] +class BaseFeatureUpdateSerializer(serializers.Serializer[_IN]): @property def environment(self) -> Environment: environment: Environment | None = self.context.get("environment") @@ -28,7 +49,7 @@ def environment(self) -> Environment: return environment def get_feature(self) -> Feature: - feature_data = self.validated_data["feature"] + feature_data: FeatureIdentifierPayload = self.validated_data["feature"] try: feature: Feature = Feature.objects.get( project_id=self.environment.project_id, **feature_data @@ -48,36 +69,36 @@ def validate_segment_id(self, segment_id: int) -> None: ) -class FeatureIdentifierSerializer(serializers.Serializer): # type: ignore[type-arg] +class FeatureIdentifierSerializer(serializers.Serializer[FeatureIdentifierPayload]): 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 + def validate(self, attrs: FeatureIdentifierPayload) -> FeatureIdentifierPayload: + has_name = "name" in attrs + has_id = "id" in attrs 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 + return attrs -class FeatureUpdateSegmentDataSerializer(serializers.Serializer): # type: ignore[type-arg] +class SegmentReferenceSerializer(serializers.Serializer[SegmentReferencePayload]): id = serializers.IntegerField(required=True) priority = serializers.IntegerField(required=False, allow_null=True) -class FeatureValueSerializer(serializers.Serializer): # type: ignore[type-arg] +class FeatureValueSerializer(serializers.Serializer[FeatureValuePayload]): 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"] + def validate(self, attrs: FeatureValuePayload) -> FeatureValuePayload: + value_type = attrs["type"] + string_val = attrs["value"] if value_type == "integer": try: @@ -92,86 +113,228 @@ def validate(self, data: dict) -> dict: # type: ignore[type-arg] f"'{string_val}' is not a valid boolean (use 'true' or 'false')" ) - return data + return attrs -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, - ) +class BaseMultivariateValueSerializer(serializers.Serializer[_IN]): + percentage_allocation = serializers.FloatField( + required=True, min_value=0, max_value=100 + ) - def save(self, **kwargs: object) -> FeatureState: - feature = self.get_feature() - return update_flag(self.environment, feature, self.flag_change_set) +class SegmentOverrideMultivariateValueSerializer( + BaseMultivariateValueSerializer[SegmentOverrideMultivariateValuePayload] +): + multivariate_feature_option = serializers.IntegerField(required=True) + value = FeatureValueSerializer(required=False) # Raises ValidationError if provided -class EnvironmentDefaultSerializer(serializers.Serializer): # type: ignore[type-arg] - enabled = serializers.BooleanField(required=True) - value = FeatureValueSerializer(required=True) +class EnvironmentMultivariateValueSerializer( + BaseMultivariateValueSerializer[EnvironmentMultivariateValuePayload] +): + multivariate_feature_option = serializers.IntegerField(required=False) + value = FeatureValueSerializer(required=False) -class MultivariateValueSerializer(serializers.Serializer): # type: ignore[type-arg] - multivariate_feature_option = serializers.IntegerField(required=True) - percentage_allocation = serializers.FloatField( - required=True, min_value=0, max_value=100 - ) + def validate( + self, attrs: EnvironmentMultivariateValuePayload + ) -> EnvironmentMultivariateValuePayload: + if "multivariate_feature_option" not in attrs and "value" not in attrs: + raise serializers.ValidationError( + "A new multivariate option requires a 'value'." + ) + return attrs def validate_multivariate_state_values( - feature: Feature, multivariate_values: list[dict[str, Any]] + feature: Feature, + multivariate_values: Sequence[ + EnvironmentMultivariateValuePayload | SegmentOverrideMultivariateValuePayload + ], ) -> None: - if not multivariate_values: + option_ids = [ + mv["multivariate_feature_option"] + for mv in multivariate_values + if "multivariate_feature_option" in mv + ] + if not option_ids: return - option_ids = [mv["multivariate_feature_option"] for mv in multivariate_values] if len(option_ids) != len(set(option_ids)): - raise serializers.ValidationError("Multivariate options must be unique") + raise serializers.ValidationError( + { + "multivariate_feature_state_values": [ + "Multivariate options must be unique" + ] + } + ) valid = set(feature.multivariate_options.values_list("id", flat=True)) if invalid := set(option_ids) - valid: raise serializers.ValidationError( - f"Multivariate options {sorted(invalid)} do not belong to the feature" + { + "multivariate_feature_state_values": [ + f"Multivariate options {sorted(invalid)} do not belong to the feature" + ] + } + ) + + +def _to_environment_multivariate_value_change_set( + multivariate_value: EnvironmentMultivariateValuePayload, +) -> EnvironmentMultivariateValueChangeSet: + if "multivariate_feature_option" in multivariate_value: + value = multivariate_value.get("value") + return MultivariateOptionUpdateChangeSet( + multivariate_feature_option_id=multivariate_value[ + "multivariate_feature_option" + ], + percentage_allocation=multivariate_value["percentage_allocation"], + value=( + FeatureValue(type_=value["type"], value=value["value"]) + if value is not None + else None + ), ) + value = multivariate_value["value"] + return NewMultivariateOptionChangeSet( + percentage_allocation=multivariate_value["percentage_allocation"], + value=FeatureValue(type_=value["type"], value=value["value"]), + ) + + +class UpdateFlagOptionASerializer(BaseFeatureUpdateSerializer[FeatureState]): + feature = FeatureIdentifierSerializer(required=True) + segment = SegmentReferenceSerializer(required=False) + enabled = serializers.BooleanField(required=False) + value = FeatureValueSerializer(required=False) + multivariate_feature_state_values = EnvironmentMultivariateValueSerializer( + many=True, required=False + ) + + def validate_segment( + self, value: SegmentReferencePayload + ) -> SegmentReferencePayload: + self.validate_segment_id(value["id"]) + return value + + def validate(self, attrs: UpdateFlagOptionAPayload) -> UpdateFlagOptionAPayload: + multivariate_values = attrs.get("multivariate_feature_state_values") + if multivariate_values is None: + return attrs + + if "segment" in attrs: + if any( + "multivariate_feature_option" not in mv for mv in multivariate_values + ): + raise serializers.ValidationError( + { + "multivariate_feature_state_values": [ + "Segment overrides require a variant 'id'." + ] + } + ) + if any("value" in mv for mv in multivariate_values): + raise serializers.ValidationError( + { + "multivariate_feature_state_values": [ + "Segment overrides can only re-weight existing variants." + ] + } + ) + elif sum(mv["percentage_allocation"] for mv in multivariate_values) > 100: + raise serializers.ValidationError( + { + "multivariate_feature_state_values": [ + "Multivariate percentage values exceed 100%." + ] + } + ) + try: + feature = Feature.objects.get( + project_id=self.environment.project_id, **attrs["feature"] + ) + except Feature.DoesNotExist: + return attrs + validate_multivariate_state_values(feature, multivariate_values) -class SegmentOverrideSerializer(serializers.Serializer): # type: ignore[type-arg] + return attrs + + @property + def change_set_option_a(self) -> FlagChangeSetOptionA: + validated_data = cast(UpdateFlagOptionAPayload, self.validated_data) + value_data = validated_data.get("value") + segment_data = validated_data.get("segment") + multivariate_values = validated_data.get("multivariate_feature_state_values") + + segment_multivariate_values: list[MultivariateValueChangeSet] | None = None + environment_multivariate_values: ( + list[EnvironmentMultivariateValueChangeSet] | None + ) = None + if multivariate_values is not None: + if segment_data is not None: + segment_multivariate_values = [ + MultivariateValueChangeSet( + multivariate_feature_option_id=mv[ + "multivariate_feature_option" + ], + percentage_allocation=mv["percentage_allocation"], + ) + for mv in multivariate_values + if "multivariate_feature_option" in mv + ] + else: + environment_multivariate_values = [ + _to_environment_multivariate_value_change_set(mv) + for mv in multivariate_values + ] + + return FlagChangeSetOptionA( + author=AuthorData.from_request(self.context["request"]), + enabled=validated_data.get("enabled"), + value=( + FeatureValue(type_=value_data["type"], value=value_data["value"]) + if value_data is not None + else None + ), + segment_id=segment_data["id"] if segment_data is not None else None, + segment_priority=( + segment_data.get("priority") if segment_data is not None else None + ), + multivariate_values=segment_multivariate_values, + environment_multivariate_values=environment_multivariate_values, + ) + + def save(self, **kwargs: object) -> FeatureState: + feature = self.get_feature() + return update_flag_option_a(self.environment, feature, self.change_set_option_a) + + +class EnvironmentDefaultSerializer(serializers.Serializer[EnvironmentDefaultPayload]): + enabled = serializers.BooleanField(required=False) + value = FeatureValueSerializer(required=False) + multivariate_feature_state_values = EnvironmentMultivariateValueSerializer( + many=True, required=False + ) + + +class SegmentOverrideSerializer(serializers.Serializer[SegmentOverridePayload]): segment_id = serializers.IntegerField(required=True) priority = serializers.IntegerField(required=False, allow_null=True) - enabled = serializers.BooleanField(required=True) - value = FeatureValueSerializer(required=True) - multivariate_feature_state_values = MultivariateValueSerializer( + enabled = serializers.BooleanField(required=False) + value = FeatureValueSerializer(required=False) + multivariate_feature_state_values = SegmentOverrideMultivariateValueSerializer( many=True, required=False ) -class UpdateFlagV2Serializer(BaseFeatureUpdateSerializer): +class UpdateFlagOptionBSerializer(BaseFeatureUpdateSerializer[None]): feature = FeatureIdentifierSerializer(required=True) - environment_default = EnvironmentDefaultSerializer(required=True) + environment_default = EnvironmentDefaultSerializer(required=False) 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] + value: list[SegmentOverridePayload], + ) -> list[SegmentOverridePayload]: if not value: return value @@ -187,83 +350,170 @@ def validate_segment_overrides( return value - def validate(self, data: dict) -> dict: # type: ignore[type-arg] - overrides = data.get("segment_overrides", []) - if any(o.get("multivariate_feature_state_values") for o in overrides): - feature = Feature.objects.filter( - project_id=self.environment.project_id, **data["feature"] - ).first() - if feature is not None: - for override in overrides: - validate_multivariate_state_values( - feature, - override.get("multivariate_feature_state_values", []), + def validate(self, attrs: UpdateFlagOptionBPayload) -> UpdateFlagOptionBPayload: + multivariate_value_lists: list[ + Sequence[ + EnvironmentMultivariateValuePayload + | SegmentOverrideMultivariateValuePayload + ] + ] = [] + + environment_option_ids: set[int] | None = None + if (environment_default := attrs.get("environment_default")) is not None: + environment_default_multivariate_values = environment_default.get( + "multivariate_feature_state_values" + ) + if environment_default_multivariate_values is not None: + if ( + sum( + mv["percentage_allocation"] + for mv in environment_default_multivariate_values ) - return data + > 100 + ): + raise serializers.ValidationError( + { + "multivariate_feature_state_values": [ + "Multivariate percentage values exceed 100%." + ] + } + ) + environment_option_ids = { + mv["multivariate_feature_option"] + for mv in environment_default_multivariate_values + if "multivariate_feature_option" in mv + } + multivariate_value_lists.append(environment_default_multivariate_values) + + for override in attrs.get("segment_overrides") or []: + override_multivariate_values = override.get( + "multivariate_feature_state_values" + ) + if override_multivariate_values is None: + continue + if any("value" in mv for mv in override_multivariate_values) or ( + # The environment list is absolute: it deletes unlisted options. + environment_option_ids is not None + and any( + mv["multivariate_feature_option"] not in environment_option_ids + for mv in override_multivariate_values + ) + ): + raise serializers.ValidationError( + { + "multivariate_feature_state_values": [ + "Segment overrides can only re-weight existing variants." + ] + } + ) + multivariate_value_lists.append(override_multivariate_values) - @property - def change_set_v2(self) -> FlagChangeSetV2: - validated_data = self.validated_data + if not multivariate_value_lists: + return attrs - env_default = validated_data["environment_default"] - env_value_data = env_default["value"] + try: + feature = Feature.objects.get( + project_id=self.environment.project_id, **attrs["feature"] + ) + except Feature.DoesNotExist: + return attrs + for multivariate_values in multivariate_value_lists: + validate_multivariate_state_values(feature, multivariate_values) - segment_overrides_data = validated_data.get("segment_overrides", []) - segment_overrides = [] + return attrs - for override_data in segment_overrides_data: - value_data = override_data["value"] - - multivariate_data = override_data.get("multivariate_feature_state_values") - 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"), - multivariate_values=[ - MultivariateValueChangeSet( - multivariate_feature_option_id=mv[ - "multivariate_feature_option" - ], - percentage_allocation=mv["percentage_allocation"], - ) - for mv in multivariate_data - ] - if multivariate_data - else None, + @property + def change_set_option_b(self) -> FlagChangeSetOptionB: + validated_data = cast(UpdateFlagOptionBPayload, self.validated_data) + + environment_default_data = validated_data.get("environment_default") + environment_default: EnvironmentDefaultChangeSet | None = None + if environment_default_data is not None: + value_data = environment_default_data.get("value") + multivariate_data = environment_default_data.get( + "multivariate_feature_state_values" + ) + environment_default = EnvironmentDefaultChangeSet( + enabled=environment_default_data.get("enabled"), + value=( + FeatureValue(type_=value_data["type"], value=value_data["value"]) + if value_data is not None + else None + ), + multivariate_values=( + [ + _to_environment_multivariate_value_change_set(mv) + for mv in multivariate_data + ] + if multivariate_data is not None + else None + ), + ) + + segment_overrides = [] + for override_data in validated_data.get("segment_overrides") or []: + override_value_data = override_data.get("value") + override_multivariate_data = override_data.get( + "multivariate_feature_state_values" + ) + segment_overrides.append( + SegmentOverrideChangeSet( + segment_id=override_data["segment_id"], + enabled=override_data.get("enabled"), + value=( + FeatureValue( + type_=override_value_data["type"], + value=override_value_data["value"], + ) + if override_value_data is not None + else None + ), + priority=override_data.get("priority"), + multivariate_values=( + [ + MultivariateValueChangeSet( + multivariate_feature_option_id=mv[ + "multivariate_feature_option" + ], + percentage_allocation=mv["percentage_allocation"], + ) + for mv in override_multivariate_data + ] + if override_multivariate_data is not None + else None + ), + ) ) - segment_overrides.append(segment_override) - return FlagChangeSetV2( + return FlagChangeSetOptionB( 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"], + environment_default=environment_default, segment_overrides=segment_overrides, ) def save(self, **kwargs: object) -> None: feature = self.get_feature() - update_flag_v2(self.environment, feature, self.change_set_v2) + update_flag_option_b(self.environment, feature, self.change_set_option_b) -class SegmentIdentifierSerializer(serializers.Serializer): # type: ignore[type-arg] +class SegmentIdentifierSerializer(serializers.Serializer[SegmentIdentifierPayload]): id = serializers.IntegerField(required=True) -class DeleteSegmentOverrideSerializer(BaseFeatureUpdateSerializer): +class DeleteSegmentOverrideSerializer(BaseFeatureUpdateSerializer[None]): 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"]) + def validate_segment( + self, value: SegmentIdentifierPayload + ) -> SegmentIdentifierPayload: + 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"] + validated_data = cast(DeleteSegmentOverridePayload, self.validated_data) + segment_id = 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/types.py b/api/features/feature_states/types.py new file mode 100644 index 000000000000..eb2446810547 --- /dev/null +++ b/api/features/feature_states/types.py @@ -0,0 +1,91 @@ +from collections.abc import Sequence +from typing import Never, NotRequired, TypeAlias, TypedDict + +from features.feature_states.models import FeatureValueType + + +class FeatureValuePayload(TypedDict): + type: FeatureValueType + value: str + + +class FeatureNamePayload(TypedDict): + name: str + + +class FeatureIdPayload(TypedDict): + id: int + + +FeatureIdentifierPayload: TypeAlias = FeatureNamePayload | FeatureIdPayload + + +class SegmentReferencePayload(TypedDict): + id: int + priority: NotRequired[int | None] + + +class SegmentIdentifierPayload(TypedDict): + id: int + + +class BaseMultivariateValuePayload(TypedDict): + percentage_allocation: float + + +class NewMultivariateOptionPayload(BaseMultivariateValuePayload): + multivariate_feature_option: NotRequired[Never] + value: FeatureValuePayload + + +class MultivariateOptionUpdatePayload(BaseMultivariateValuePayload): + multivariate_feature_option: int + value: NotRequired[FeatureValuePayload] + + +EnvironmentMultivariateValuePayload: TypeAlias = ( + NewMultivariateOptionPayload | MultivariateOptionUpdatePayload +) + + +class SegmentOverrideMultivariateValuePayload(BaseMultivariateValuePayload): + multivariate_feature_option: int + + +class UpdateFlagOptionAPayload(TypedDict): + feature: FeatureIdentifierPayload + segment: NotRequired[SegmentReferencePayload] + enabled: NotRequired[bool] + value: NotRequired[FeatureValuePayload] + multivariate_feature_state_values: NotRequired[ + Sequence[EnvironmentMultivariateValuePayload] + ] + + +class EnvironmentDefaultPayload(TypedDict): + enabled: NotRequired[bool] + value: NotRequired[FeatureValuePayload] + multivariate_feature_state_values: NotRequired[ + Sequence[EnvironmentMultivariateValuePayload] + ] + + +class SegmentOverridePayload(TypedDict): + segment_id: int + priority: NotRequired[int | None] + enabled: NotRequired[bool] + value: NotRequired[FeatureValuePayload] + multivariate_feature_state_values: NotRequired[ + list[SegmentOverrideMultivariateValuePayload] + ] + + +class UpdateFlagOptionBPayload(TypedDict): + feature: FeatureIdentifierPayload + environment_default: NotRequired[EnvironmentDefaultPayload] + segment_overrides: NotRequired[list[SegmentOverridePayload]] + + +class DeleteSegmentOverridePayload(TypedDict): + feature: FeatureIdentifierPayload + segment: SegmentIdentifierPayload diff --git a/api/features/feature_states/views.py b/api/features/feature_states/views.py index e3d5208005eb..e9338d7c36ff 100644 --- a/api/features/feature_states/views.py +++ b/api/features/feature_states/views.py @@ -11,8 +11,8 @@ from .permissions import EnvironmentUpdateFeatureStatePermission from .serializers import ( DeleteSegmentOverrideSerializer, - UpdateFlagSerializer, - UpdateFlagV2Serializer, + UpdateFlagOptionASerializer, + UpdateFlagOptionBSerializer, ) @@ -58,17 +58,17 @@ def _check_workflow_not_enabled(environment: Environment) -> None: required=True, ) ], - request=UpdateFlagSerializer, + request=UpdateFlagOptionASerializer, responses={204: None}, tags=["experimental"], ) @api_view(http_method_names=["POST"]) @permission_classes([IsAuthenticated, EnvironmentUpdateFeatureStatePermission]) -def update_flag_v1(request: Request, environment_key: str) -> Response: +def update_flag_option_a(request: Request, environment_key: str) -> Response: environment = Environment.objects.get(api_key=environment_key) _check_workflow_not_enabled(environment) - serializer = UpdateFlagSerializer( + serializer = UpdateFlagOptionASerializer( data=request.data, context={"request": request, "environment": environment}, ) @@ -120,17 +120,17 @@ def update_flag_v1(request: Request, environment_key: str) -> Response: required=True, ) ], - request=UpdateFlagV2Serializer, + request=UpdateFlagOptionBSerializer, responses={204: None}, tags=["experimental"], ) @api_view(http_method_names=["POST"]) @permission_classes([IsAuthenticated, EnvironmentUpdateFeatureStatePermission]) -def update_flag_v2(request: Request, environment_key: str) -> Response: +def update_flag_option_b(request: Request, environment_key: str) -> Response: environment = Environment.objects.get(api_key=environment_key) _check_workflow_not_enabled(environment) - serializer = UpdateFlagV2Serializer( + serializer = UpdateFlagOptionBSerializer( data=request.data, context={"request": request, "environment": environment}, ) diff --git a/api/features/models.py b/api/features/models.py index e30a6bbb9fda..7409b9c6e0b1 100644 --- a/api/features/models.py +++ b/api/features/models.py @@ -318,12 +318,8 @@ def __str__(self): # type: ignore[no-untyped-def] + str(self.priority) ) - def __lt__(self, other): # type: ignore[no-untyped-def] - """ - Kind of counter intuitive but since priority 1 is highest, we want to check if priority is GREATER than the - priority of the other feature segment. - """ - return other and self.priority > other.priority + def __lt__(self, other: "FeatureSegment") -> bool: + return bool(other) and self.priority > other.priority # Lowest wins def clone( self, diff --git a/api/features/versioning/dataclasses.py b/api/features/versioning/dataclasses.py index 65fd78dacf70..96b646c87868 100644 --- a/api/features/versioning/dataclasses.py +++ b/api/features/versioning/dataclasses.py @@ -2,6 +2,7 @@ from dataclasses import dataclass, field from datetime import datetime +from typing import TypeAlias from pydantic import BaseModel, computed_field @@ -21,15 +22,23 @@ def is_environment_default(self) -> bool: @dataclass -class FlagChangeSet: - author: AuthorData - enabled: bool - feature_state_value: str +class FeatureValue: type_: FeatureValueType + value: str + + +@dataclass +class FlagChangeSetOptionA: + author: AuthorData + enabled: bool | None = None + value: FeatureValue | None = None segment_id: int | None = None segment_priority: int | None = None multivariate_values: list[MultivariateValueChangeSet] | None = None + environment_multivariate_values: ( + list[EnvironmentMultivariateValueChangeSet] | None + ) = None @dataclass @@ -38,21 +47,43 @@ class MultivariateValueChangeSet: percentage_allocation: float +@dataclass +class NewMultivariateOptionChangeSet: + percentage_allocation: float + value: FeatureValue + + +@dataclass +class MultivariateOptionUpdateChangeSet: + multivariate_feature_option_id: int + percentage_allocation: float + value: FeatureValue | None = None + + +EnvironmentMultivariateValueChangeSet: TypeAlias = ( + NewMultivariateOptionChangeSet | MultivariateOptionUpdateChangeSet +) + + @dataclass class SegmentOverrideChangeSet: segment_id: int - enabled: bool - feature_state_value: str - type_: FeatureValueType + enabled: bool | None = None + value: FeatureValue | None = None priority: int | None = None multivariate_values: list[MultivariateValueChangeSet] | None = None @dataclass -class FlagChangeSetV2: +class EnvironmentDefaultChangeSet: + enabled: bool | None = None + value: FeatureValue | None = None + multivariate_values: list[EnvironmentMultivariateValueChangeSet] | None = None + + +@dataclass +class FlagChangeSetOptionB: author: AuthorData - environment_default_enabled: bool - environment_default_value: str - environment_default_type: FeatureValueType + environment_default: EnvironmentDefaultChangeSet | None = None 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 1428faee7d5e..77edc80a5187 100644 --- a/api/features/versioning/versioning_service.py +++ b/api/features/versioning/versioning_service.py @@ -1,19 +1,27 @@ import typing from common.core.utils import using_database_replica +from django.db import transaction from django.db.models import Prefetch, Q, QuerySet from django.utils import timezone from rest_framework.exceptions import NotFound, ValidationError from core.dataclasses import AuthorData from environments.models import Environment -from features.feature_states.models import FeatureValueType from features.models import Feature, FeatureSegment, FeatureState, FeatureStateValue -from features.multivariate.models import MultivariateFeatureStateValue +from features.multivariate.models import ( + MultivariateFeatureOption, + MultivariateFeatureStateValue, +) from features.versioning.dataclasses import ( - FlagChangeSet, - FlagChangeSetV2, + EnvironmentDefaultChangeSet, + EnvironmentMultivariateValueChangeSet, + FeatureValue, + FlagChangeSetOptionA, + FlagChangeSetOptionB, + MultivariateOptionUpdateChangeSet, MultivariateValueChangeSet, + NewMultivariateOptionChangeSet, ) from features.versioning.exceptions import DirectFeatureStateWriteNotAllowedError from features.versioning.models import EnvironmentFeatureVersion @@ -131,17 +139,22 @@ def get_current_live_environment_feature_version( ) -def update_flag( - environment: Environment, feature: Feature, change_set: FlagChangeSet +def update_flag_option_a( + environment: Environment, feature: Feature, change_set: FlagChangeSetOptionA ) -> 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) + with transaction.atomic(): + if environment.use_v2_feature_versioning: + return _update_flag_option_a_for_versioning_v2( + environment, feature, change_set + ) + else: + return _update_flag_option_a_for_versioning_v1( + environment, feature, change_set + ) -def _update_flag_for_versioning_v2( - environment: Environment, feature: Feature, change_set: FlagChangeSet +def _update_flag_option_a_for_versioning_v2( + environment: Environment, feature: Feature, change_set: FlagChangeSetOptionA ) -> FeatureState: from features.models import FeatureSegment, FeatureState @@ -166,13 +179,22 @@ def _update_flag_for_versioning_v2( environment_feature_version=new_version, ) + enabled, inherited_value = _resolve_new_override_inheritance( + change_set.enabled, + change_set.value, + lambda: new_version.feature_states.get( + feature_segment__isnull=True, identity_id=None + ), + ) target_feature_state = FeatureState.objects.create( feature=feature, environment=environment, feature_segment=feature_segment, environment_feature_version=new_version, - enabled=change_set.enabled, + enabled=enabled, ) + if inherited_value is not None: + target_feature_state.feature_state_value.copy_from(inherited_value) else: # Environment default - always exists target_feature_state = new_version.feature_states.get( @@ -180,15 +202,21 @@ def _update_flag_for_versioning_v2( identity_id=None, ) - target_feature_state.enabled = change_set.enabled - target_feature_state.save() + if change_set.enabled is not 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_, - ) - update_multivariate_values(target_feature_state, change_set.multivariate_values) + if change_set.value is not None: + _update_feature_state_value( + target_feature_state.feature_state_value, change_set.value + ) + + if change_set.segment_id is not None: + update_multivariate_values(target_feature_state, change_set.multivariate_values) + else: + update_environment_multivariate_options( + target_feature_state, feature, change_set.environment_multivariate_values + ) 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) @@ -201,8 +229,8 @@ def _update_flag_for_versioning_v2( return target_feature_state -def _update_flag_for_versioning_v1( - environment: Environment, feature: Feature, change_set: FlagChangeSet +def _update_flag_option_a_for_versioning_v1( + environment: Environment, feature: Feature, change_set: FlagChangeSetOptionA ) -> FeatureState: from features.models import FeatureSegment, FeatureState @@ -224,24 +252,37 @@ def _update_flag_for_versioning_v1( environment=environment, ) + enabled, inherited_value = _resolve_new_override_inheritance( + change_set.enabled, + change_set.value, + lambda: _get_environment_default_feature_state(environment, feature), + ) target_feature_state: FeatureState = FeatureState.objects.create( feature=feature, environment=environment, feature_segment=feature_segment, - enabled=change_set.enabled, + enabled=enabled, ) + if inherited_value is not None: + target_feature_state.feature_state_value.copy_from(inherited_value) 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() + if change_set.enabled is not 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_, - ) - update_multivariate_values(target_feature_state, change_set.multivariate_values) + if change_set.value is not None: + _update_feature_state_value( + target_feature_state.feature_state_value, change_set.value + ) + + if change_set.segment_id is not None: + update_multivariate_values(target_feature_state, change_set.multivariate_values) + else: + update_environment_multivariate_options( + target_feature_state, feature, change_set.environment_multivariate_values + ) 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) @@ -249,10 +290,38 @@ def _update_flag_for_versioning_v1( return target_feature_state -def _update_feature_state_value( - fsv: FeatureStateValue, value: str, type_: FeatureValueType -) -> None: - fsv.set_value(value, type_) +def _get_environment_default_feature_state( + environment: Environment, feature: Feature +) -> FeatureState: + environment_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(environment_default_states) == 1 + return list(environment_default_states.values())[0] + + +def _resolve_new_override_inheritance( + enabled: bool | None, + value: FeatureValue | None, + get_environment_default: typing.Callable[[], FeatureState], +) -> tuple[bool, FeatureStateValue | None]: + """ + Resolve a new segment override's enabled flag and, when its value is + omitted, the environment default FeatureStateValue to copy it from. + """ + if enabled is not None and value is not None: + return enabled, None + environment_default = get_environment_default() + return ( + enabled if enabled is not None else environment_default.enabled, + environment_default.feature_state_value if value is None else None, + ) + + +def _update_feature_state_value(fsv: FeatureStateValue, value: FeatureValue) -> None: + fsv.set_value(value.value, value.type_) fsv.save() @@ -293,6 +362,54 @@ def update_multivariate_values( mv.save() +def update_environment_multivariate_options( + feature_state: FeatureState, + feature: Feature, + values: list[EnvironmentMultivariateValueChangeSet] | None, +) -> None: + """ + Reconcile the feature's multivariate options with the given absolute list, + then re-weight the given environment default feature state accordingly. + """ + if values is None: + return + + listed_option_ids = { + value.multivariate_feature_option_id + for value in values + if isinstance(value, MultivariateOptionUpdateChangeSet) + } + # Instance-level deletes so django-lifecycle hooks fire + for option in feature.multivariate_options.exclude(id__in=listed_option_ids): + option.delete() + + reweighted_values = [] + for value in values: + if isinstance(value, NewMultivariateOptionChangeSet): + option = MultivariateFeatureOption( + feature=feature, + default_percentage_allocation=value.percentage_allocation, + ) + option.set_value(value.value.value, value.value.type_) + option.save() + else: + option = feature.multivariate_options.get( + id=value.multivariate_feature_option_id + ) + option.default_percentage_allocation = value.percentage_allocation + if value.value is not None: + option.set_value(value.value.value, value.value.type_) + option.save() + reweighted_values.append( + MultivariateValueChangeSet( + multivariate_feature_option_id=option.id, + percentage_allocation=value.percentage_allocation, + ) + ) + + update_multivariate_values(feature_state, reweighted_values) + + def _create_segment_override( feature: Feature, environment: Environment, @@ -330,17 +447,37 @@ def _update_segment_priority(feature_state: FeatureState, priority: int) -> None feature_segment.to(priority) -def update_flag_v2( - environment: Environment, feature: Feature, change_set: FlagChangeSetV2 +def update_flag_option_b( + environment: Environment, feature: Feature, change_set: FlagChangeSetOptionB ) -> 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) + with transaction.atomic(): + if environment.use_v2_feature_versioning: + _update_flag_option_b_for_versioning_v2(environment, feature, change_set) + else: + _update_flag_option_b_for_versioning_v1(environment, feature, change_set) -def _update_flag_v2_for_versioning_v2( - environment: Environment, feature: Feature, change_set: FlagChangeSetV2 +def _apply_environment_default_change_set( + env_default_state: FeatureState, + feature: Feature, + environment_default: EnvironmentDefaultChangeSet, +) -> None: + if environment_default.enabled is not None: + env_default_state.enabled = environment_default.enabled + env_default_state.save() + + if environment_default.value is not None: + _update_feature_state_value( + env_default_state.feature_state_value, environment_default.value + ) + + update_environment_multivariate_options( + env_default_state, feature, environment_default.multivariate_values + ) + + +def _update_flag_option_b_for_versioning_v2( + environment: Environment, feature: Feature, change_set: FlagChangeSetOptionB ) -> None: new_version = EnvironmentFeatureVersion.objects.create( environment=environment, @@ -349,50 +486,55 @@ def _update_flag_v2_for_versioning_v2( 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() + def get_environment_default() -> FeatureState: + environment_default: FeatureState = new_version.feature_states.get( + feature_segment__isnull=True, identity_id=None + ) + return environment_default - _update_feature_state_value( - env_default_state.feature_state_value, - change_set.environment_default_value, - change_set.environment_default_type, - ) + if change_set.environment_default is not None: + _apply_environment_default_change_set( + get_environment_default(), feature, change_set.environment_default + ) 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.enabled is not None: + segment_state.enabled = override.enabled + segment_state.save() + + if override.value is not None: + _update_feature_state_value( + segment_state.feature_state_value, override.value + ) update_multivariate_values(segment_state, override.multivariate_values) if override.priority is not None: _update_segment_priority(segment_state, override.priority) except FeatureState.DoesNotExist: + enabled, inherited_value = _resolve_new_override_inheritance( + override.enabled, + override.value, + get_environment_default, + ) segment_state = _create_segment_override( feature=feature, environment=environment, segment_id=override.segment_id, - enabled=override.enabled, + enabled=enabled, priority=override.priority, version=new_version, ) - _update_feature_state_value( - segment_state.feature_state_value, - override.feature_state_value, - override.type_, - ) + if override.value is not None: + _update_feature_state_value( + segment_state.feature_state_value, override.value + ) + elif inherited_value is not None: + segment_state.feature_state_value.copy_from(inherited_value) update_multivariate_values(segment_state, override.multivariate_values) new_version.publish( @@ -401,25 +543,15 @@ def _update_flag_v2_for_versioning_v2( ) -def _update_flag_v2_for_versioning_v1( - environment: Environment, feature: Feature, change_set: FlagChangeSetV2 +def _update_flag_option_b_for_versioning_v1( + environment: Environment, feature: Feature, change_set: FlagChangeSetOptionB ) -> 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, - ) + if change_set.environment_default is not None: + _apply_environment_default_change_set( + _get_environment_default_feature_state(environment, feature), + feature, + change_set.environment_default, + ) for override in change_set.segment_overrides: # TODO: optimise this once this is out of the @@ -431,32 +563,38 @@ def _update_flag_v2_for_versioning_v1( ) if len(segment_states) == 0: + enabled, inherited_value = _resolve_new_override_inheritance( + override.enabled, + override.value, + lambda: _get_environment_default_feature_state(environment, feature), + ) segment_state = _create_segment_override( feature=feature, environment=environment, segment_id=override.segment_id, - enabled=override.enabled, + enabled=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_, - ) + if override.value is not None: + _update_feature_state_value( + segment_state.feature_state_value, override.value + ) + elif inherited_value is not None: + segment_state.feature_state_value.copy_from(inherited_value) update_multivariate_values(segment_state, override.multivariate_values) 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.enabled is not None: + segment_state.enabled = override.enabled + segment_state.save() + + if override.value is not None: + _update_feature_state_value( + segment_state.feature_state_value, override.value + ) update_multivariate_values(segment_state, override.multivariate_values) if override.priority is not None: diff --git a/api/tests/integration/conftest.py b/api/tests/integration/conftest.py index 1adc9aca6f81..18051756cd0c 100644 --- a/api/tests/integration/conftest.py +++ b/api/tests/integration/conftest.py @@ -17,6 +17,7 @@ from environments.enums import EnvironmentDocumentCacheMode from organisations.models import Organisation from tests.integration.helpers import create_mv_option_with_api +from users.models import FFAdminUser @pytest.fixture() @@ -30,12 +31,12 @@ def django_client(): # type: ignore[no-untyped-def] @pytest.fixture() -def api_client(): # type: ignore[no-untyped-def] +def api_client() -> APIClient: return APIClient() @pytest.fixture() -def admin_client(api_client, admin_user): # type: ignore[no-untyped-def] +def admin_client(api_client: APIClient, admin_user: FFAdminUser) -> APIClient: api_client.force_authenticate(user=admin_user) return api_client diff --git a/api/tests/integration/environments/identities/test_integration_identities.py b/api/tests/integration/environments/identities/test_integration_identities.py index 58e865ba3dbb..5be2e646b900 100644 --- a/api/tests/integration/environments/identities/test_integration_identities.py +++ b/api/tests/integration/environments/identities/test_integration_identities.py @@ -73,14 +73,14 @@ def test_get_feature_states_for_identity__mv_percentage_allocation__returns_corr create_mv_option_with_api( admin_client, project, - multivariate_feature_id, # type: ignore[arg-type] + multivariate_feature_id, variant_1_percentage_allocation, variant_1_value, ) variant_2_mvfo_id = create_mv_option_with_api( admin_client, project, - multivariate_feature_id, # type: ignore[arg-type] + multivariate_feature_id, variant_2_percentage_allocation, variant_2_value, ) @@ -193,7 +193,7 @@ def test_get_feature_states_for_identity__mv_allocation__returns_variant( # typ create_mv_option_with_api( admin_client, project, - multivariate_feature_id, # type: ignore[arg-type] + multivariate_feature_id, variant_1_percentage_allocation, variant_1_value, key="variant-1", @@ -201,7 +201,7 @@ def test_get_feature_states_for_identity__mv_allocation__returns_variant( # typ create_mv_option_with_api( admin_client, project, - multivariate_feature_id, # type: ignore[arg-type] + multivariate_feature_id, variant_2_percentage_allocation, variant_2_value, key="variant-2", @@ -242,7 +242,7 @@ def test_get_flags__multivariate_feature__response_excludes_variant( # type: ig create_mv_option_with_api( admin_client, project, - multivariate_feature_id, # type: ignore[arg-type] + multivariate_feature_id, 100, variant_1_value, key="variant-1", @@ -279,14 +279,14 @@ def test_get_feature_states_for_identity__multiple_mv_features__single_mv_query( create_mv_option_with_api( admin_client, project, - feature_id, # type: ignore[arg-type] + feature_id, variant_1_percentage_allocation, variant_1_value, ) create_mv_option_with_api( admin_client, project, - feature_id, # type: ignore[arg-type] + feature_id, variant_2_percentage_allocation, variant_2_value, ) @@ -314,14 +314,14 @@ def test_get_feature_states_for_identity__multiple_mv_features__single_mv_query( create_mv_option_with_api( admin_client, project, - feature_id, # type: ignore[arg-type] + feature_id, variant_1_percentage_allocation, variant_1_value, ) create_mv_option_with_api( admin_client, project, - feature_id, # type: ignore[arg-type] + feature_id, variant_2_percentage_allocation, variant_2_value, ) diff --git a/api/tests/integration/features/versioning/test_update_flag_endpoints.py b/api/tests/integration/features/versioning/test_update_flag_endpoints.py new file mode 100644 index 000000000000..afa59eca30da --- /dev/null +++ b/api/tests/integration/features/versioning/test_update_flag_endpoints.py @@ -0,0 +1,891 @@ +"""https://docs.flagsmith.com/integrating-with-flagsmith/flagsmith-api-overview/admin-api/updating-flags""" + +from collections.abc import Callable +from types import SimpleNamespace +from typing import Any, Literal, TypeAlias + +import pytest +from rest_framework.test import APIClient + +from environments.models import Environment +from features.feature_types import STANDARD +from features.models import Feature, FeatureState +from features.multivariate.models import MultivariateFeatureOption +from features.versioning.tasks import enable_v2_versioning +from tests.integration.helpers import create_mv_option_with_api + +FeatureUpdatePayload: TypeAlias = dict[str, Any] +UpdateFlagEndpointOption = Literal["update-flag-v1", "update-flag-v2"] + + +@pytest.fixture(params=["feature_versioning_v1", "feature_versioning_v2"], autouse=True) +def versioned_environment( + request: pytest.FixtureRequest, + environment: int, +) -> Environment: + if request.param == "feature_versioning_v2": + enable_v2_versioning(environment_id=environment) + return Environment.objects.get(id=environment) # type: ignore[no-any-return] + + +@pytest.mark.parametrize( + "endpoint,payload", + [ + pytest.param( + "update-flag-v1", + lambda feature: { + "feature": {"id": feature}, + "value": {"type": "string", "value": "control"}, + "multivariate_feature_state_values": [ + { + "percentage_allocation": 50, + "value": {"type": "string", "value": "half"}, + }, + { + "percentage_allocation": 10, + "value": {"type": "string", "value": "bit more"}, + }, + ], + }, + id="option_a", + ), + pytest.param( + "update-flag-v2", + lambda feature: { + "feature": {"id": feature}, + "environment_default": { + "value": {"type": "string", "value": "control"}, + "multivariate_feature_state_values": [ + { + "percentage_allocation": 50, + "value": {"type": "string", "value": "half"}, + }, + { + "percentage_allocation": 10, + "value": {"type": "string", "value": "bit more"}, + }, + ], + }, + }, + id="option_b", + ), + ], +) +def test_update_flag__environment_defaults__adds_multivariate_options( + admin_client: APIClient, + environment_api_key: str, + feature: int, + versioned_environment: Environment, + endpoint: UpdateFlagEndpointOption, + payload: Callable[[int], FeatureUpdatePayload], +) -> None: + # Given / When + response = admin_client.post( + f"/api/experiments/environments/{environment_api_key}/{endpoint}/", + payload(feature), + format="json", + ) + + # Then + assert response.status_code == 204 + assert dict( + FeatureState.objects.get_live_feature_states( + environment=versioned_environment, + feature_id=feature, + feature_segment=None, + ) + .get() + .multivariate_feature_state_values.values_list( + "multivariate_feature_option__string_value", "percentage_allocation" + ) + ) == {"half": 50, "bit more": 10} + + +@pytest.mark.parametrize( + "endpoint,payload", + [ + pytest.param( + "update-flag-v1", + lambda feature, option: { + "feature": {"id": feature}, + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": option, + "percentage_allocation": 25, + "value": {"type": "string", "value": "halfer"}, + }, + ], + }, + id="option_a", + ), + pytest.param( + "update-flag-v2", + lambda feature, option: { + "feature": {"id": feature}, + "environment_default": { + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": option, + "percentage_allocation": 25, + "value": {"type": "string", "value": "halfer"}, + }, + ], + }, + }, + id="option_b", + ), + ], +) +def test_update_flag__environment_defaults__updates_multivariate_options( + admin_client: APIClient, + environment_api_key: str, + feature: int, + mv_option_50_percent: int, + versioned_environment: Environment, + endpoint: UpdateFlagEndpointOption, + payload: Callable[[int, int], FeatureUpdatePayload], +) -> None: + # Given / When + response = admin_client.post( + f"/api/experiments/environments/{environment_api_key}/{endpoint}/", + payload(feature, mv_option_50_percent), + format="json", + ) + + # Then + assert response.status_code == 204 + assert dict( + FeatureState.objects.get_live_feature_states( + environment=versioned_environment, + feature_id=feature, + feature_segment=None, + ) + .get() + .multivariate_feature_state_values.values_list( + "multivariate_feature_option__string_value", "percentage_allocation" + ) + ) == {"halfer": 25} + assert ( + MultivariateFeatureOption.objects.get( + id=mv_option_50_percent + ).default_percentage_allocation + == 25 + ) + + +@pytest.mark.parametrize( + "endpoint,payload", + [ + pytest.param( + "update-flag-v1", + lambda feature, segment, option: { + "feature": {"id": feature}, + "segment": {"id": segment}, + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": option, + "percentage_allocation": 80, + }, + ], + }, + id="option_a", + ), + pytest.param( + "update-flag-v2", + lambda feature, segment, option: { + "feature": {"id": feature}, + "segment_overrides": [ + { + "segment_id": segment, + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": option, + "percentage_allocation": 80, + }, + ], + }, + ], + }, + id="option_b", + ), + ], +) +def test_update_flag__segment_override__updates_multivariate_options( + admin_client: APIClient, + environment_api_key: str, + feature: int, + mv_option_value: str, + mv_option_50_percent: int, + segment: int, + versioned_environment: Environment, + endpoint: UpdateFlagEndpointOption, + payload: Callable[[int, int, int], FeatureUpdatePayload], +) -> None: + # Given / When + response = admin_client.post( + f"/api/experiments/environments/{environment_api_key}/{endpoint}/", + payload(feature, segment, mv_option_50_percent), + format="json", + ) + + # Then + assert response.status_code == 204 + live_feature_states = FeatureState.objects.get_live_feature_states( + environment=versioned_environment, + feature_id=feature, + ) + assert dict( + live_feature_states.get( + feature_segment__segment_id=segment + ).multivariate_feature_state_values.values_list( + "multivariate_feature_option__string_value", "percentage_allocation" + ) + ) == {mv_option_value: 80} + assert dict( + live_feature_states.get( + feature_segment=None + ).multivariate_feature_state_values.values_list( + "multivariate_feature_option__string_value", "percentage_allocation" + ) + ) == {mv_option_value: 50} + + +@pytest.mark.parametrize( + "endpoint,payload", + [ + pytest.param( + "update-flag-v1", + lambda feature, kept_option: { + "feature": {"id": feature}, + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": kept_option, + "percentage_allocation": 50, + }, + ], + }, + id="option_a", + ), + pytest.param( + "update-flag-v2", + lambda feature, kept_option: { + "feature": {"id": feature}, + "environment_default": { + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": kept_option, + "percentage_allocation": 50, + }, + ], + }, + }, + id="option_b", + ), + ], +) +def test_update_flag__environment_defaults__deletes_multivariate_options( + admin_client: APIClient, + environment_api_key: str, + project: int, + feature: int, + mv_option_50_percent: int, + endpoint: UpdateFlagEndpointOption, + payload: Callable[[int, int], FeatureUpdatePayload], +) -> None: + # Given + deleted_option = create_mv_option_with_api( + admin_client, project, feature, 40, "other_mv_value" + ) + + # When + response = admin_client.post( + f"/api/experiments/environments/{environment_api_key}/{endpoint}/", + payload(feature, mv_option_50_percent), + format="json", + ) + + # Then + assert response.status_code == 204 + assert not MultivariateFeatureOption.objects.filter(id=deleted_option).exists() + assert MultivariateFeatureOption.objects.filter(id=mv_option_50_percent).exists() + + +@pytest.mark.parametrize( + "endpoint,payload", + [ + pytest.param( + "update-flag-v1", + lambda feature: { + "feature": {"id": feature}, + "multivariate_feature_state_values": [ + { + "percentage_allocation": 60, + "value": {"type": "string", "value": "more"}, + }, + { + "percentage_allocation": 50, + "value": {"type": "string", "value": "less"}, + }, + ], + }, + id="option_a", + ), + pytest.param( + "update-flag-v2", + lambda feature: { + "feature": {"id": feature}, + "environment_default": { + "multivariate_feature_state_values": [ + { + "percentage_allocation": 60, + "value": {"type": "string", "value": "more"}, + }, + { + "percentage_allocation": 50, + "value": {"type": "string", "value": "less"}, + }, + ], + }, + }, + id="option_b", + ), + ], +) +def test_update_flag__multivariate_percentage_allocation_exceeds_100__responds_400( + admin_client: APIClient, + environment_api_key: str, + feature: int, + endpoint: UpdateFlagEndpointOption, + payload: Callable[[int], FeatureUpdatePayload], +) -> None: + # Given / When + response = admin_client.post( + f"/api/experiments/environments/{environment_api_key}/{endpoint}/", + payload(feature), + format="json", + ) + + # Then + assert response.status_code == 400 + assert "multivariate_feature_state_values" in response.json() + assert not MultivariateFeatureOption.objects.filter(feature_id=feature).exists() + + +@pytest.mark.parametrize( + ["endpoint", "payload", "expected_errors"], + [ + test_case + for scenario in [ + SimpleNamespace( + id="add-without-value", + payload=lambda **_: { + "multivariate_feature_state_values": [ + {"percentage_allocation": 50} + ], + }, + expected_errors=[ + { # Option A + "multivariate_feature_state_values": [ + { + "non_field_errors": [ + "A new multivariate option requires a 'value'." + ] + } + ] + }, + { # Option B + "environment_default": { + "multivariate_feature_state_values": [ + { + "non_field_errors": [ + "A new multivariate option requires a 'value'." + ] + } + ] + } + }, + ], + ), + SimpleNamespace( + id="add-without-allocation", + payload=lambda **_: { + "multivariate_feature_state_values": [ + {"value": {"type": "string", "value": "variant"}}, + ], + }, + expected_errors=[ + { # Option A + "multivariate_feature_state_values": [ + {"percentage_allocation": ["This field is required."]} + ] + }, + { # Option B + "environment_default": { + "multivariate_feature_state_values": [ + {"percentage_allocation": ["This field is required."]} + ] + } + }, + ], + ), + SimpleNamespace( + id="unknown", + payload=lambda **_: { + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": 999999, + "percentage_allocation": 50, + }, + ], + }, + expected_errors=[ + { + "multivariate_feature_state_values": [ + "Multivariate options [999999] do not belong to the feature" + ] + }, + ], + ), + SimpleNamespace( + id="duplicate", + payload=lambda multivariate_option_id, **_: { + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": multivariate_option_id, + "percentage_allocation": 60, + }, + { + "multivariate_feature_option": multivariate_option_id, + "percentage_allocation": 40, + }, + ], + }, + expected_errors=[ + { + "multivariate_feature_state_values": [ + "Multivariate options must be unique" + ] + }, + ], + ), + ] + for test_case in [ + pytest.param( + "update-flag-v1", + scenario.payload, + scenario.expected_errors, + id=f"{scenario.id}-option_a", + ), + pytest.param( + "update-flag-v2", + lambda scenario=scenario, **kw: { + "environment_default": scenario.payload(**kw), + }, + scenario.expected_errors, + id=f"{scenario.id}-option_b", + ), + ] + ], +) +def test_update_flag__invalid_environment_multivariate_options__responds_400( + admin_client: APIClient, + endpoint: UpdateFlagEndpointOption, + environment_api_key: str, + expected_errors: list[Any], + feature: int, + mv_option_50_percent: int, + payload: Callable[..., FeatureUpdatePayload], +) -> None: + # Given / When + response = admin_client.post( + f"/api/experiments/environments/{environment_api_key}/{endpoint}/", + data={ + "feature": {"id": feature}, + **payload(multivariate_option_id=mv_option_50_percent), + }, + format="json", + ) + + # Then + assert response.status_code == 400 + assert response.json() in expected_errors + + +@pytest.mark.parametrize( + ["endpoint", "payload", "expected_errors"], + [ + test_case + for scenario in [ + SimpleNamespace( + id="without-id", + payload=lambda **_: { + "multivariate_feature_state_values": [ + { + "percentage_allocation": 50, + "value": {"type": "string", "value": "variant"}, + }, + ], + }, + expected_errors=[ + { # Option A + "multivariate_feature_state_values": [ + "Segment overrides require a variant 'id'." + ] + }, + { # Option B + "segment_overrides": [ + { + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": [ + "This field is required." + ] + } + ] + } + ] + }, + ], + ), + SimpleNamespace( + id="unknown-id", + payload=lambda **_: { + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": 999999, + "percentage_allocation": 50, + }, + ], + }, + expected_errors=[ + { + "multivariate_feature_state_values": [ + "Multivariate options [999999] do not belong to the feature" + ] + }, + ], + ), + SimpleNamespace( + id="duplicate", + payload=lambda multivariate_option_id, **_: { + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": multivariate_option_id, + "percentage_allocation": 60, + }, + { + "multivariate_feature_option": multivariate_option_id, + "percentage_allocation": 40, + }, + ], + }, + expected_errors=[ + { + "multivariate_feature_state_values": [ + "Multivariate options must be unique" + ] + }, + ], + ), + SimpleNamespace( + id="with-value", + payload=lambda multivariate_option_id, **_: { + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": multivariate_option_id, + "percentage_allocation": 50, + "value": {"type": "string", "value": "variant"}, + }, + ], + }, + expected_errors=[ + { + "multivariate_feature_state_values": [ + "Segment overrides can only re-weight existing variants.", + ], + }, + ], + ), + ] + for test_case in [ + pytest.param( + "update-flag-v1", + lambda segment_id, scenario=scenario, **kw: { + "segment": {"id": segment_id}, + **scenario.payload(**kw), + }, + scenario.expected_errors, + id=f"{scenario.id}-option_a", + ), + pytest.param( + "update-flag-v2", + lambda segment_id, scenario=scenario, **kw: { + "segment_overrides": [ + { + "segment_id": segment_id, + **scenario.payload(**kw), + }, + ], + }, + scenario.expected_errors, + id=f"{scenario.id}-option_b", + ), + ] + ], +) +def test_update_flag__invalid_segment_multivariate_options__responds_400( + admin_client: APIClient, + endpoint: UpdateFlagEndpointOption, + environment_api_key: str, + expected_errors: list[Any], + feature: int, + mv_option_50_percent: int, + payload: Callable[..., FeatureUpdatePayload], + segment: int, +) -> None: + # Given / When + response = admin_client.post( + f"/api/experiments/environments/{environment_api_key}/{endpoint}/", + data={ + "feature": {"id": feature}, + **payload(multivariate_option_id=mv_option_50_percent, segment_id=segment), + }, + format="json", + ) + + # Then + assert response.status_code == 400 + assert response.json() in expected_errors + + +@pytest.mark.parametrize( + "endpoint,payload", + [ + pytest.param( + "update-flag-v1", + lambda feature: { + "feature": {"id": feature}, + "multivariate_feature_state_values": [], + }, + id="option_a", + ), + pytest.param( + "update-flag-v2", + lambda feature: { + "feature": {"id": feature}, + "environment_default": {"multivariate_feature_state_values": []}, + }, + id="option_b", + ), + ], +) +def test_update_flag__empty_multivariate_list__deletes_all_options( + admin_client: APIClient, + environment_api_key: str, + feature: int, + mv_option_50_percent: int, + versioned_environment: Environment, + endpoint: UpdateFlagEndpointOption, + payload: Callable[[int], FeatureUpdatePayload], +) -> None: + # Given / When + response = admin_client.post( + f"/api/experiments/environments/{environment_api_key}/{endpoint}/", + payload(feature), + format="json", + ) + + # Then + assert response.status_code == 204 + assert not MultivariateFeatureOption.objects.filter(feature_id=feature).exists() + assert Feature.objects.get(id=feature).type == STANDARD + assert not ( + FeatureState.objects.get_live_feature_states( + environment=versioned_environment, + feature_id=feature, + feature_segment=None, + ) + .get() + .multivariate_feature_state_values.exists() + ) + + +@pytest.mark.parametrize( + "endpoint,setup_payload,payload", + [ + pytest.param( + "update-flag-v1", + lambda feature: { + "feature": {"id": feature}, + "enabled": True, + "value": {"type": "string", "value": "control"}, + }, + lambda feature, segment, option: { + "feature": {"id": feature}, + "segment": {"id": segment}, + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": option, + "percentage_allocation": 80, + }, + ], + }, + id="option_a", + ), + pytest.param( + "update-flag-v2", + lambda feature: { + "feature": {"id": feature}, + "environment_default": { + "enabled": True, + "value": {"type": "string", "value": "control"}, + }, + }, + lambda feature, segment, option: { + "feature": {"id": feature}, + "segment_overrides": [ + { + "segment_id": segment, + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": option, + "percentage_allocation": 80, + }, + ], + }, + ], + }, + id="option_b", + ), + ], +) +def test_update_flag__new_segment_override_without_enabled_and_value__inherits_environment_default( + admin_client: APIClient, + environment_api_key: str, + feature: int, + mv_option_50_percent: int, + segment: int, + versioned_environment: Environment, + endpoint: UpdateFlagEndpointOption, + setup_payload: Callable[[int], FeatureUpdatePayload], + payload: Callable[[int, int, int], FeatureUpdatePayload], +) -> None: + # Given + setup_response = admin_client.post( + f"/api/experiments/environments/{environment_api_key}/{endpoint}/", + setup_payload(feature), + format="json", + ) + assert setup_response.status_code == 204 + + # When + response = admin_client.post( + f"/api/experiments/environments/{environment_api_key}/{endpoint}/", + payload(feature, segment, mv_option_50_percent), + format="json", + ) + + # Then + assert response.status_code == 204 + override_feature_state = FeatureState.objects.get_live_feature_states( + environment=versioned_environment, + feature_id=feature, + ).get(feature_segment__segment_id=segment) + assert override_feature_state.enabled is True + assert override_feature_state.get_feature_state_value() == "control" + + +def test_update_flag_option_b__segment_override_reweights_option_deleted_from_environment__responds_400( + admin_client: APIClient, + environment_api_key: str, + feature: int, + mv_option_50_percent: int, + segment: int, +) -> None: + # Given / When + response = admin_client.post( + f"/api/experiments/environments/{environment_api_key}/update-flag-v2/", + { + "feature": {"id": feature}, + "environment_default": { + "multivariate_feature_state_values": [ + { + "percentage_allocation": 30, + "value": {"type": "string", "value": "replacement"}, + }, + ], + }, + "segment_overrides": [ + { + "segment_id": segment, + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": mv_option_50_percent, + "percentage_allocation": 80, + }, + ], + }, + ], + }, + format="json", + ) + + # Then + assert response.status_code == 400 + assert response.json() == { + "multivariate_feature_state_values": [ + "Segment overrides can only re-weight existing variants." + ] + } + + +@pytest.mark.parametrize( + "endpoint,payload", + [ + pytest.param( + "update-flag-v1", + lambda option: { + "feature": {"name": "unknown_feature"}, + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": option, + "percentage_allocation": 50, + }, + ], + }, + id="option_a", + ), + pytest.param( + "update-flag-v2", + lambda option: { + "feature": {"name": "unknown_feature"}, + "environment_default": { + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": option, + "percentage_allocation": 50, + }, + ], + }, + }, + id="option_b", + ), + ], +) +def test_update_flag__unknown_feature_with_multivariate_options__responds_400( + admin_client: APIClient, + environment_api_key: str, + mv_option_50_percent: int, + endpoint: UpdateFlagEndpointOption, + payload: Callable[[int], FeatureUpdatePayload], +) -> None: + # Given / When + response = admin_client.post( + f"/api/experiments/environments/{environment_api_key}/{endpoint}/", + payload(mv_option_50_percent), + format="json", + ) + + # Then + assert response.status_code == 400 + assert "not found in project" in str(response.json()) diff --git a/api/tests/integration/helpers.py b/api/tests/integration/helpers.py index cd98edd267bc..0145d252c513 100644 --- a/api/tests/integration/helpers.py +++ b/api/tests/integration/helpers.py @@ -45,7 +45,7 @@ def create_feature_with_api( def create_mv_option_with_api( client: APIClient, project_id: int, - feature_id: str, + feature_id: int, default_percentage_allocation: float, value: str, key: str | None = None, diff --git a/api/tests/unit/experimentation/test_services.py b/api/tests/unit/experimentation/test_services.py index 2f881ecbfc73..6517a83a1779 100644 --- a/api/tests/unit/experimentation/test_services.py +++ b/api/tests/unit/experimentation/test_services.py @@ -1485,7 +1485,7 @@ def test_apply_experiment_rollout__update_flag_fails__rolls_back( # Given experiment = experiment_with_rollout mocker.patch( - "experimentation.services.update_flag", + "experimentation.services.update_flag_option_a", side_effect=RuntimeError("boom"), ) @@ -1770,7 +1770,7 @@ def test_enable_experiment_rollout__already_enabled__no_op( mocker: MockerFixture, ) -> None: # Given a rollout that is already enabled - update_flag = mocker.patch("experimentation.services.update_flag") + update_flag_option_a = mocker.patch("experimentation.services.update_flag_option_a") # When services.enable_experiment_rollout( @@ -1778,7 +1778,7 @@ def test_enable_experiment_rollout__already_enabled__no_op( ) # Then no flag write is made - update_flag.assert_not_called() + update_flag_option_a.assert_not_called() def test_enable_experiment_rollout__no_rollout__no_op( @@ -1787,13 +1787,13 @@ def test_enable_experiment_rollout__no_rollout__no_op( mocker: MockerFixture, ) -> None: # Given an experiment without a rollout - update_flag = mocker.patch("experimentation.services.update_flag") + update_flag_option_a = mocker.patch("experimentation.services.update_flag_option_a") # When services.enable_experiment_rollout(experiment, AuthorData(user=admin_user)) # Then nothing is written - update_flag.assert_not_called() + update_flag_option_a.assert_not_called() def test_apply_experiment_rollout__reapplied_under_v2__keeps_variant_assignment( diff --git a/api/tests/unit/features/feature_states/test_serializers.py b/api/tests/unit/features/feature_states/test_serializers.py index 42eedd413584..18a50093651a 100644 --- a/api/tests/unit/features/feature_states/test_serializers.py +++ b/api/tests/unit/features/feature_states/test_serializers.py @@ -6,10 +6,11 @@ from environments.models import Environment from features.feature_states.serializers import ( FeatureValueSerializer, - UpdateFlagSerializer, - UpdateFlagV2Serializer, + UpdateFlagOptionASerializer, + UpdateFlagOptionBSerializer, validate_multivariate_state_values, ) +from features.feature_states.types import SegmentOverrideMultivariateValuePayload from features.models import Feature from projects.models import Project from segments.models import Segment @@ -19,7 +20,7 @@ def test_get_feature__no_environment_in_context__raises_validation_error( feature: Feature, ) -> None: # Given - serializer = UpdateFlagSerializer( + serializer = UpdateFlagOptionASerializer( data={ "feature": {"name": feature.name}, "enabled": True, @@ -39,7 +40,7 @@ def test_get_feature__no_environment_in_context__raises_validation_error( def test_validate_segment_overrides__empty_list__returns_empty_list() -> None: # Given - serializer = UpdateFlagV2Serializer() + serializer = UpdateFlagOptionBSerializer() # When result = serializer.validate_segment_overrides([]) @@ -78,7 +79,7 @@ def test_feature_value_serializer__invalid_boolean__returns_not_valid() -> None: "serializer_class,data_factory", [ ( - UpdateFlagSerializer, + UpdateFlagOptionASerializer, lambda feature, segment_id: { "feature": {"name": feature.name}, "segment": {"id": segment_id}, @@ -87,7 +88,7 @@ def test_feature_value_serializer__invalid_boolean__returns_not_valid() -> None: }, ), ( - UpdateFlagV2Serializer, + UpdateFlagOptionBSerializer, lambda feature, segment_id: { "feature": {"name": feature.name}, "environment_default": { @@ -129,7 +130,7 @@ def test_update_flag_serializer__nonexistent_segment__returns_invalid( "serializer_class,data_factory", [ ( - UpdateFlagSerializer, + UpdateFlagOptionASerializer, lambda feature, segment_id: { "feature": {"name": feature.name}, "segment": {"id": segment_id}, @@ -138,7 +139,7 @@ def test_update_flag_serializer__nonexistent_segment__returns_invalid( }, ), ( - UpdateFlagV2Serializer, + UpdateFlagOptionBSerializer, lambda feature, segment_id: { "feature": {"name": feature.name}, "environment_default": { @@ -182,13 +183,13 @@ def test_update_flag_serializer__cross_project_segment__returns_invalid( assert "not found in project" in str(serializer.errors) -def test_update_flag_v2_serializer__mv_option_not_on_feature__returns_invalid( +def test_update_flag_option_b_serializer__mv_option_not_on_feature__returns_invalid( multivariate_feature: Feature, environment: Environment, segment: Segment, ) -> None: # Given - serializer = UpdateFlagV2Serializer( + serializer = UpdateFlagOptionBSerializer( data={ "feature": {"name": multivariate_feature.name}, "environment_default": { @@ -220,7 +221,7 @@ def test_update_flag_v2_serializer__mv_option_not_on_feature__returns_invalid( assert "do not belong to the feature" in str(serializer.errors) -def test_update_flag_v2_serializer__duplicate_mv_option__returns_invalid( +def test_update_flag_option_b_serializer__duplicate_mv_option__returns_invalid( multivariate_feature: Feature, multivariate_options: list[typing.Any], environment: Environment, @@ -228,7 +229,7 @@ def test_update_flag_v2_serializer__duplicate_mv_option__returns_invalid( ) -> None: # Given the same multivariate option is passed twice option = multivariate_options[0] - serializer = UpdateFlagV2Serializer( + serializer = UpdateFlagOptionBSerializer( data={ "feature": {"name": multivariate_feature.name}, "environment_default": { @@ -268,13 +269,13 @@ def test_validate_multivariate_state_values__empty_list__is_noop( feature: Feature, ) -> None: # Given - multivariate_values: list[dict[str, typing.Any]] = [] + multivariate_values: list[SegmentOverrideMultivariateValuePayload] = [] # When / Then no exception is raised validate_multivariate_state_values(feature, multivariate_values) -def test_update_flag_v2_serializer__valid_mv_option__change_set_carries_mv( +def test_update_flag_option_b_serializer__valid_mv_option__change_set_carries_mv( multivariate_feature: Feature, multivariate_options: list, # type: ignore[type-arg] environment: Environment, @@ -286,7 +287,7 @@ def test_update_flag_v2_serializer__valid_mv_option__change_set_carries_mv( option = multivariate_options[0] request = rf.post("/") request.user = admin_user - serializer = UpdateFlagV2Serializer( + serializer = UpdateFlagOptionBSerializer( data={ "feature": {"name": multivariate_feature.name}, "environment_default": { @@ -312,7 +313,7 @@ def test_update_flag_v2_serializer__valid_mv_option__change_set_carries_mv( # When is_valid = serializer.is_valid() - change_set = serializer.change_set_v2 + change_set = serializer.change_set_option_b # Then assert is_valid is True 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 d11244a590c4..b607077db255 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 @@ -31,7 +31,7 @@ "environment_", (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), ) -def test_update_flag_v1__by_feature_name__updates_feature_state( +def test_update_flag_option_a__by_feature_name__updates_feature_state( staff_client: APIClient, feature: Feature, environment_: Environment, @@ -43,7 +43,7 @@ def test_update_flag_v1__by_feature_name__updates_feature_state( # Given with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] url = reverse( - "api-experiments:update-flag-v1", + "api-experiments:update-flag-option-a", kwargs={"environment_key": environment_.api_key}, ) @@ -73,7 +73,7 @@ def test_update_flag_v1__by_feature_name__updates_feature_state( "environment_", (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), ) -def test_update_flag_v1__by_feature_id__updates_feature_state( +def test_update_flag_option_a__by_feature_id__updates_feature_state( staff_client: APIClient, feature: Feature, environment_: Environment, @@ -82,7 +82,7 @@ def test_update_flag_v1__by_feature_id__updates_feature_state( # Given with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] url = reverse( - "api-experiments:update-flag-v1", + "api-experiments:update-flag-option-a", kwargs={"environment_key": environment_.api_key}, ) @@ -112,7 +112,7 @@ def test_update_flag_v1__by_feature_id__updates_feature_state( "environment_", (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), ) -def test_update_flag_v1__both_name_and_id_provided__returns_400( +def test_update_flag_option_a__both_name_and_id_provided__returns_400( staff_client: APIClient, feature: Feature, environment_: Environment, @@ -121,7 +121,7 @@ def test_update_flag_v1__both_name_and_id_provided__returns_400( # Given with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] url = reverse( - "api-experiments:update-flag-v1", + "api-experiments:update-flag-option-a", kwargs={"environment_key": environment_.api_key}, ) @@ -147,7 +147,7 @@ def test_update_flag_v1__both_name_and_id_provided__returns_400( "environment_", (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), ) -def test_update_flag_v1__both_name_and_id_for_different_features__returns_400( +def test_update_flag_option_a__both_name_and_id_for_different_features__returns_400( staff_client: APIClient, feature: Feature, environment_: Environment, @@ -163,7 +163,7 @@ def test_update_flag_v1__both_name_and_id_for_different_features__returns_400( ) url = reverse( - "api-experiments:update-flag-v1", + "api-experiments:update-flag-option-a", kwargs={"environment_key": environment_.api_key}, ) @@ -189,7 +189,7 @@ def test_update_flag_v1__both_name_and_id_for_different_features__returns_400( "environment_", (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), ) -def test_update_flag_v1__neither_name_nor_id_provided__returns_400( +def test_update_flag_option_a__neither_name_nor_id_provided__returns_400( staff_client: APIClient, feature: Feature, environment_: Environment, @@ -198,7 +198,7 @@ def test_update_flag_v1__neither_name_nor_id_provided__returns_400( # Given with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] url = reverse( - "api-experiments:update-flag-v1", + "api-experiments:update-flag-option-a", kwargs={"environment_key": environment_.api_key}, ) @@ -224,7 +224,7 @@ def test_update_flag_v1__neither_name_nor_id_provided__returns_400( "environment_", (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), ) -def test_update_flag_v1__feature_not_found_by_name__returns_400( +def test_update_flag_option_a__feature_not_found_by_name__returns_400( staff_client: APIClient, environment_: Environment, with_environment_permissions: WithEnvironmentPermissionsCallable, @@ -232,7 +232,7 @@ def test_update_flag_v1__feature_not_found_by_name__returns_400( # Given with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] url = reverse( - "api-experiments:update-flag-v1", + "api-experiments:update-flag-option-a", kwargs={"environment_key": environment_.api_key}, ) @@ -256,7 +256,7 @@ def test_update_flag_v1__feature_not_found_by_name__returns_400( "environment_", (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), ) -def test_update_flag_v1__feature_not_found_by_id__returns_400( +def test_update_flag_option_a__feature_not_found_by_id__returns_400( staff_client: APIClient, environment_: Environment, with_environment_permissions: WithEnvironmentPermissionsCallable, @@ -264,7 +264,7 @@ def test_update_flag_v1__feature_not_found_by_id__returns_400( # Given with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] url = reverse( - "api-experiments:update-flag-v1", + "api-experiments:update-flag-option-a", kwargs={"environment_key": environment_.api_key}, ) @@ -288,7 +288,7 @@ def test_update_flag_v1__feature_not_found_by_id__returns_400( "environment_", (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), ) -def test_update_flag_v1__segment_override_by_name__creates_override( +def test_update_flag_option_a__segment_override_by_name__creates_override( staff_client: APIClient, feature: Feature, environment_: Environment, @@ -303,7 +303,7 @@ def test_update_flag_v1__segment_override_by_name__creates_override( ) url = reverse( - "api-experiments:update-flag-v1", + "api-experiments:update-flag-option-a", kwargs={"environment_key": environment_.api_key}, ) @@ -339,7 +339,7 @@ def test_update_flag_v1__segment_override_by_name__creates_override( "environment_", (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), ) -def test_update_flag_v1__segment_override_no_existing_feature_segment__creates_feature_segment( +def test_update_flag_option_a__segment_override_no_existing_feature_segment__creates_feature_segment( staff_client: APIClient, feature: Feature, environment_: Environment, @@ -361,7 +361,7 @@ def test_update_flag_v1__segment_override_no_existing_feature_segment__creates_f ).exists() url = reverse( - "api-experiments:update-flag-v1", + "api-experiments:update-flag-option-a", kwargs={"environment_key": environment_.api_key}, ) @@ -395,7 +395,7 @@ def test_update_flag_v1__segment_override_no_existing_feature_segment__creates_f assert segment_override.feature_segment.priority == 10 -def test_update_flag_v1__new_segment_override_at_priority_zero__reorders_existing_priorities( +def test_update_flag_option_a__new_segment_override_at_priority_zero__reorders_existing_priorities( staff_client: APIClient, feature: Feature, environment: Environment, @@ -423,7 +423,7 @@ def test_update_flag_v1__new_segment_override_at_priority_zero__reorders_existin ) url = reverse( - "api-experiments:update-flag-v1", + "api-experiments:update-flag-option-a", kwargs={"environment_key": environment.api_key}, ) @@ -459,7 +459,7 @@ def test_update_flag_v1__new_segment_override_at_priority_zero__reorders_existin "environment_", (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), ) -def test_update_flag_v2__new_segment_overrides__creates_overrides( +def test_update_flag_option_b__new_segment_overrides__creates_overrides( staff_client: APIClient, feature: Feature, environment_: Environment, @@ -486,7 +486,7 @@ def test_update_flag_v2__new_segment_overrides__creates_overrides( ).exists() url = reverse( - "api-experiments:update-flag-v2", + "api-experiments:update-flag-option-b", kwargs={"environment_key": environment_.api_key}, ) @@ -547,7 +547,7 @@ def test_update_flag_v2__new_segment_overrides__creates_overrides( "environment_", (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), ) -def test_update_flag_v2__environment_default_only__updates_default_state( +def test_update_flag_option_b__environment_default_only__updates_default_state( staff_client: APIClient, feature: Feature, environment_: Environment, @@ -556,7 +556,7 @@ def test_update_flag_v2__environment_default_only__updates_default_state( # Given with_environment_permissions([UPDATE_FEATURE_STATE]) # type: ignore[call-arg] url = reverse( - "api-experiments:update-flag-v2", + "api-experiments:update-flag-option-b", kwargs={"environment_key": environment_.api_key}, ) @@ -588,7 +588,7 @@ def test_update_flag_v2__environment_default_only__updates_default_state( assert env_default.get_feature_state_value() == 100 -def test_update_flag_v2__duplicate_segment_ids__returns_400( +def test_update_flag_option_b__duplicate_segment_ids__returns_400( staff_client: APIClient, feature: Feature, environment: Environment, @@ -601,7 +601,7 @@ def test_update_flag_v2__duplicate_segment_ids__returns_400( segment1 = Segment.objects.create(name="segment1", project=project) url = reverse( - "api-experiments:update-flag-v2", + "api-experiments:update-flag-option-b", kwargs={"environment_key": environment.api_key}, ) @@ -640,7 +640,7 @@ def test_update_flag_v2__duplicate_segment_ids__returns_400( "environment_", (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), ) -def test_update_flag_v1__existing_segment_override_with_new_priority__updates_priority( +def test_update_flag_option_a__existing_segment_override_with_new_priority__updates_priority( staff_client: APIClient, feature: Feature, environment_: Environment, @@ -668,7 +668,7 @@ def test_update_flag_v1__existing_segment_override_with_new_priority__updates_pr ) url = reverse( - "api-experiments:update-flag-v1", + "api-experiments:update-flag-option-a", kwargs={"environment_key": environment_.api_key}, ) @@ -702,7 +702,7 @@ def test_update_flag_v1__existing_segment_override_with_new_priority__updates_pr "environment_", (lazy_fixture("environment"), lazy_fixture("environment_v2_versioning")), ) -def test_update_flag_v2__existing_segment_override_with_new_priority__updates_priority( +def test_update_flag_option_b__existing_segment_override_with_new_priority__updates_priority( staff_client: APIClient, feature: Feature, environment_: Environment, @@ -731,7 +731,7 @@ def test_update_flag_v2__existing_segment_override_with_new_priority__updates_pr # When - Update the existing segment override using V2 endpoint v2_url = reverse( - "api-experiments:update-flag-v2", + "api-experiments:update-flag-option-b", kwargs={"environment_key": environment_.api_key}, ) update_data = { @@ -768,7 +768,7 @@ def test_update_flag_v2__existing_segment_override_with_new_priority__updates_pr assert feature_segment.priority == 2 -def test_update_flag_v2__new_segment_override_at_priority_zero__reorders_existing_priorities( +def test_update_flag_option_b__new_segment_override_at_priority_zero__reorders_existing_priorities( staff_client: APIClient, feature: Feature, environment: Environment, @@ -796,7 +796,7 @@ def test_update_flag_v2__new_segment_override_at_priority_zero__reorders_existin ) url = reverse( - "api-experiments:update-flag-v2", + "api-experiments:update-flag-option-b", kwargs={"environment_key": environment.api_key}, ) @@ -834,7 +834,7 @@ def test_update_flag_v2__new_segment_override_at_priority_zero__reorders_existin assert feature_segment1.priority == 1 -def test_update_flag_v1__workflow_enabled__returns_403( +def test_update_flag_option_a__workflow_enabled__returns_403( staff_client: APIClient, feature: Feature, environment: Environment, @@ -846,7 +846,7 @@ def test_update_flag_v1__workflow_enabled__returns_403( environment.save() url = reverse( - "api-experiments:update-flag-v1", + "api-experiments:update-flag-option-a", kwargs={"environment_key": environment.api_key}, ) @@ -866,7 +866,7 @@ def test_update_flag_v1__workflow_enabled__returns_403( assert "change requests are enabled" in str(response.json()) -def test_update_flag_v2__workflow_enabled__returns_403( +def test_update_flag_option_b__workflow_enabled__returns_403( staff_client: APIClient, feature: Feature, environment: Environment, @@ -878,7 +878,7 @@ def test_update_flag_v2__workflow_enabled__returns_403( environment.save() url = reverse( - "api-experiments:update-flag-v2", + "api-experiments:update-flag-option-b", kwargs={"environment_key": environment.api_key}, ) @@ -900,7 +900,7 @@ def test_update_flag_v2__workflow_enabled__returns_403( assert "change requests are enabled" in str(response.json()) -def test_update_flag_v2__existing_segment_override_with_v2_versioning__updates_override( +def test_update_flag_option_b__existing_segment_override_with_v2_versioning__updates_override( staff_client: APIClient, feature: Feature, environment_v2_versioning: Environment, @@ -931,7 +931,7 @@ def test_update_flag_v2__existing_segment_override_with_v2_versioning__updates_o ) url = reverse( - "api-experiments:update-flag-v2", + "api-experiments:update-flag-option-b", kwargs={"environment_key": environment_v2_versioning.api_key}, ) @@ -974,14 +974,14 @@ def test_update_flag_v2__existing_segment_override_with_v2_versioning__updates_o assert segment_override.feature_segment.priority == 1 -def test_update_flag_v1__no_permission__returns_403( +def test_update_flag_option_a__no_permission__returns_403( staff_client: APIClient, feature: Feature, environment: Environment, ) -> None: # Given - no permissions granted url = reverse( - "api-experiments:update-flag-v1", + "api-experiments:update-flag-option-a", kwargs={"environment_key": environment.api_key}, ) @@ -1000,14 +1000,14 @@ def test_update_flag_v1__no_permission__returns_403( assert response.status_code == status.HTTP_403_FORBIDDEN -def test_update_flag_v2__no_permission__returns_403( +def test_update_flag_option_b__no_permission__returns_403( staff_client: APIClient, feature: Feature, environment: Environment, ) -> None: # Given - no permissions granted url = reverse( - "api-experiments:update-flag-v2", + "api-experiments:update-flag-option-b", kwargs={"environment_key": environment.api_key}, ) @@ -1028,12 +1028,12 @@ def test_update_flag_v2__no_permission__returns_403( assert response.status_code == status.HTTP_403_FORBIDDEN -def test_update_flag_v1__nonexistent_environment__returns_403( +def test_update_flag_option_a__nonexistent_environment__returns_403( staff_client: APIClient, ) -> None: # Given url = reverse( - "api-experiments:update-flag-v1", + "api-experiments:update-flag-option-a", kwargs={"environment_key": "nonexistent_key"}, ) 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 67e9d2e5dbc3..9add40294731 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 @@ -16,8 +16,10 @@ MultivariateFeatureStateValue, ) from features.versioning.dataclasses import ( - FlagChangeSet, - FlagChangeSetV2, + EnvironmentDefaultChangeSet, + FeatureValue, + FlagChangeSetOptionA, + FlagChangeSetOptionB, MultivariateValueChangeSet, SegmentOverrideChangeSet, ) @@ -27,8 +29,8 @@ get_environment_flags_list, get_environment_flags_queryset, get_updated_feature_states_for_version, - update_flag, - update_flag_v2, + update_flag_option_a, + update_flag_option_b, ) from projects.models import Project from segments.models import Segment @@ -639,18 +641,18 @@ def _mv_change_set( segment: Segment, *, multivariate_values: list[MultivariateValueChangeSet] | None, -) -> FlagChangeSetV2: - return FlagChangeSetV2( +) -> FlagChangeSetOptionB: + return FlagChangeSetOptionB( author=author, - environment_default_enabled=True, - environment_default_value="control", - environment_default_type="string", + environment_default=EnvironmentDefaultChangeSet( + enabled=True, + value=FeatureValue(type_="string", value="control"), + ), segment_overrides=[ SegmentOverrideChangeSet( segment_id=segment.id, enabled=True, - feature_state_value="control", - type_="string", + value=FeatureValue(type_="string", value="control"), multivariate_values=multivariate_values, ) ], @@ -686,7 +688,7 @@ def _override_allocations(override: FeatureState) -> dict[int, float]: "environment_fixture_name", ["environment", "environment_v2_versioning"], ) -def test_update_flag_v2__new_segment_override_with_mv__creates_mv_values( +def test_update_flag_option_b__new_segment_override_with_mv__creates_mv_values( environment_fixture_name: str, multivariate_feature: Feature, multivariate_options: list[MultivariateFeatureOption], @@ -707,7 +709,7 @@ def test_update_flag_v2__new_segment_override_with_mv__creates_mv_values( ) # When - update_flag_v2(environment, multivariate_feature, change_set) + update_flag_option_b(environment, multivariate_feature, change_set) # Then override = _get_live_override(environment, multivariate_feature, segment) @@ -718,7 +720,7 @@ def test_update_flag_v2__new_segment_override_with_mv__creates_mv_values( "environment_fixture_name", ["environment", "environment_v2_versioning"], ) -def test_update_flag_v2__existing_override_mv_changed__updates_allocations( +def test_update_flag_option_b__existing_override_mv_changed__updates_allocations( environment_fixture_name: str, multivariate_feature: Feature, multivariate_options: list[MultivariateFeatureOption], @@ -730,7 +732,7 @@ def test_update_flag_v2__existing_override_mv_changed__updates_allocations( environment: Environment = request.getfixturevalue(environment_fixture_name) author = AuthorData(user=admin_user) option_a, option_b, _ = multivariate_options - update_flag_v2( + update_flag_option_b( environment, multivariate_feature, _mv_change_set( @@ -744,7 +746,7 @@ def test_update_flag_v2__existing_override_mv_changed__updates_allocations( ) # When - update_flag_v2( + update_flag_option_b( environment, multivariate_feature, _mv_change_set( @@ -766,7 +768,7 @@ def test_update_flag_v2__existing_override_mv_changed__updates_allocations( "environment_fixture_name", ["environment", "environment_v2_versioning"], ) -def test_update_flag_v2__option_not_passed__is_retained( +def test_update_flag_option_b__option_not_passed__is_retained( environment_fixture_name: str, multivariate_feature: Feature, multivariate_options: list[MultivariateFeatureOption], @@ -778,7 +780,7 @@ def test_update_flag_v2__option_not_passed__is_retained( environment: Environment = request.getfixturevalue(environment_fixture_name) author = AuthorData(user=admin_user) option_a, option_b, _ = multivariate_options - update_flag_v2( + update_flag_option_b( environment, multivariate_feature, _mv_change_set( @@ -792,7 +794,7 @@ def test_update_flag_v2__option_not_passed__is_retained( ) # When only option_a is passed - update_flag_v2( + update_flag_option_b( environment, multivariate_feature, _mv_change_set( @@ -811,7 +813,7 @@ def test_update_flag_v2__option_not_passed__is_retained( "environment_fixture_name", ["environment", "environment_v2_versioning"], ) -def test_update_flag_v2__no_mv_values__leaves_existing_mv_untouched( +def test_update_flag_option_b__no_mv_values__leaves_existing_mv_untouched( environment_fixture_name: str, multivariate_feature: Feature, multivariate_options: list[MultivariateFeatureOption], @@ -823,7 +825,7 @@ def test_update_flag_v2__no_mv_values__leaves_existing_mv_untouched( environment: Environment = request.getfixturevalue(environment_fixture_name) author = AuthorData(user=admin_user) option_a, option_b, _ = multivariate_options - update_flag_v2( + update_flag_option_b( environment, multivariate_feature, _mv_change_set( @@ -837,7 +839,7 @@ def test_update_flag_v2__no_mv_values__leaves_existing_mv_untouched( ) # When - update_flag_v2( + update_flag_option_b( environment, multivariate_feature, _mv_change_set(author, segment, multivariate_values=None), @@ -852,7 +854,7 @@ def test_update_flag_v2__no_mv_values__leaves_existing_mv_untouched( "environment_fixture_name", ["environment", "environment_v2_versioning"], ) -def test_update_flag__segment_override_with_mv__sets_mv_values( +def test_update_flag_option_a__segment_override_with_mv__sets_mv_values( environment_fixture_name: str, multivariate_feature: Feature, multivariate_options: list[MultivariateFeatureOption], @@ -865,14 +867,13 @@ def test_update_flag__segment_override_with_mv__sets_mv_values( option_a, option_b, _ = multivariate_options # When - update_flag( + update_flag_option_a( environment, multivariate_feature, - FlagChangeSet( + FlagChangeSetOptionA( author=AuthorData(user=admin_user), enabled=True, - feature_state_value="control", - type_="string", + value=FeatureValue(type_="string", value="control"), segment_id=segment.id, multivariate_values=[ MultivariateValueChangeSet(option_a.id, 70.0), @@ -890,7 +891,7 @@ def test_update_flag__segment_override_with_mv__sets_mv_values( "environment_fixture_name", ["environment", "environment_v2_versioning"], ) -def test_update_flag_v2__retained_plus_passed_exceeds_100__raises( +def test_update_flag_option_b__retained_plus_passed_exceeds_100__raises( environment_fixture_name: str, multivariate_feature: Feature, multivariate_options: list[MultivariateFeatureOption], @@ -902,7 +903,7 @@ def test_update_flag_v2__retained_plus_passed_exceeds_100__raises( environment: Environment = request.getfixturevalue(environment_fixture_name) author = AuthorData(user=admin_user) option_a, option_b, _ = multivariate_options - update_flag_v2( + update_flag_option_b( environment, multivariate_feature, _mv_change_set( @@ -918,7 +919,7 @@ def test_update_flag_v2__retained_plus_passed_exceeds_100__raises( # When option_b alone is raised to 100% (retained option_a 80% → 180% total) # Then it is rejected with pytest.raises(ValidationError): - update_flag_v2( + update_flag_option_b( environment, multivariate_feature, _mv_change_set( diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index 21ceb55de3a7..ac842a0e9428 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -532,7 +532,7 @@ Attributes: ### `warehouse.connection.connected` Logged at `info` from: - - `api/experimentation/services.py:742` + - `api/experimentation/services.py:747` Attributes: - `environment.id` @@ -541,7 +541,7 @@ Attributes: ### `warehouse.connection.test_event_sent` Logged at `info` from: - - `api/experimentation/services.py:722` + - `api/experimentation/services.py:727` Attributes: - `environment.id` diff --git a/docs/docs/integrating-with-flagsmith/flagsmith-api-overview/admin-api/updating-flags.md b/docs/docs/integrating-with-flagsmith/flagsmith-api-overview/admin-api/updating-flags.md index 6a8ca75fbfb1..704cde049705 100644 --- a/docs/docs/integrating-with-flagsmith/flagsmith-api-overview/admin-api/updating-flags.md +++ b/docs/docs/integrating-with-flagsmith/flagsmith-api-overview/admin-api/updating-flags.md @@ -4,14 +4,12 @@ sidebar_label: Updating Flags (Experimental) sidebar_position: 3 --- -These experimental endpoints let you update feature flag values and segment overrides via the Admin API. They're -purpose-built for automation and CI/CD — minimal payloads, no need to look up internal IDs, and they work the same -regardless of whether your environment has Feature Versioning enabled. +These endpoints let you update feature flags, segment overrides, and variant allocations via the Admin API. :::caution -These endpoints are experimental and may change without notice. They do not support multivariate values and cannot be -used when [change requests](/administration-and-security/governance-and-compliance/change-requests) are enabled. +**These endpoints are experimental and may change without notice.** They cannot be used when +[change requests](/administration-and-security/governance-and-compliance/change-requests) are enabled. ::: @@ -23,13 +21,14 @@ in one request). Each scenario below shows both. Try them and - Identify features by `name` or `id` (pick one, not both). - All endpoints return **204 No Content** on success. +- Omitted fields are treated as "no change" (e.g. if `enabled` is omitted, the feature's enabled state is unchanged). - Values are passed as a `value` object with `type` and `value` (always a string): -| Type | Example | -| --------- | ------------------------------------------------ | -| `string` | `{"type": "string", "value": "hello"}` | -| `integer` | `{"type": "integer", "value": "42"}` | -| `boolean` | `{"type": "boolean", "value": "true"}` | +| Type | Example | +| --------- | -------------------------------------- | +| `string` | `{"type": "string", "value": "hello"}` | +| `integer` | `{"type": "integer", "value": "42"}` | +| `boolean` | `{"type": "boolean", "value": "true"}` | --- @@ -37,7 +36,8 @@ in one request). Each scenario below shows both. Try them and The simplest case — flip a feature flag in an environment. -**Option A** — [`POST /api/experiments/environments/{environment_key}/update-flag-v1/`](https://api.flagsmith.com/api/v1/docs/#/experimental/api_experiments_environments_update_flag_v1_create) +**Option A** — +[`POST /api/experiments/environments/{environment_key}/update-flag-v1/`](https://api.flagsmith.com/api/v1/docs/#/experimental/api_experiments_environments_update_flag_v1_create) ```bash curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environment_key}/update-flag-v1/' \ @@ -50,7 +50,8 @@ curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environmen }' ``` -**Option B** — [`POST /api/experiments/environments/{environment_key}/update-flag-v2/`](https://api.flagsmith.com/api/v1/docs/#/experimental/api_experiments_environments_update_flag_v2_create) +**Option B** — +[`POST /api/experiments/environments/{environment_key}/update-flag-v2/`](https://api.flagsmith.com/api/v1/docs/#/experimental/api_experiments_environments_update_flag_v2_create) ```bash curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environment_key}/update-flag-v2/' \ @@ -71,7 +72,8 @@ curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environmen Change a feature's value — for example, setting a rate limit. -**Option A** +**Option A** — +[`POST /api/experiments/environments/{environment_key}/update-flag-v1/`](https://api.flagsmith.com/api/v1/docs/#/experimental/api_experiments_environments_update_flag_v1_create) ```bash curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environment_key}/update-flag-v1/' \ @@ -84,7 +86,8 @@ curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environmen }' ``` -**Option B** +**Option B** — +[`POST /api/experiments/environments/{environment_key}/update-flag-v2/`](https://api.flagsmith.com/api/v1/docs/#/experimental/api_experiments_environments_update_flag_v2_create) ```bash curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environment_key}/update-flag-v2/' \ @@ -105,7 +108,8 @@ curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environmen Enable a feature for a specific segment (e.g. beta users) while keeping it off for everyone else. -**Option A** +**Option A** — +[`POST /api/experiments/environments/{environment_key}/update-flag-v1/`](https://api.flagsmith.com/api/v1/docs/#/experimental/api_experiments_environments_update_flag_v1_create) ```bash curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environment_key}/update-flag-v1/' \ @@ -119,7 +123,8 @@ curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environmen }' ``` -**Option B** — single request: +**Option B** — +[`POST /api/experiments/environments/{environment_key}/update-flag-v2/`](https://api.flagsmith.com/api/v1/docs/#/experimental/api_experiments_environments_update_flag_v2_create) ```bash curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environment_key}/update-flag-v2/' \ @@ -141,7 +146,8 @@ curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environmen }' ``` -The `priority` field on segment overrides is optional. Omit it to add at the lowest priority. Priority `1` is highest. +The `priority` field on segment overrides is optional. Omit it to add at the lowest priority. The lowest number has the +highest priority. --- @@ -149,10 +155,11 @@ The `priority` field on segment overrides is optional. Omit it to add at the low Set different values per segment — for example, pricing tiers. -**Option A** — one request per segment override plus one for the default: +**Option A** — +[`POST /api/experiments/environments/{environment_key}/update-flag-v1/`](https://api.flagsmith.com/api/v1/docs/#/experimental/api_experiments_environments_update_flag_v1_create) ```bash -# Default +# Set environment default curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environment_key}/update-flag-v1/' \ -H 'Authorization: Api-Key ' \ -H 'Content-Type: application/json' \ @@ -162,7 +169,7 @@ curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environmen "value": {"type": "string", "value": "standard"} }' -# Enterprise segment (highest priority) +# Set override for segment "Enterprise" (higher priority) curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environment_key}/update-flag-v1/' \ -H 'Authorization: Api-Key ' \ -H 'Content-Type: application/json' \ @@ -173,7 +180,7 @@ curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environmen "value": {"type": "string", "value": "enterprise"} }' -# Premium segment +# Set override for segment "Premium" curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environment_key}/update-flag-v1/' \ -H 'Authorization: Api-Key ' \ -H 'Content-Type: application/json' \ @@ -185,7 +192,8 @@ curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environmen }' ``` -**Option B** — single request: +**Option B** — +[`POST /api/experiments/environments/{environment_key}/update-flag-v2/`](https://api.flagsmith.com/api/v1/docs/#/experimental/api_experiments_environments_update_flag_v2_create) ```bash curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environment_key}/update-flag-v2/' \ @@ -216,6 +224,169 @@ curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environmen --- +## Configure A/B/n experiments (multivariate flags) + +Set up multivariate flags and customise weights per segment. + +Multivariate options can only be added, deleted, or updated in the environment. The `multivariate_feature_state_values` +list is absolute: omitting an option means deleting it. + +Segment overrides can only re-weight existing variants. They cannot update the variant value, or add or delete +multivariate options. + +**Option A** — +[`POST /api/experiments/environments/{environment_key}/update-flag-v1/`](https://api.flagsmith.com/api/v1/docs/#/experimental/api_experiments_environments_update_flag_v1_create) + +```bash +# First, configure multivariate flags in the environment +curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environment_key}/update-flag-v1/' \ + -H 'Authorization: Api-Key ' \ + -H 'Content-Type: application/json' \ + -d '{ + "feature": {"name": "new_payment_gateway_experiment"}, + "enabled": true, + "value": {"type": "string", "value": "default"}, + "multivariate_feature_state_values": [ + { + "percentage_allocation": 20, + "value": {"type": "string", "value": "sharp_payments"} + }, + { + "percentage_allocation": 10, + "value": {"type": "string", "value": "gemstone_express"} + } + ] + }' + +# Multivariate option `id`s can be fetched via the admin API +curl -X GET '/api/v1/projects/{project_id}/features/{feature_id}/mv-options/' \ + -H 'Authorization: Api-Key ' \ + -H 'Content-Type: application/json' + +# You can update (add, delete, re-weight) multivariate options in the environment +curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environment_key}/update-flag-v1/' \ + -H 'Authorization: Api-Key ' \ + -H 'Content-Type: application/json' \ + -d '{ + "feature": {"name": "new_payment_gateway_experiment"}, + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": 991, + "percentage_allocation": 20, + "value": {"type": "string", "value": "sharp_payments"} + }, + { + "percentage_allocation": 5, + "value": {"type": "string", "value": "e-z-pay"} + } + ] + }' + +# Segment overrides can re-weight multivariate options (but can't add, delete, or update values) +curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environment_key}/update-flag-v1/' \ + -H 'Authorization: Api-Key ' \ + -H 'Content-Type: application/json' \ + -d '{ + "feature": {"name": "new_payment_gateway_experiment"}, + "segment": {"id": 101, "priority": 1}, + "multivariate_feature_state_values": [ + {"multivariate_feature_option": 991, "percentage_allocation": 0}, + {"multivariate_feature_option": 993, "percentage_allocation": 100} + ] + }' +``` + +**Option B** — +[`POST /api/experiments/environments/{environment_key}/update-flag-v2/`](https://api.flagsmith.com/api/v1/docs/#/experimental/api_experiments_environments_update_flag_v2_create) + +```bash +# Configure the environment default and its multivariate options +curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environment_key}/update-flag-v2/' \ + -H 'Authorization: Api-Key ' \ + -H 'Content-Type: application/json' \ + -d '{ + "feature": {"name": "new_payment_gateway_experiment"}, + "environment_default": { + "enabled": true, + "value": {"type": "string", "value": "default"}, + "multivariate_feature_state_values": [ + { + "percentage_allocation": 20, + "value": {"type": "string", "value": "sharp_payments"} + }, + { + "percentage_allocation": 10, + "value": {"type": "string", "value": "gemstone_express"} + } + ] + } + }' + +# Multivariate option `id`s can be fetched via the admin API +curl -X GET '/api/v1/projects/{project_id}/features/{feature_id}/mv-options/' \ + -H 'Authorization: Api-Key ' \ + -H 'Content-Type: application/json' + +# Segment overrides can re-weight multivariate options (but can't add, delete, or update values) +curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environment_key}/update-flag-v2/' \ + -H 'Authorization: Api-Key ' \ + -H 'Content-Type: application/json' \ + -d '{ + "feature": {"name": "new_payment_gateway_experiment"}, + "segment_overrides": [ + { + "segment_id": 101, + "multivariate_feature_state_values": [ + {"multivariate_feature_option": 991, "percentage_allocation": 50}, + {"multivariate_feature_option": 992, "percentage_allocation": 50} + ] + } + ] + }' + +# Re-weight the environment default and segment overrides in a single request +curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environment_key}/update-flag-v2/' \ + -H 'Authorization: Api-Key ' \ + -H 'Content-Type: application/json' \ + -d '{ + "feature": {"name": "new_payment_gateway_experiment"}, + "environment_default": { + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": 991, + "percentage_allocation": 30, + "value": {"type": "string", "value": "sharp_payments"} + }, + { + "multivariate_feature_option": 992, + "percentage_allocation": 5, + "value": {"type": "string", "value": "gemstone_express"} + } + ] + }, + "segment_overrides": [ + { + "segment_id": 101, + "priority": 1, + "multivariate_feature_state_values": [ + {"multivariate_feature_option": 991, "percentage_allocation": 100}, + {"multivariate_feature_option": 992, "percentage_allocation": 0} + ] + }, + { + "segment_id": 202, + "priority": 2, + "multivariate_feature_state_values": [ + {"multivariate_feature_option": 991, "percentage_allocation": 0}, + {"multivariate_feature_option": 992, "percentage_allocation": 100} + ] + } + ] + }' +``` + +--- + ## Remove a segment override A separate endpoint for removing a segment override from a feature: @@ -231,17 +402,3 @@ curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environmen "segment": {"id": 202} }' ``` - ---- - -## Quick reference - -| Aspect | Details | -| -------------------- | ---------------------------------------------------------------------------- | -| Feature ID | `name` or `id` — use one, not both | -| Value types | `string`, `integer`, `boolean` | -| Segment priority | Optional — omit to add at lowest priority; `1` is highest | -| Feature Versioning | Works the same whether enabled or not | -| Success response | `204 No Content` | -| Limitations | No multivariate support; incompatible with change requests | -| Full API schema | [Swagger Explorer](https://api.flagsmith.com/api/v1/docs/) | diff --git a/mcp/src/flagsmith_mcp/openapi.json b/mcp/src/flagsmith_mcp/openapi.json index 008774b33d4d..4a532a02eda6 100644 --- a/mcp/src/flagsmith_mcp/openapi.json +++ b/mcp/src/flagsmith_mcp/openapi.json @@ -4582,7 +4582,7 @@ "multivariate_feature_state_values": { "type": "array", "items": { - "$ref": "#/components/schemas/MultivariateValue" + "$ref": "#/components/schemas/SegmentOverrideMultivariateValue" } } }, @@ -5623,24 +5623,6 @@ "percentage_allocation" ] }, - "MultivariateValue": { - "type": "object", - "properties": { - "multivariate_feature_option": { - "type": "integer" - }, - "percentage_allocation": { - "type": "number", - "format": "double", - "maximum": 100, - "minimum": 0 - } - }, - "required": [ - "multivariate_feature_option", - "percentage_allocation" - ] - }, "NestedMultivariateFeatureOption": { "type": "object", "properties": { @@ -6915,6 +6897,27 @@ } } }, + "SegmentOverrideMultivariateValue": { + "type": "object", + "properties": { + "percentage_allocation": { + "type": "number", + "format": "double", + "maximum": 100, + "minimum": 0 + }, + "multivariate_feature_option": { + "type": "integer" + }, + "value": { + "$ref": "#/components/schemas/FeatureValue" + } + }, + "required": [ + "multivariate_feature_option", + "percentage_allocation" + ] + }, "SegmentRule": { "description": "Adds nested create feature", "type": "object", diff --git a/openapi.yaml b/openapi.yaml index 10083805de9d..025c40182416 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -99,13 +99,13 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/UpdateFlag' + $ref: '#/components/schemas/UpdateFlagOptionA' application/x-www-form-urlencoded: schema: - $ref: '#/components/schemas/UpdateFlag' + $ref: '#/components/schemas/UpdateFlagOptionA' multipart/form-data: schema: - $ref: '#/components/schemas/UpdateFlag' + $ref: '#/components/schemas/UpdateFlagOptionA' responses: '204': description: No response body @@ -162,13 +162,13 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/UpdateFlagV2' + $ref: '#/components/schemas/UpdateFlagOptionB' application/x-www-form-urlencoded: schema: - $ref: '#/components/schemas/UpdateFlagV2' + $ref: '#/components/schemas/UpdateFlagOptionB' multipart/form-data: schema: - $ref: '#/components/schemas/UpdateFlagV2' + $ref: '#/components/schemas/UpdateFlagOptionB' responses: '204': description: No response body @@ -19473,9 +19473,10 @@ components: type: boolean value: $ref: '#/components/schemas/FeatureValue' - required: - - enabled - - value + multivariate_feature_state_values: + type: array + items: + $ref: '#/components/schemas/EnvironmentMultivariateValue' EnvironmentFeatureVersion: type: object properties: @@ -19667,6 +19668,20 @@ components: items: $ref: '#/components/schemas/MetricItem' readOnly: true + EnvironmentMultivariateValue: + type: object + properties: + percentage_allocation: + type: number + format: double + maximum: 100 + minimum: 0 + multivariate_feature_option: + type: integer + value: + $ref: '#/components/schemas/FeatureValue' + required: + - percentage_allocation EnvironmentRetrieveSerializerWithMetadata: description: Functionality for serializers that need to handle metadata type: object @@ -20172,7 +20187,7 @@ components: multivariate_feature_state_values: type: array items: - $ref: '#/components/schemas/MultivariateValue' + $ref: '#/components/schemas/SegmentOverrideMultivariateValue' required: - enabled - feature_state_value @@ -20855,17 +20870,6 @@ components: type: - boolean - 'null' - FeatureUpdateSegmentData: - type: object - properties: - id: - type: integer - priority: - type: - - integer - - 'null' - required: - - id FeatureValue: type: object properties: @@ -21964,19 +21968,6 @@ components: - type: boolean - type: 'null' readOnly: true - MultivariateValue: - type: object - properties: - multivariate_feature_option: - type: integer - percentage_allocation: - type: number - format: double - maximum: 100 - minimum: 0 - required: - - multivariate_feature_option - - percentage_allocation NameEnum: description: |- * `Webhook` - Webhook @@ -26244,11 +26235,35 @@ components: multivariate_feature_state_values: type: array items: - $ref: '#/components/schemas/MultivariateValue' + $ref: '#/components/schemas/SegmentOverrideMultivariateValue' required: - - enabled - segment_id - - value + SegmentOverrideMultivariateValue: + type: object + properties: + percentage_allocation: + type: number + format: double + maximum: 100 + minimum: 0 + multivariate_feature_option: + type: integer + value: + $ref: '#/components/schemas/FeatureValue' + required: + - multivariate_feature_option + - percentage_allocation + SegmentReference: + type: object + properties: + id: + type: integer + priority: + type: + - integer + - 'null' + required: + - id SegmentRule: description: Adds nested create feature type: object @@ -27009,22 +27024,24 @@ components: allOf: - $ref: '#/components/schemas/LifecycleStageEnum' readOnly: true - UpdateFlag: + UpdateFlagOptionA: type: object properties: feature: $ref: '#/components/schemas/FeatureIdentifier' segment: - $ref: '#/components/schemas/FeatureUpdateSegmentData' + $ref: '#/components/schemas/SegmentReference' enabled: type: boolean value: $ref: '#/components/schemas/FeatureValue' + multivariate_feature_state_values: + type: array + items: + $ref: '#/components/schemas/EnvironmentMultivariateValue' required: - - enabled - feature - - value - UpdateFlagV2: + UpdateFlagOptionB: type: object properties: feature: @@ -27036,7 +27053,6 @@ components: items: $ref: '#/components/schemas/SegmentOverride' required: - - environment_default - feature UpdateSubscription: type: object