diff --git a/api/experimentation/serializers.py b/api/experimentation/serializers.py index 4a1556300a00..50684ece3680 100644 --- a/api/experimentation/serializers.py +++ b/api/experimentation/serializers.py @@ -26,12 +26,10 @@ from experimentation.types import ( SNOWFLAKE_DEFAULTS, MetricExperimentResult, + RolloutMultivariateValuePayload, SnowflakeConfig, ) -from features.feature_states.serializers import ( - FeatureValueSerializer, - SegmentOverrideMultivariateValueSerializer, -) +from features.feature_states.serializers import FeatureValueSerializer from features.feature_types import MULTIVARIATE from features.models import Feature from features.multivariate.serializers import NestedMultivariateFeatureOptionSerializer @@ -217,13 +215,22 @@ class ExperimentMetricInlineSerializer(serializers.Serializer): # type: ignore[ expected_direction = serializers.ChoiceField(choices=ExpectedDirection.choices) +class RolloutMultivariateValueSerializer( + serializers.Serializer[RolloutMultivariateValuePayload] +): + multivariate_feature_option = serializers.IntegerField(required=True) + percentage_allocation = serializers.FloatField( + required=True, min_value=0, max_value=100 + ) + + class ExperimentRolloutSerializer(serializers.Serializer): # type: ignore[type-arg] enabled = serializers.BooleanField(required=True) rollout_percentage = serializers.FloatField( required=True, min_value=0, max_value=100 ) feature_state_value = FeatureValueSerializer(required=True) - multivariate_feature_state_values = SegmentOverrideMultivariateValueSerializer( + multivariate_feature_state_values = RolloutMultivariateValueSerializer( many=True, required=False ) diff --git a/api/experimentation/types.py b/api/experimentation/types.py index 996384956c6c..b99102f26f77 100644 --- a/api/experimentation/types.py +++ b/api/experimentation/types.py @@ -3,6 +3,11 @@ ExposureGranularity = Literal["hour", "day"] +class RolloutMultivariateValuePayload(TypedDict): + multivariate_feature_option: int + percentage_allocation: float + + class MetricDefinitionV1(TypedDict): """The recipe a metric is computed from. diff --git a/api/features/feature_states/serializers.py b/api/features/feature_states/serializers.py index a908924c17ee..3a2b620e1997 100644 --- a/api/features/feature_states/serializers.py +++ b/api/features/feature_states/serializers.py @@ -1,10 +1,11 @@ -from collections.abc import Sequence +from collections.abc import Mapping, Sequence from typing import TypeVar, cast from rest_framework import serializers from core.dataclasses import AuthorData from environments.models import Environment +from features.constants import CONTROL_VARIANT_KEY, RESERVED_VARIANT_KEY_MESSAGE from features.feature_states.types import ( DeleteSegmentOverridePayload, EnvironmentDefaultPayload, @@ -25,9 +26,12 @@ FeatureValue, FlagChangeSetOptionA, FlagChangeSetOptionB, + KeyedMultivariateOptionChangeSet, + MultivariateKeyValueChangeSet, MultivariateOptionUpdateChangeSet, MultivariateValueChangeSet, NewMultivariateOptionChangeSet, + SegmentMultivariateValueChangeSet, SegmentOverrideChangeSet, ) from features.versioning.versioning_service import ( @@ -120,32 +124,59 @@ class BaseMultivariateValueSerializer(serializers.Serializer[_IN]): percentage_allocation = serializers.FloatField( required=True, min_value=0, max_value=100 ) + multivariate_feature_option = serializers.IntegerField(required=False) + key = serializers.SlugField(required=False) + + def validate_key(self, value: str) -> str: + if value == CONTROL_VARIANT_KEY: + raise serializers.ValidationError(RESERVED_VARIANT_KEY_MESSAGE) + return value + + +def _validate_single_variant_identifier(attrs: Mapping[str, object]) -> None: + if "multivariate_feature_option" in attrs and "key" in attrs: + raise serializers.ValidationError( + "Provide either 'multivariate_feature_option' or 'key', not both" + ) class SegmentOverrideMultivariateValueSerializer( BaseMultivariateValueSerializer[SegmentOverrideMultivariateValuePayload] ): - multivariate_feature_option = serializers.IntegerField(required=True) value = FeatureValueSerializer(required=False) # Raises ValidationError if provided + def validate( + self, attrs: SegmentOverrideMultivariateValuePayload + ) -> SegmentOverrideMultivariateValuePayload: + _validate_single_variant_identifier(attrs) + if "multivariate_feature_option" not in attrs and "key" not in attrs: + raise serializers.ValidationError( + "Segment overrides require a variant 'id' or 'key'." + ) + return attrs + class EnvironmentMultivariateValueSerializer( BaseMultivariateValueSerializer[EnvironmentMultivariateValuePayload] ): - multivariate_feature_option = serializers.IntegerField(required=False) value = FeatureValueSerializer(required=False) def validate( self, attrs: EnvironmentMultivariateValuePayload ) -> EnvironmentMultivariateValuePayload: - if "multivariate_feature_option" not in attrs and "value" not in attrs: + _validate_single_variant_identifier(attrs) + if ( + "multivariate_feature_option" not in attrs + and "key" 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( +def _validate_multivariate_option_references( feature: Feature, multivariate_values: Sequence[ EnvironmentMultivariateValuePayload | SegmentOverrideMultivariateValuePayload @@ -156,9 +187,21 @@ def validate_multivariate_state_values( for mv in multivariate_values if "multivariate_feature_option" in mv ] - if not option_ids: + keys = [mv["key"] for mv in multivariate_values if "key" in mv] + if not option_ids and not keys: return - if len(option_ids) != len(set(option_ids)): + option_key_by_id: dict[int, str | None] = dict( + feature.multivariate_options.values_list("id", "key") + ) + option_id_by_key = { + key: option_id for option_id, key in option_key_by_id.items() if key is not None + } + referenced_option_ids = option_ids + [ + option_id_by_key[key] for key in keys if key in option_id_by_key + ] + if len(keys) != len(set(keys)) or len(referenced_option_ids) != len( + set(referenced_option_ids) + ): raise serializers.ValidationError( { "multivariate_feature_state_values": [ @@ -166,8 +209,7 @@ def validate_multivariate_state_values( ] } ) - valid = set(feature.multivariate_options.values_list("id", flat=True)) - if invalid := set(option_ids) - valid: + if invalid := set(option_ids) - option_key_by_id.keys(): raise serializers.ValidationError( { "multivariate_feature_state_values": [ @@ -177,6 +219,91 @@ def validate_multivariate_state_values( ) +def _validate_multivariate_keys_exist( + feature: Feature, + multivariate_values: Sequence[ + EnvironmentMultivariateValuePayload | SegmentOverrideMultivariateValuePayload + ], +) -> None: + keys = {mv["key"] for mv in multivariate_values if "key" in mv} + if not keys: + return + existing_keys = set( + feature.multivariate_options.filter(key__in=keys).values_list("key", flat=True) + ) + if unknown_keys := keys - existing_keys: + raise serializers.ValidationError( + { + "multivariate_feature_state_values": [ + f"Multivariate keys {sorted(unknown_keys)} do not belong to the feature" + ] + } + ) + + +def _validate_overrides_reweight_environment_options( + feature: Feature, + environment_multivariate_values: Sequence[EnvironmentMultivariateValuePayload], + override_multivariate_value_lists: Sequence[ + Sequence[SegmentOverrideMultivariateValuePayload] + ], +) -> None: + option_id_by_key: dict[str, int] = { + key: option_id + for key, option_id in feature.multivariate_options.filter( + key__isnull=False + ).values_list("key", "id") + if key is not None + } + allowed_keys = {mv["key"] for mv in environment_multivariate_values if "key" in mv} + allowed_option_ids = { + mv["multivariate_feature_option"] + for mv in environment_multivariate_values + if "multivariate_feature_option" in mv + } | {option_id_by_key[key] for key in allowed_keys if key in option_id_by_key} + for multivariate_values in override_multivariate_value_lists: + for mv in multivariate_values: + if "multivariate_feature_option" in mv: + allowed = mv["multivariate_feature_option"] in allowed_option_ids + else: + allowed = ( + mv["key"] in allowed_keys + or option_id_by_key.get(mv["key"]) in allowed_option_ids + ) + if not allowed: + raise serializers.ValidationError( + { + "multivariate_feature_state_values": [ + "Segment overrides can only re-weight existing variants." + ] + } + ) + + +def _validate_new_keyed_options_have_values( + feature: Feature, + multivariate_values: Sequence[EnvironmentMultivariateValuePayload], +) -> None: + keys_without_value = { + mv["key"] for mv in multivariate_values if "key" in mv and "value" not in mv + } + if not keys_without_value: + return + existing_keys = set( + feature.multivariate_options.filter(key__in=keys_without_value).values_list( + "key", flat=True + ) + ) + if keys_without_value - existing_keys: + raise serializers.ValidationError( + { + "multivariate_feature_state_values": [ + "A new multivariate option requires a 'value'." + ] + } + ) + + def _to_environment_multivariate_value_change_set( multivariate_value: EnvironmentMultivariateValuePayload, ) -> EnvironmentMultivariateValueChangeSet: @@ -193,6 +320,17 @@ def _to_environment_multivariate_value_change_set( else None ), ) + if "key" in multivariate_value: + value = multivariate_value.get("value") + return KeyedMultivariateOptionChangeSet( + key=multivariate_value["key"], + 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"], @@ -222,12 +360,13 @@ def validate(self, attrs: UpdateFlagOptionAPayload) -> UpdateFlagOptionAPayload: if "segment" in attrs: if any( - "multivariate_feature_option" not in mv for mv in multivariate_values + "multivariate_feature_option" not in mv and "key" not in mv + for mv in multivariate_values ): raise serializers.ValidationError( { "multivariate_feature_state_values": [ - "Segment overrides require a variant 'id'." + "Segment overrides require a variant 'id' or 'key'." ] } ) @@ -254,7 +393,11 @@ def validate(self, attrs: UpdateFlagOptionAPayload) -> UpdateFlagOptionAPayload: ) except Feature.DoesNotExist: return attrs - validate_multivariate_state_values(feature, multivariate_values) + if "segment" in attrs: + _validate_multivariate_keys_exist(feature, multivariate_values) + else: + _validate_new_keyed_options_have_values(feature, multivariate_values) + _validate_multivariate_option_references(feature, multivariate_values) return attrs @@ -265,7 +408,9 @@ def change_set_option_a(self) -> FlagChangeSetOptionA: segment_data = validated_data.get("segment") multivariate_values = validated_data.get("multivariate_feature_state_values") - segment_multivariate_values: list[MultivariateValueChangeSet] | None = None + segment_multivariate_values: list[SegmentMultivariateValueChangeSet] | None = ( + None + ) environment_multivariate_values: ( list[EnvironmentMultivariateValueChangeSet] | None ) = None @@ -278,8 +423,12 @@ def change_set_option_a(self) -> FlagChangeSetOptionA: ], percentage_allocation=mv["percentage_allocation"], ) - for mv in multivariate_values if "multivariate_feature_option" in mv + else MultivariateKeyValueChangeSet( + key=mv["key"], + percentage_allocation=mv["percentage_allocation"], + ) + for mv in multivariate_values ] else: environment_multivariate_values = [ @@ -350,65 +499,59 @@ def validate_segment_overrides( return value - def validate(self, attrs: UpdateFlagOptionBPayload) -> UpdateFlagOptionBPayload: + @staticmethod + def _collect_override_multivariate_values( + overrides: Sequence[SegmentOverridePayload], + ) -> list[Sequence[SegmentOverrideMultivariateValuePayload]]: multivariate_value_lists: list[ - Sequence[ - EnvironmentMultivariateValuePayload - | SegmentOverrideMultivariateValuePayload - ] + Sequence[SegmentOverrideMultivariateValuePayload] ] = [] + for override in overrides: + multivariate_values = override.get("multivariate_feature_state_values") + if multivariate_values is None: + continue + 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." + ] + } + ) + multivariate_value_lists.append(multivariate_values) + return multivariate_value_lists - environment_option_ids: set[int] | None = None + def validate(self, attrs: UpdateFlagOptionBPayload) -> UpdateFlagOptionBPayload: + environment_multivariate_values: ( + Sequence[EnvironmentMultivariateValuePayload] | None + ) = None if (environment_default := attrs.get("environment_default")) is not None: - environment_default_multivariate_values = environment_default.get( + environment_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 - ) - > 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 + if environment_multivariate_values is not None and ( + sum( + mv["percentage_allocation"] + for mv in environment_multivariate_values ) + > 100 ): raise serializers.ValidationError( { "multivariate_feature_state_values": [ - "Segment overrides can only re-weight existing variants." + "Multivariate percentage values exceed 100%." ] } ) - multivariate_value_lists.append(override_multivariate_values) - if not multivariate_value_lists: + override_multivariate_value_lists = self._collect_override_multivariate_values( + attrs.get("segment_overrides") or [] + ) + + if ( + environment_multivariate_values is None + and not override_multivariate_value_lists + ): return attrs try: @@ -417,8 +560,23 @@ def validate(self, attrs: UpdateFlagOptionBPayload) -> UpdateFlagOptionBPayload: ) except Feature.DoesNotExist: return attrs - for multivariate_values in multivariate_value_lists: - validate_multivariate_state_values(feature, multivariate_values) + if environment_multivariate_values is not None: + _validate_new_keyed_options_have_values( + feature, environment_multivariate_values + ) + _validate_multivariate_option_references( + feature, environment_multivariate_values + ) + _validate_overrides_reweight_environment_options( + feature, + environment_multivariate_values, + override_multivariate_value_lists, + ) + else: + for multivariate_values in override_multivariate_value_lists: + _validate_multivariate_keys_exist(feature, multivariate_values) + for multivariate_values in override_multivariate_value_lists: + _validate_multivariate_option_references(feature, multivariate_values) return attrs @@ -477,6 +635,11 @@ def change_set_option_b(self) -> FlagChangeSetOptionB: ], percentage_allocation=mv["percentage_allocation"], ) + if "multivariate_feature_option" in mv + else MultivariateKeyValueChangeSet( + key=mv["key"], + percentage_allocation=mv["percentage_allocation"], + ) for mv in override_multivariate_data ] if override_multivariate_data is not None diff --git a/api/features/feature_states/types.py b/api/features/feature_states/types.py index eb2446810547..0252564809eb 100644 --- a/api/features/feature_states/types.py +++ b/api/features/feature_states/types.py @@ -35,21 +35,43 @@ class BaseMultivariateValuePayload(TypedDict): class NewMultivariateOptionPayload(BaseMultivariateValuePayload): multivariate_feature_option: NotRequired[Never] + key: NotRequired[Never] value: FeatureValuePayload +class KeyedMultivariateOptionPayload(BaseMultivariateValuePayload): + multivariate_feature_option: NotRequired[Never] + key: str + value: NotRequired[FeatureValuePayload] + + class MultivariateOptionUpdatePayload(BaseMultivariateValuePayload): multivariate_feature_option: int + key: NotRequired[Never] value: NotRequired[FeatureValuePayload] EnvironmentMultivariateValuePayload: TypeAlias = ( - NewMultivariateOptionPayload | MultivariateOptionUpdatePayload + NewMultivariateOptionPayload + | KeyedMultivariateOptionPayload + | MultivariateOptionUpdatePayload ) -class SegmentOverrideMultivariateValuePayload(BaseMultivariateValuePayload): +class SegmentOverrideMultivariateValueByIdPayload(BaseMultivariateValuePayload): multivariate_feature_option: int + key: NotRequired[Never] + + +class SegmentOverrideMultivariateValueByKeyPayload(BaseMultivariateValuePayload): + multivariate_feature_option: NotRequired[Never] + key: str + + +SegmentOverrideMultivariateValuePayload: TypeAlias = ( + SegmentOverrideMultivariateValueByIdPayload + | SegmentOverrideMultivariateValueByKeyPayload +) class UpdateFlagOptionAPayload(TypedDict): diff --git a/api/features/versioning/dataclasses.py b/api/features/versioning/dataclasses.py index 96b646c87868..ea9dc2f47435 100644 --- a/api/features/versioning/dataclasses.py +++ b/api/features/versioning/dataclasses.py @@ -1,5 +1,6 @@ from __future__ import annotations +from collections.abc import Sequence from dataclasses import dataclass, field from datetime import datetime from typing import TypeAlias @@ -35,7 +36,7 @@ class FlagChangeSetOptionA: segment_id: int | None = None segment_priority: int | None = None - multivariate_values: list[MultivariateValueChangeSet] | None = None + multivariate_values: Sequence[SegmentMultivariateValueChangeSet] | None = None environment_multivariate_values: ( list[EnvironmentMultivariateValueChangeSet] | None ) = None @@ -47,12 +48,30 @@ class MultivariateValueChangeSet: percentage_allocation: float +@dataclass +class MultivariateKeyValueChangeSet: + key: str + percentage_allocation: float + + +SegmentMultivariateValueChangeSet: TypeAlias = ( + MultivariateValueChangeSet | MultivariateKeyValueChangeSet +) + + @dataclass class NewMultivariateOptionChangeSet: percentage_allocation: float value: FeatureValue +@dataclass +class KeyedMultivariateOptionChangeSet: + key: str + percentage_allocation: float + value: FeatureValue | None = None + + @dataclass class MultivariateOptionUpdateChangeSet: multivariate_feature_option_id: int @@ -61,7 +80,9 @@ class MultivariateOptionUpdateChangeSet: EnvironmentMultivariateValueChangeSet: TypeAlias = ( - NewMultivariateOptionChangeSet | MultivariateOptionUpdateChangeSet + NewMultivariateOptionChangeSet + | KeyedMultivariateOptionChangeSet + | MultivariateOptionUpdateChangeSet ) @@ -71,7 +92,7 @@ class SegmentOverrideChangeSet: enabled: bool | None = None value: FeatureValue | None = None priority: int | None = None - multivariate_values: list[MultivariateValueChangeSet] | None = None + multivariate_values: Sequence[SegmentMultivariateValueChangeSet] | None = None @dataclass diff --git a/api/features/versioning/versioning_service.py b/api/features/versioning/versioning_service.py index 77edc80a5187..32a249b7ba07 100644 --- a/api/features/versioning/versioning_service.py +++ b/api/features/versioning/versioning_service.py @@ -1,4 +1,5 @@ import typing +from collections.abc import Sequence from common.core.utils import using_database_replica from django.db import transaction @@ -19,9 +20,12 @@ FeatureValue, FlagChangeSetOptionA, FlagChangeSetOptionB, + KeyedMultivariateOptionChangeSet, + MultivariateKeyValueChangeSet, MultivariateOptionUpdateChangeSet, MultivariateValueChangeSet, NewMultivariateOptionChangeSet, + SegmentMultivariateValueChangeSet, ) from features.versioning.exceptions import DirectFeatureStateWriteNotAllowedError from features.versioning.models import EnvironmentFeatureVersion @@ -327,18 +331,50 @@ def _update_feature_state_value(fsv: FeatureStateValue, value: FeatureValue) -> def update_multivariate_values( feature_state: FeatureState, - values: list[MultivariateValueChangeSet] | None, + values: Sequence[SegmentMultivariateValueChangeSet] | None, ) -> None: if values is None: return + keys = { + value.key + for value in values + if isinstance(value, MultivariateKeyValueChangeSet) + } + option_id_by_key: dict[str, int] = {} + if keys: + option_id_by_key = { + key: option_id + for key, option_id in MultivariateFeatureOption.objects.filter( + feature_id=feature_state.feature_id, key__in=keys + ).values_list("key", "id") + if key is not None + } + if unknown_keys := keys - option_id_by_key.keys(): + raise ValidationError( + f"Multivariate keys {sorted(unknown_keys)} do not belong to the feature" + ) + resolved_values = [ + value + if isinstance(value, MultivariateValueChangeSet) + else MultivariateValueChangeSet( + multivariate_feature_option_id=option_id_by_key[value.key], + percentage_allocation=value.percentage_allocation, + ) + for value in values + ] + existing = { mv.multivariate_feature_option_id: mv for mv in feature_state.multivariate_feature_state_values.all() } - passed_option_ids = {value.multivariate_feature_option_id for value in values} - effective_total = sum(value.percentage_allocation for value in values) + sum( + passed_option_ids = { + value.multivariate_feature_option_id for value in resolved_values + } + effective_total = sum( + value.percentage_allocation for value in resolved_values + ) + sum( mv.percentage_allocation for option_id, mv in existing.items() if option_id not in passed_option_ids @@ -349,7 +385,7 @@ def update_multivariate_values( f"100%, got {effective_total}%." ) - for value in values: + for value in resolved_values: mv = existing.get(value.multivariate_feature_option_id) if mv is None: MultivariateFeatureStateValue.objects.create( @@ -379,8 +415,15 @@ def update_environment_multivariate_options( for value in values if isinstance(value, MultivariateOptionUpdateChangeSet) } + listed_keys = { + value.key + for value in values + if isinstance(value, KeyedMultivariateOptionChangeSet) + } # Instance-level deletes so django-lifecycle hooks fire - for option in feature.multivariate_options.exclude(id__in=listed_option_ids): + for option in feature.multivariate_options.all(): + if option.id in listed_option_ids or option.key in listed_keys: + continue option.delete() reweighted_values = [] @@ -392,6 +435,8 @@ def update_environment_multivariate_options( ) option.set_value(value.value.value, value.value.type_) option.save() + elif isinstance(value, KeyedMultivariateOptionChangeSet): + option = _upsert_multivariate_option_by_key(feature, value) else: option = feature.multivariate_options.get( id=value.multivariate_feature_option_id @@ -410,6 +455,30 @@ def update_environment_multivariate_options( update_multivariate_values(feature_state, reweighted_values) +def _upsert_multivariate_option_by_key( + feature: Feature, + value: KeyedMultivariateOptionChangeSet, +) -> MultivariateFeatureOption: + try: + option = feature.multivariate_options.get(key=value.key) + except MultivariateFeatureOption.DoesNotExist: + if value.value is None: + raise ValidationError("A new multivariate option requires a 'value'.") + option = MultivariateFeatureOption( + feature=feature, + key=value.key, + default_percentage_allocation=value.percentage_allocation, + ) + option.set_value(value.value.value, value.value.type_) + option.save() + return option + option.default_percentage_allocation = value.percentage_allocation + if value.value is not None: + option.set_value(value.value.value, value.value.type_) + option.save() + return option + + def _create_segment_override( feature: Feature, environment: Environment, diff --git a/api/tests/integration/conftest.py b/api/tests/integration/conftest.py index 18051756cd0c..7eeea85e49d4 100644 --- a/api/tests/integration/conftest.py +++ b/api/tests/integration/conftest.py @@ -25,6 +25,11 @@ def mv_option_value(): # type: ignore[no-untyped-def] return "test_mv_value" +@pytest.fixture() +def mv_option_key() -> str: + return "test-mv-key" + + @pytest.fixture() def django_client(): # type: ignore[no-untyped-def] return DjangoClient() @@ -304,9 +309,11 @@ def feature_2(admin_client, project, default_feature_value, feature_2_name): # @pytest.fixture() -def mv_option_50_percent(project, admin_client, feature, mv_option_value): # type: ignore[no-untyped-def] +def mv_option_50_percent( # type: ignore[no-untyped-def] + project, admin_client, feature, mv_option_value, mv_option_key +): return create_mv_option_with_api( - admin_client, project, feature, 50, mv_option_value + admin_client, project, feature, 50, mv_option_value, key=mv_option_key ) diff --git a/api/tests/integration/edge_api/identities/test_edge_identity_featurestates_viewset.py b/api/tests/integration/edge_api/identities/test_edge_identity_featurestates_viewset.py index 001e025a9957..ef34968a632e 100644 --- a/api/tests/integration/edge_api/identities/test_edge_identity_featurestates_viewset.py +++ b/api/tests/integration/edge_api/identities/test_edge_identity_featurestates_viewset.py @@ -595,6 +595,7 @@ def test_edge_identities_update_mv_featurestate__new_allocation__updates_documen feature: Feature, mv_option_50_percent: MultivariateFeatureOption, mv_option_value: str, + mv_option_key: str, webhook_mock: mock.MagicMock, flagsmith_identities_table: Table, ): @@ -661,7 +662,7 @@ def test_edge_identities_update_mv_featurestate__new_allocation__updates_documen "multivariate_feature_option": { "id": mv_option_50_percent, "value": mv_option_value, - "key": None, + "key": mv_option_key, }, "mv_fs_value_uuid": mock.ANY, } diff --git a/api/tests/integration/features/versioning/test_update_flag_endpoints.py b/api/tests/integration/features/versioning/test_update_flag_endpoints.py index afa59eca30da..63a89342506a 100644 --- a/api/tests/integration/features/versioning/test_update_flag_endpoints.py +++ b/api/tests/integration/features/versioning/test_update_flag_endpoints.py @@ -15,6 +15,7 @@ from tests.integration.helpers import create_mv_option_with_api FeatureUpdatePayload: TypeAlias = dict[str, Any] +VariantIdentifier: TypeAlias = dict[str, Any] UpdateFlagEndpointOption = Literal["update-flag-v1", "update-flag-v2"] @@ -38,6 +39,7 @@ def versioned_environment( "value": {"type": "string", "value": "control"}, "multivariate_feature_state_values": [ { + "key": "one-half", "percentage_allocation": 50, "value": {"type": "string", "value": "half"}, }, @@ -57,6 +59,7 @@ def versioned_environment( "value": {"type": "string", "value": "control"}, "multivariate_feature_state_values": [ { + "key": "one-half", "percentage_allocation": 50, "value": {"type": "string", "value": "half"}, }, @@ -99,18 +102,33 @@ def test_update_flag__environment_defaults__adds_multivariate_options( "multivariate_feature_option__string_value", "percentage_allocation" ) ) == {"half": 50, "bit more": 10} + assert dict( + MultivariateFeatureOption.objects.filter(feature_id=feature).values_list( + "string_value", "key" + ) + ) == {"half": "one-half", "bit more": None} +@pytest.mark.parametrize( + "variant_identifier", + [ + pytest.param( + lambda option_id, option_key: {"multivariate_feature_option": option_id}, + id="by_id", + ), + pytest.param(lambda option_id, option_key: {"key": option_key}, id="by_key"), + ], +) @pytest.mark.parametrize( "endpoint,payload", [ pytest.param( "update-flag-v1", - lambda feature, option: { + lambda feature, identifier: { "feature": {"id": feature}, "multivariate_feature_state_values": [ { - "multivariate_feature_option": option, + **identifier, "percentage_allocation": 25, "value": {"type": "string", "value": "halfer"}, }, @@ -120,12 +138,12 @@ def test_update_flag__environment_defaults__adds_multivariate_options( ), pytest.param( "update-flag-v2", - lambda feature, option: { + lambda feature, identifier: { "feature": {"id": feature}, "environment_default": { "multivariate_feature_state_values": [ { - "multivariate_feature_option": option, + **identifier, "percentage_allocation": 25, "value": {"type": "string", "value": "halfer"}, }, @@ -141,14 +159,16 @@ def test_update_flag__environment_defaults__updates_multivariate_options( environment_api_key: str, feature: int, mv_option_50_percent: int, + mv_option_key: str, versioned_environment: Environment, endpoint: UpdateFlagEndpointOption, - payload: Callable[[int, int], FeatureUpdatePayload], + payload: Callable[[int, VariantIdentifier], FeatureUpdatePayload], + variant_identifier: Callable[[int, str], VariantIdentifier], ) -> None: # Given / When response = admin_client.post( f"/api/experiments/environments/{environment_api_key}/{endpoint}/", - payload(feature, mv_option_50_percent), + payload(feature, variant_identifier(mv_option_50_percent, mv_option_key)), format="json", ) @@ -173,35 +193,39 @@ def test_update_flag__environment_defaults__updates_multivariate_options( ) +@pytest.mark.parametrize( + "variant_identifier", + [ + pytest.param( + lambda option_id, option_key: {"multivariate_feature_option": option_id}, + id="by_id", + ), + pytest.param(lambda option_id, option_key: {"key": option_key}, id="by_key"), + ], +) @pytest.mark.parametrize( "endpoint,payload", [ pytest.param( "update-flag-v1", - lambda feature, segment, option: { + lambda feature, segment, identifier: { "feature": {"id": feature}, "segment": {"id": segment}, "multivariate_feature_state_values": [ - { - "multivariate_feature_option": option, - "percentage_allocation": 80, - }, + {**identifier, "percentage_allocation": 80}, ], }, id="option_a", ), pytest.param( "update-flag-v2", - lambda feature, segment, option: { + lambda feature, segment, identifier: { "feature": {"id": feature}, "segment_overrides": [ { "segment_id": segment, "multivariate_feature_state_values": [ - { - "multivariate_feature_option": option, - "percentage_allocation": 80, - }, + {**identifier, "percentage_allocation": 80}, ], }, ], @@ -216,15 +240,19 @@ def test_update_flag__segment_override__updates_multivariate_options( feature: int, mv_option_value: str, mv_option_50_percent: int, + mv_option_key: str, segment: int, versioned_environment: Environment, endpoint: UpdateFlagEndpointOption, - payload: Callable[[int, int, int], FeatureUpdatePayload], + payload: Callable[[int, int, VariantIdentifier], FeatureUpdatePayload], + variant_identifier: Callable[[int, str], VariantIdentifier], ) -> None: # Given / When response = admin_client.post( f"/api/experiments/environments/{environment_api_key}/{endpoint}/", - payload(feature, segment, mv_option_50_percent), + payload( + feature, segment, variant_identifier(mv_option_50_percent, mv_option_key) + ), format="json", ) @@ -250,32 +278,36 @@ def test_update_flag__segment_override__updates_multivariate_options( ) == {mv_option_value: 50} +@pytest.mark.parametrize( + "variant_identifier", + [ + pytest.param( + lambda option_id, option_key: {"multivariate_feature_option": option_id}, + id="by_id", + ), + pytest.param(lambda option_id, option_key: {"key": option_key}, id="by_key"), + ], +) @pytest.mark.parametrize( "endpoint,payload", [ pytest.param( "update-flag-v1", - lambda feature, kept_option: { + lambda feature, identifier: { "feature": {"id": feature}, "multivariate_feature_state_values": [ - { - "multivariate_feature_option": kept_option, - "percentage_allocation": 50, - }, + {**identifier, "percentage_allocation": 50}, ], }, id="option_a", ), pytest.param( "update-flag-v2", - lambda feature, kept_option: { + lambda feature, identifier: { "feature": {"id": feature}, "environment_default": { "multivariate_feature_state_values": [ - { - "multivariate_feature_option": kept_option, - "percentage_allocation": 50, - }, + {**identifier, "percentage_allocation": 50}, ], }, }, @@ -289,8 +321,10 @@ def test_update_flag__environment_defaults__deletes_multivariate_options( project: int, feature: int, mv_option_50_percent: int, + mv_option_key: str, endpoint: UpdateFlagEndpointOption, - payload: Callable[[int, int], FeatureUpdatePayload], + payload: Callable[[int, VariantIdentifier], FeatureUpdatePayload], + variant_identifier: Callable[[int, str], VariantIdentifier], ) -> None: # Given deleted_option = create_mv_option_with_api( @@ -300,7 +334,7 @@ def test_update_flag__environment_defaults__deletes_multivariate_options( # When response = admin_client.post( f"/api/experiments/environments/{environment_api_key}/{endpoint}/", - payload(feature, mv_option_50_percent), + payload(feature, variant_identifier(mv_option_50_percent, mv_option_key)), format="json", ) @@ -446,6 +480,150 @@ def test_update_flag__multivariate_percentage_allocation_exceeds_100__responds_4 }, ], ), + SimpleNamespace( + id="both-id-and-key", + payload=lambda multivariate_option_id, multivariate_option_key, **_: { + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": multivariate_option_id, + "key": multivariate_option_key, + "percentage_allocation": 50, + }, + ], + }, + expected_errors=[ + { # Option A + "multivariate_feature_state_values": [ + { + "non_field_errors": [ + "Provide either 'multivariate_feature_option' or 'key', not both" + ] + } + ] + }, + { # Option B + "environment_default": { + "multivariate_feature_state_values": [ + { + "non_field_errors": [ + "Provide either 'multivariate_feature_option' or 'key', not both" + ] + } + ] + } + }, + ], + ), + SimpleNamespace( + id="new-key-without-value", + payload=lambda **_: { + "multivariate_feature_state_values": [ + {"key": "brand-new", "percentage_allocation": 50}, + ], + }, + expected_errors=[ + { + "multivariate_feature_state_values": [ + "A new multivariate option requires a 'value'." + ] + }, + ], + ), + SimpleNamespace( + id="reserved-key", + payload=lambda **_: { + "multivariate_feature_state_values": [ + { + "key": "control", + "percentage_allocation": 50, + "value": {"type": "string", "value": "variant"}, + }, + ], + }, + expected_errors=[ + { # Option A + "multivariate_feature_state_values": [ + {"key": ['"control" is a reserved variant key.']} + ] + }, + { # Option B + "environment_default": { + "multivariate_feature_state_values": [ + {"key": ['"control" is a reserved variant key.']} + ] + } + }, + ], + ), + SimpleNamespace( + id="invalid-slug-key", + payload=lambda **_: { + "multivariate_feature_state_values": [ + { + "key": "not a key!", + "percentage_allocation": 50, + "value": {"type": "string", "value": "variant"}, + }, + ], + }, + expected_errors=[ + { # Option A + "multivariate_feature_state_values": [ + { + "key": [ + 'Enter a valid "slug" consisting of letters, numbers, underscores or hyphens.' + ] + } + ] + }, + { # Option B + "environment_default": { + "multivariate_feature_state_values": [ + { + "key": [ + 'Enter a valid "slug" consisting of letters, numbers, underscores or hyphens.' + ] + } + ] + } + }, + ], + ), + SimpleNamespace( + id="duplicate-key", + payload=lambda multivariate_option_key, **_: { + "multivariate_feature_state_values": [ + {"key": multivariate_option_key, "percentage_allocation": 60}, + {"key": multivariate_option_key, "percentage_allocation": 40}, + ], + }, + expected_errors=[ + { + "multivariate_feature_state_values": [ + "Multivariate options must be unique" + ] + }, + ], + ), + SimpleNamespace( + id="duplicate-mixed-id-and-key", + payload=lambda multivariate_option_id, multivariate_option_key, **_: { + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": multivariate_option_id, + "percentage_allocation": 60, + }, + {"key": multivariate_option_key, "percentage_allocation": 40}, + ], + }, + expected_errors=[ + { + "multivariate_feature_state_values": [ + "Multivariate options must be unique" + ] + }, + ], + ), SimpleNamespace( id="duplicate", payload=lambda multivariate_option_id, **_: { @@ -494,6 +672,7 @@ def test_update_flag__invalid_environment_multivariate_options__responds_400( expected_errors: list[Any], feature: int, mv_option_50_percent: int, + mv_option_key: str, payload: Callable[..., FeatureUpdatePayload], ) -> None: # Given / When @@ -501,7 +680,10 @@ def test_update_flag__invalid_environment_multivariate_options__responds_400( f"/api/experiments/environments/{environment_api_key}/{endpoint}/", data={ "feature": {"id": feature}, - **payload(multivariate_option_id=mv_option_50_percent), + **payload( + multivariate_option_id=mv_option_50_percent, + multivariate_option_key=mv_option_key, + ), }, format="json", ) @@ -517,7 +699,7 @@ def test_update_flag__invalid_environment_multivariate_options__responds_400( test_case for scenario in [ SimpleNamespace( - id="without-id", + id="without-identifier", payload=lambda **_: { "multivariate_feature_state_values": [ { @@ -529,7 +711,43 @@ def test_update_flag__invalid_environment_multivariate_options__responds_400( expected_errors=[ { # Option A "multivariate_feature_state_values": [ - "Segment overrides require a variant 'id'." + "Segment overrides require a variant 'id' or 'key'." + ] + }, + { # Option B + "segment_overrides": [ + { + "multivariate_feature_state_values": [ + { + "non_field_errors": [ + "Segment overrides require a variant 'id' or 'key'." + ] + } + ] + } + ] + }, + ], + ), + SimpleNamespace( + id="both-id-and-key", + payload=lambda multivariate_option_id, multivariate_option_key, **_: { + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": multivariate_option_id, + "key": multivariate_option_key, + "percentage_allocation": 50, + }, + ], + }, + expected_errors=[ + { # Option A + "multivariate_feature_state_values": [ + { + "non_field_errors": [ + "Provide either 'multivariate_feature_option' or 'key', not both" + ] + } ] }, { # Option B @@ -537,8 +755,8 @@ def test_update_flag__invalid_environment_multivariate_options__responds_400( { "multivariate_feature_state_values": [ { - "multivariate_feature_option": [ - "This field is required." + "non_field_errors": [ + "Provide either 'multivariate_feature_option' or 'key', not both" ] } ] @@ -547,6 +765,21 @@ def test_update_flag__invalid_environment_multivariate_options__responds_400( }, ], ), + SimpleNamespace( + id="unknown-key", + payload=lambda **_: { + "multivariate_feature_state_values": [ + {"key": "ghost", "percentage_allocation": 50}, + ], + }, + expected_errors=[ + { + "multivariate_feature_state_values": [ + "Multivariate keys ['ghost'] do not belong to the feature" + ] + }, + ], + ), SimpleNamespace( id="unknown-id", payload=lambda **_: { @@ -587,6 +820,22 @@ def test_update_flag__invalid_environment_multivariate_options__responds_400( }, ], ), + SimpleNamespace( + id="duplicate-key", + payload=lambda multivariate_option_key, **_: { + "multivariate_feature_state_values": [ + {"key": multivariate_option_key, "percentage_allocation": 60}, + {"key": multivariate_option_key, "percentage_allocation": 40}, + ], + }, + expected_errors=[ + { + "multivariate_feature_state_values": [ + "Multivariate options must be unique" + ] + }, + ], + ), SimpleNamespace( id="with-value", payload=lambda multivariate_option_id, **_: { @@ -640,6 +889,7 @@ def test_update_flag__invalid_segment_multivariate_options__responds_400( expected_errors: list[Any], feature: int, mv_option_50_percent: int, + mv_option_key: str, payload: Callable[..., FeatureUpdatePayload], segment: int, ) -> None: @@ -648,7 +898,11 @@ def test_update_flag__invalid_segment_multivariate_options__responds_400( f"/api/experiments/environments/{environment_api_key}/{endpoint}/", data={ "feature": {"id": feature}, - **payload(multivariate_option_id=mv_option_50_percent, segment_id=segment), + **payload( + multivariate_option_id=mv_option_50_percent, + multivariate_option_key=mv_option_key, + segment_id=segment, + ), }, format="json", ) @@ -795,12 +1049,24 @@ def test_update_flag__new_segment_override_without_enabled_and_value__inherits_e assert override_feature_state.get_feature_state_value() == "control" +@pytest.mark.parametrize( + "variant_identifier", + [ + pytest.param( + lambda option_id, option_key: {"multivariate_feature_option": option_id}, + id="by_id", + ), + pytest.param(lambda option_id, option_key: {"key": option_key}, id="by_key"), + ], +) 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, + mv_option_key: str, segment: int, + variant_identifier: Callable[[int, str], VariantIdentifier], ) -> None: # Given / When response = admin_client.post( @@ -820,7 +1086,7 @@ def test_update_flag_option_b__segment_override_reweights_option_deleted_from_en "segment_id": segment, "multivariate_feature_state_values": [ { - "multivariate_feature_option": mv_option_50_percent, + **variant_identifier(mv_option_50_percent, mv_option_key), "percentage_allocation": 80, }, ], @@ -889,3 +1155,140 @@ def test_update_flag__unknown_feature_with_multivariate_options__responds_400( # Then assert response.status_code == 400 assert "not found in project" in str(response.json()) + + +def test_update_flag_option_b__keyed_variants_in_single_request__configures_environment_and_overrides_segment( + admin_client: APIClient, + environment_api_key: str, + feature: int, + segment: int, + versioned_environment: Environment, +) -> None: + # Given / When + response = admin_client.post( + f"/api/experiments/environments/{environment_api_key}/update-flag-v2/", + { + "feature": {"id": feature}, + "environment_default": { + "enabled": True, + "value": {"type": "string", "value": "control"}, + "multivariate_feature_state_values": [ + { + "key": "gold", + "percentage_allocation": 30, + "value": {"type": "string", "value": "#ffd700"}, + }, + { + "key": "silver", + "percentage_allocation": 10, + "value": {"type": "string", "value": "#c0c0c0"}, + }, + ], + }, + "segment_overrides": [ + { + "segment_id": segment, + "multivariate_feature_state_values": [ + {"key": "gold", "percentage_allocation": 100}, + {"key": "silver", "percentage_allocation": 0}, + ], + }, + ], + }, + 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=None + ).multivariate_feature_state_values.values_list( + "multivariate_feature_option__key", "percentage_allocation" + ) + ) == {"gold": 30, "silver": 10} + assert dict( + live_feature_states.get( + feature_segment__segment_id=segment + ).multivariate_feature_state_values.values_list( + "multivariate_feature_option__key", "percentage_allocation" + ) + ) == {"gold": 100, "silver": 0} + + +@pytest.mark.parametrize( + "endpoint,payload", + [ + pytest.param( + "update-flag-v1", + lambda feature: { + "feature": {"id": feature}, + "multivariate_feature_state_values": [ + { + "key": "gold", + "percentage_allocation": 30, + "value": {"type": "string", "value": "#ffd700"}, + }, + ], + }, + id="option_a", + ), + pytest.param( + "update-flag-v2", + lambda feature: { + "feature": {"id": feature}, + "environment_default": { + "multivariate_feature_state_values": [ + { + "key": "gold", + "percentage_allocation": 30, + "value": {"type": "string", "value": "#ffd700"}, + }, + ], + }, + }, + id="option_b", + ), + ], +) +def test_update_flag__repeated_keyed_multivariate_payload__preserves_variant_identities( + admin_client: APIClient, + environment_api_key: str, + feature: int, + endpoint: UpdateFlagEndpointOption, + payload: Callable[[int], FeatureUpdatePayload], +) -> None: + # Given + setup_response = admin_client.post( + f"/api/experiments/environments/{environment_api_key}/{endpoint}/", + payload(feature), + format="json", + ) + assert setup_response.status_code == 204 + option_ids = set( + MultivariateFeatureOption.objects.filter(feature_id=feature).values_list( + "id", flat=True + ) + ) + + # When + response = admin_client.post( + f"/api/experiments/environments/{environment_api_key}/{endpoint}/", + payload(feature), + format="json", + ) + + # Then + assert response.status_code == 204 + assert ( + set( + MultivariateFeatureOption.objects.filter(feature_id=feature).values_list( + "id", flat=True + ) + ) + == option_ids + ) diff --git a/api/tests/unit/features/feature_states/test_serializers.py b/api/tests/unit/features/feature_states/test_serializers.py index 18a50093651a..9dd419b63c7e 100644 --- a/api/tests/unit/features/feature_states/test_serializers.py +++ b/api/tests/unit/features/feature_states/test_serializers.py @@ -8,10 +8,9 @@ FeatureValueSerializer, UpdateFlagOptionASerializer, UpdateFlagOptionBSerializer, - validate_multivariate_state_values, ) -from features.feature_states.types import SegmentOverrideMultivariateValuePayload from features.models import Feature +from features.versioning.dataclasses import MultivariateValueChangeSet from projects.models import Project from segments.models import Segment @@ -265,16 +264,6 @@ def test_update_flag_option_b_serializer__duplicate_mv_option__returns_invalid( assert "must be unique" in str(serializer.errors) -def test_validate_multivariate_state_values__empty_list__is_noop( - feature: Feature, -) -> None: - # Given - multivariate_values: list[SegmentOverrideMultivariateValuePayload] = [] - - # When / Then no exception is raised - validate_multivariate_state_values(feature, multivariate_values) - - def test_update_flag_option_b_serializer__valid_mv_option__change_set_carries_mv( multivariate_feature: Feature, multivariate_options: list, # type: ignore[type-arg] @@ -319,5 +308,7 @@ def test_update_flag_option_b_serializer__valid_mv_option__change_set_carries_mv assert is_valid is True mv_values = change_set.segment_overrides[0].multivariate_values assert mv_values is not None - assert mv_values[0].multivariate_feature_option_id == option.id - assert mv_values[0].percentage_allocation == 75 + assert mv_values[0] == MultivariateValueChangeSet( + multivariate_feature_option_id=option.id, + percentage_allocation=75, + ) 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 9add40294731..e22e8ff28532 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 @@ -1,3 +1,4 @@ +from collections.abc import Sequence from datetime import timedelta import pytest @@ -20,7 +21,10 @@ FeatureValue, FlagChangeSetOptionA, FlagChangeSetOptionB, + KeyedMultivariateOptionChangeSet, + MultivariateKeyValueChangeSet, MultivariateValueChangeSet, + SegmentMultivariateValueChangeSet, SegmentOverrideChangeSet, ) from features.versioning.models import EnvironmentFeatureVersion @@ -640,7 +644,7 @@ def _mv_change_set( author: AuthorData, segment: Segment, *, - multivariate_values: list[MultivariateValueChangeSet] | None, + multivariate_values: Sequence[SegmentMultivariateValueChangeSet] | None, ) -> FlagChangeSetOptionB: return FlagChangeSetOptionB( author=author, @@ -928,3 +932,63 @@ def test_update_flag_option_b__retained_plus_passed_exceeds_100__raises( multivariate_values=[MultivariateValueChangeSet(option_b.id, 100.0)], ), ) + + +@pytest.mark.parametrize( + "environment_fixture_name", + ["environment", "environment_v2_versioning"], +) +def test_update_flag_option_b__segment_override_reweights_unknown_key__raises( + environment_fixture_name: str, + multivariate_feature: Feature, + multivariate_options: list[MultivariateFeatureOption], + segment: Segment, + admin_user: FFAdminUser, + request: pytest.FixtureRequest, +) -> None: + # Given + environment: Environment = request.getfixturevalue(environment_fixture_name) + change_set = _mv_change_set( + AuthorData(user=admin_user), + segment, + multivariate_values=[ + MultivariateKeyValueChangeSet(key="ghost", percentage_allocation=50.0) + ], + ) + + # When / Then + with pytest.raises(ValidationError, match="do not belong to the feature"): + update_flag_option_b(environment, multivariate_feature, change_set) + + +@pytest.mark.parametrize( + "environment_fixture_name", + ["environment", "environment_v2_versioning"], +) +def test_update_flag_option_b__new_keyed_environment_option_without_value__raises( + environment_fixture_name: str, + multivariate_feature: Feature, + multivariate_options: list[MultivariateFeatureOption], + admin_user: FFAdminUser, + request: pytest.FixtureRequest, +) -> None: + # Given + environment: Environment = request.getfixturevalue(environment_fixture_name) + change_set = FlagChangeSetOptionB( + author=AuthorData(user=admin_user), + environment_default=EnvironmentDefaultChangeSet( + enabled=True, + value=FeatureValue(type_="string", value="control"), + multivariate_values=[ + KeyedMultivariateOptionChangeSet( + key="brand-new", percentage_allocation=50.0 + ), + ], + ), + ) + + # When / Then + with pytest.raises( + ValidationError, match="A new multivariate option requires a 'value'." + ): + update_flag_option_b(environment, multivariate_feature, change_set) 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 d365b7f198ff..1448d90b0eee 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 @@ -234,6 +234,11 @@ 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. +Identify a variant by its `key` — an optional, human-readable slug, unique per feature, with `control` reserved — or +by its `id`, passed as `multivariate_feature_option`. Pick one, not both. A keyed item whose key is not on the feature +yet creates the variant, so a whole experiment can be configured without ever fetching variant `id`s. Variants created +without a key can only be identified by `id`. + **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) @@ -248,21 +253,18 @@ curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environmen "value": {"type": "string", "value": "default"}, "multivariate_feature_state_values": [ { + "key": "sharp-payments", "percentage_allocation": 20, - "value": {"type": "string", "value": "sharp_payments"} + "value": {"type": "string", "value": "Sharp Payments"} }, { + "key": "gemstone-express", "percentage_allocation": 10, - "value": {"type": "string", "value": "gemstone_express"} + "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 ' \ @@ -271,17 +273,23 @@ curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environmen "feature": {"name": "new_payment_gateway_experiment"}, "multivariate_feature_state_values": [ { - "multivariate_feature_option": 991, + "key": "sharp-payments", "percentage_allocation": 20, - "value": {"type": "string", "value": "sharp_payments"} + "value": {"type": "string", "value": "Sharp Payments"} }, { + "key": "e-z-pay", "percentage_allocation": 5, - "value": {"type": "string", "value": "e-z-pay"} + "value": {"type": "string", "value": "E-Z Pay"} } ] }' +# Variants without a key are identified by their `id`, 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-v1/' \ -H 'Authorization: Api-Key ' \ @@ -290,7 +298,7 @@ curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environmen "feature": {"name": "new_payment_gateway_experiment"}, "segment": {"id": 101, "priority": 1}, "multivariate_feature_state_values": [ - {"multivariate_feature_option": 991, "percentage_allocation": 0}, + {"key": "sharp-payments", "percentage_allocation": 0}, {"multivariate_feature_option": 993, "percentage_allocation": 100} ] }' @@ -300,7 +308,7 @@ curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environmen [`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 +# Configure a whole experiment — variants and their per-segment weights — 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' \ @@ -311,74 +319,54 @@ curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environmen "value": {"type": "string", "value": "default"}, "multivariate_feature_state_values": [ { + "key": "sharp-payments", "percentage_allocation": 20, - "value": {"type": "string", "value": "sharp_payments"} + "value": {"type": "string", "value": "Sharp Payments"} }, { + "key": "gemstone-express", "percentage_allocation": 10, - "value": {"type": "string", "value": "gemstone_express"} + "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, + "priority": 1, "multivariate_feature_state_values": [ - {"multivariate_feature_option": 991, "percentage_allocation": 50}, - {"multivariate_feature_option": 992, "percentage_allocation": 50} + {"key": "sharp-payments", "percentage_allocation": 100}, + {"key": "gemstone-express", "percentage_allocation": 0} + ] + }, + { + "segment_id": 202, + "priority": 2, + "multivariate_feature_state_values": [ + {"key": "sharp-payments", "percentage_allocation": 0}, + {"key": "gemstone-express", "percentage_allocation": 100} ] } ] }' -# Re-weight the environment default and segment overrides in a single request +# Variants without a key are identified by their `id`, 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"}, - "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} + {"key": "sharp-payments", "percentage_allocation": 50}, + {"multivariate_feature_option": 992, "percentage_allocation": 50} ] } ] diff --git a/mcp/src/flagsmith_mcp/openapi.json b/mcp/src/flagsmith_mcp/openapi.json index 4a532a02eda6..ed3070b16b18 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/SegmentOverrideMultivariateValue" + "$ref": "#/components/schemas/RolloutMultivariateValue" } } }, @@ -6799,6 +6799,24 @@ "USER" ] }, + "RolloutMultivariateValue": { + "type": "object", + "properties": { + "multivariate_feature_option": { + "type": "integer" + }, + "percentage_allocation": { + "type": "number", + "format": "double", + "maximum": 100, + "minimum": 0 + } + }, + "required": [ + "multivariate_feature_option", + "percentage_allocation" + ] + }, "Segment": { "description": "Functionality for serializers that need to handle metadata", "type": "object", @@ -6897,27 +6915,6 @@ } } }, - "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 025c40182416..56acf8604026 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -19678,6 +19678,9 @@ components: minimum: 0 multivariate_feature_option: type: integer + key: + type: string + pattern: '^[-a-zA-Z0-9_]+$' value: $ref: '#/components/schemas/FeatureValue' required: @@ -20187,7 +20190,7 @@ components: multivariate_feature_state_values: type: array items: - $ref: '#/components/schemas/SegmentOverrideMultivariateValue' + $ref: '#/components/schemas/RolloutMultivariateValue' required: - enabled - feature_state_value @@ -25858,6 +25861,19 @@ components: required: - permissions - project + RolloutMultivariateValue: + type: object + properties: + multivariate_feature_option: + type: integer + percentage_allocation: + type: number + format: double + maximum: 100 + minimum: 0 + required: + - multivariate_feature_option + - percentage_allocation RudderstackConfiguration: type: object properties: @@ -26248,10 +26264,12 @@ components: minimum: 0 multivariate_feature_option: type: integer + key: + type: string + pattern: '^[-a-zA-Z0-9_]+$' value: $ref: '#/components/schemas/FeatureValue' required: - - multivariate_feature_option - percentage_allocation SegmentReference: type: object