diff --git a/api/environments/identities/views.py b/api/environments/identities/views.py index 82a6ebd6721e..ddd01d90fba9 100644 --- a/api/environments/identities/views.py +++ b/api/environments/identities/views.py @@ -33,7 +33,7 @@ IdentifyWithTraitsSerializer, IdentitySerializerWithTraitsAndSegments, ) -from features.serializers import SDKFeatureStateSerializer +from features.serializers import SDKIdentityFeatureStateSerializer from integrations.integration import identify_integrations from util.views import SDKAPIView @@ -290,7 +290,9 @@ def _get_single_feature_state_response( additional_filters=self._get_additional_filters(), ): if feature_state.feature.name == feature_name: - serializer = SDKFeatureStateSerializer(feature_state, context=context) + serializer = SDKIdentityFeatureStateSerializer( + feature_state, context=context + ) return Response( data=serializer.data, status=status.HTTP_200_OK, headers=headers ) diff --git a/api/environments/sdk/serializers.py b/api/environments/sdk/serializers.py index 23b1013eed9b..78750f2350df 100644 --- a/api/environments/sdk/serializers.py +++ b/api/environments/sdk/serializers.py @@ -19,7 +19,7 @@ from environments.sdk.types import SDKTraitData from features.serializers import ( FeatureStateSerializerFull, - SDKFeatureStateSerializer, + SDKIdentityFeatureStateSerializer, ) from integrations.integration import identify_integrations from segments.serializers import SegmentSerializerBasic @@ -138,7 +138,7 @@ class IdentifyWithTraitsSerializer( ) transient = serializers.BooleanField(write_only=True, default=False) traits = TraitSerializerBasic(required=False, many=True) - flags = SDKFeatureStateSerializer(read_only=True, many=True) + flags = SDKIdentityFeatureStateSerializer(read_only=True, many=True) sensitive_fields = ("traits",) diff --git a/api/features/constants.py b/api/features/constants.py index e520cf4f9727..b64b8b8fc31a 100644 --- a/api/features/constants.py +++ b/api/features/constants.py @@ -7,6 +7,9 @@ COMMITTED = "COMMITTED" DRAFT = "DRAFT" +# Multivariate variant reported when an identity falls through to the control value +CONTROL_VARIANT_KEY = "control" + # Tag filtering strategy UNION = "UNION" INTERSECTION = "INTERSECTION" diff --git a/api/features/serializers.py b/api/features/serializers.py index 1eca40fffe8e..87d120109a43 100644 --- a/api/features/serializers.py +++ b/api/features/serializers.py @@ -45,7 +45,7 @@ DeleteBeforeUpdateWritableNestedModelSerializer, ) -from .constants import INTERSECTION, UNION +from .constants import CONTROL_VARIANT_KEY, INTERSECTION, UNION from .feature_segments.limits import ( SEGMENT_OVERRIDE_LIMIT_EXCEEDED_MESSAGE, exceeds_segment_override_limit, @@ -53,8 +53,9 @@ from .feature_segments.serializers import ( CustomCreateSegmentOverrideFeatureSegmentSerializer, ) -from .feature_types import FEATURE_TYPE_CHOICES +from .feature_types import FEATURE_TYPE_CHOICES, MULTIVARIATE from .models import Feature, FeatureState +from .multivariate.models import MultivariateFeatureOption from .multivariate.serializers import NestedMultivariateFeatureOptionSerializer @@ -627,6 +628,27 @@ class SDKFeatureStateSerializer( ) +class SDKIdentityFeatureStateSerializer(SDKFeatureStateSerializer): + variant = serializers.SerializerMethodField() + + class Meta(SDKFeatureStateSerializer.Meta): + fields = SDKFeatureStateSerializer.Meta.fields + ("variant",) # type: ignore[assignment] + + @extend_schema_field({"type": "string", "nullable": True}) + def get_variant(self, obj: FeatureState) -> str | None: + if obj.feature.type != MULTIVARIATE: + return None + identity = self.context["identity"] + value_object = obj.get_multivariate_feature_state_value( + identity.get_hash_key( + identity.environment.use_identity_composite_key_for_hashing + ) + ) + if isinstance(value_object, MultivariateFeatureOption): + return value_object.key + return CONTROL_VARIANT_KEY + + class FeatureStateSerializerBasic(WritableNestedModelSerializer): feature_state_value = serializers.SerializerMethodField() multivariate_feature_state_values = MultivariateFeatureStateValueSerializer( diff --git a/api/tests/integration/environments/identities/test_integration_identities.py b/api/tests/integration/environments/identities/test_integration_identities.py index fa2fa0dec78d..58e865ba3dbb 100644 --- a/api/tests/integration/environments/identities/test_integration_identities.py +++ b/api/tests/integration/environments/identities/test_integration_identities.py @@ -151,6 +151,111 @@ def test_get_feature_states_for_identity__mv_percentage_allocation__returns_corr assert values_dict[multivariate_feature_id] == variant_2_value +@pytest.mark.parametrize( + "hashed_percentage, expected_variant", + ( + (variant_1_percentage_allocation - 1, "variant-1"), + (total_variance_percentage - 1, "variant-2"), + (total_variance_percentage + 1, "control"), + ), +) +@mock.patch("features.models.get_hashed_percentage_for_object_ids") +def test_get_feature_states_for_identity__mv_allocation__returns_variant( # type: ignore[no-untyped-def] + mock_get_hashed_percentage_value, + hashed_percentage, + expected_variant, + sdk_client, + admin_client, + project, + environment_api_key, + environment, + identity, + identity_identifier, +): + # Given + # a standard (non-multivariate) feature + standard_feature_id = create_feature_with_api( + client=admin_client, + project_id=project, + feature_name="standard_feature", + initial_value="control", + ) + + # and a multivariate feature with two keyed variants spanning part of the range, + # so the remainder falls through to the control + multivariate_feature_id = create_feature_with_api( + client=admin_client, + project_id=project, + feature_name="multivariate_feature", + initial_value=control_value, + feature_type=MULTIVARIATE, + ) + create_mv_option_with_api( + admin_client, + project, + multivariate_feature_id, # type: ignore[arg-type] + variant_1_percentage_allocation, + variant_1_value, + key="variant-1", + ) + create_mv_option_with_api( + admin_client, + project, + multivariate_feature_id, # type: ignore[arg-type] + variant_2_percentage_allocation, + variant_2_value, + key="variant-2", + ) + + # When + # the identity hashes into a known allocation band + mock_get_hashed_percentage_value.return_value = hashed_percentage + base_url = reverse("api-v1:sdk-identities") + url = f"{base_url}?identifier={identity_identifier}" + response = sdk_client.get(url) + + # Then + assert response.status_code == status.HTTP_200_OK + variant_by_feature = { + flag["feature"]["id"]: flag["variant"] for flag in response.json()["flags"] + } + # the multivariate flag reports the variant key (or "control" on fall-through) + assert variant_by_feature[multivariate_feature_id] == expected_variant + # and the standard flag has no variant + assert variant_by_feature[standard_feature_id] is None + + +def test_get_flags__multivariate_feature__response_excludes_variant( # type: ignore[no-untyped-def] + sdk_client, + admin_client, + project, + environment, +): + # Given - a multivariate feature with a keyed variant + multivariate_feature_id = create_feature_with_api( + client=admin_client, + project_id=project, + feature_name="multivariate_feature", + initial_value=control_value, + feature_type=MULTIVARIATE, + ) + create_mv_option_with_api( + admin_client, + project, + multivariate_feature_id, # type: ignore[arg-type] + 100, + variant_1_value, + key="variant-1", + ) + + # When - the environment flags are fetched (no identity / remote evaluation) + response = sdk_client.get(reverse("api-v1:flags")) + + # Then - the variant field is scoped to the identities endpoint only + assert response.status_code == status.HTTP_200_OK + assert all("variant" not in flag for flag in response.json()) + + def test_get_feature_states_for_identity__multiple_mv_features__single_mv_query( # type: ignore[no-untyped-def] sdk_client, admin_client, diff --git a/api/tests/integration/helpers.py b/api/tests/integration/helpers.py index fb786e3f504e..cd98edd267bc 100644 --- a/api/tests/integration/helpers.py +++ b/api/tests/integration/helpers.py @@ -1,4 +1,5 @@ import json +import typing from django.urls import reverse from django.utils.http import urlencode @@ -47,17 +48,20 @@ def create_mv_option_with_api( feature_id: str, default_percentage_allocation: float, value: str, + key: str | None = None, ) -> int: url = reverse( "api-v1:projects:feature-mv-options-list", args=[project_id, feature_id], ) - data = { + data: dict[str, typing.Any] = { "type": STRING, "feature": feature_id, "string_value": value, "default_percentage_allocation": default_percentage_allocation, } + if key is not None: + data["key"] = key response = client.post( url, data=json.dumps(data), diff --git a/api/tests/unit/features/test_unit_features_serializers.py b/api/tests/unit/features/test_unit_features_serializers.py index ee747dffe7a1..32b9bc0ec970 100644 --- a/api/tests/unit/features/test_unit_features_serializers.py +++ b/api/tests/unit/features/test_unit_features_serializers.py @@ -3,6 +3,7 @@ from core.constants import BOOLEAN, INTEGER, STRING from environments.models import Environment +from features.feature_types import STANDARD from features.models import Feature, FeatureState from features.multivariate.models import ( MultivariateFeatureOption, @@ -12,7 +13,22 @@ FeatureMVOptionsValuesResponseSerializer, MultivariateOptionValuesSerializer, ) -from features.serializers import FeatureStateSerializerBasic +from features.serializers import ( + FeatureStateSerializerBasic, + SDKIdentityFeatureStateSerializer, +) + + +def test_sdk_identity_feature_state_serializer__non_multivariate_feature__variant_is_none( + mocker: MockerFixture, +) -> None: + # Given - a standard (non-multivariate) feature state + feature_state = mocker.MagicMock() + feature_state.feature.type = STANDARD + serializer = SDKIdentityFeatureStateSerializer(context={}) + + # When / Then - non-multivariate features are not part of an experiment + assert serializer.get_variant(feature_state) is None @pytest.mark.parametrize(