Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# Generated by Django 5.2.14 on 2026-06-04 05:55

import django.core.validators
import re
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("features", "0066_constrain_feature_type"),
("multivariate", "0008_make_feature_value_size_configurable"),
]

operations = [
migrations.AddField(
model_name="historicalmultivariatefeatureoption",
name="key",
field=models.CharField(
help_text="A stable, human-readable identifier for the variant.",
max_length=255,
null=True,
validators=[
django.core.validators.RegexValidator(
re.compile("^[-a-zA-Z0-9_]+\\Z"),
"Enter a valid “slug” consisting of letters, numbers, underscores or hyphens.",
"invalid",
)
],
),
),
migrations.AddField(
model_name="multivariatefeatureoption",
name="key",
field=models.CharField(
help_text="A stable, human-readable identifier for the variant.",
max_length=255,
null=True,
validators=[
django.core.validators.RegexValidator(
re.compile("^[-a-zA-Z0-9_]+\\Z"),
"Enter a valid “slug” consisting of letters, numbers, underscores or hyphens.",
"invalid",
)
],
),
),
migrations.AlterUniqueTogether(
name="multivariatefeatureoption",
unique_together={("feature", "key")},
),
]
16 changes: 15 additions & 1 deletion api/features/multivariate/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@
import uuid

from django.core.exceptions import ObjectDoesNotExist
from django.core.validators import MaxValueValidator, MinValueValidator
from django.core.validators import (
MaxValueValidator,
MinValueValidator,
validate_slug,
)
from django.db import models
from django_lifecycle import ( # type: ignore[import-untyped]
AFTER_CREATE,
Expand Down Expand Up @@ -50,6 +54,13 @@ class MultivariateFeatureOption(
related_name="multivariate_options",
)

key = models.CharField(
max_length=255,
null=True,
validators=[validate_slug],
help_text="A stable, human-readable identifier for the variant.",
)

# This field is stored at the feature level but not used here - it is transferred
# to the MultivariateFeatureStateValue on creation of a new option or when creating
# a new environment.
Expand All @@ -58,6 +69,9 @@ class MultivariateFeatureOption(
validators=[MinValueValidator(0), MaxValueValidator(100)],
)

class Meta:
unique_together = ("feature", "key")

@hook(AFTER_CREATE)
def create_multivariate_feature_state_values(self): # type: ignore[no-untyped-def]
for feature_state in self.feature.feature_states.filter(
Expand Down
30 changes: 29 additions & 1 deletion api/features/multivariate/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,12 @@ class Meta:
"string_value",
"boolean_value",
"default_percentage_allocation",
"key",
Comment thread
Zaimwa9 marked this conversation as resolved.
)
read_only_fields = ("uuid",)
# `key` is only writable via the dedicated mv-options endpoint
# (`MultivariateFeatureOptionSerializer`), where its uniqueness is
# validated.
read_only_fields = ("uuid", "key")


class MultivariateOptionValuesSerializer(serializers.ModelSerializer): # type: ignore[type-arg]
Expand Down Expand Up @@ -55,6 +59,17 @@ def get_control_value(self, obj: dict[str, typing.Any]) -> str | int | bool | No
class MultivariateFeatureOptionSerializer(NestedMultivariateFeatureOptionSerializer):
class Meta(NestedMultivariateFeatureOptionSerializer.Meta):
fields = NestedMultivariateFeatureOptionSerializer.Meta.fields + ("feature",) # type: ignore[assignment]
read_only_fields = ("uuid",) # type: ignore[assignment]
# `key` participates in the ("feature", "key") unique_together, which
# makes DRF mark it as required. It is optional (nullable), so override.
extra_kwargs = {"key": {"required": False}}

def get_unique_together_validators(self): # type: ignore[no-untyped-def]
# The auto-generated `UniqueTogetherValidator` for ("feature", "key")
# also forces `key` to be required on create. We enforce uniqueness
# ourselves in `validate()`, with the database constraint as the final
# guard, so drop it.
return []

def validate(self, attrs): # type: ignore[no-untyped-def]
attrs = super().validate(attrs)
Expand All @@ -73,8 +88,21 @@ def validate(self, attrs): # type: ignore[no-untyped-def]
{"default_percentage_allocation": "Invalid percentage allocation"}
)

self._validate_key_is_unique(attrs)

return attrs

def _validate_key_is_unique(self, attrs: dict[str, typing.Any]) -> None:
key = attrs.get("key")
if key is None:
return
if self._get_siblings(attrs["feature"]).filter(key=key).exists():
raise ValidationError(
{
"key": "Multivariate option with this key already exists for the feature."
}
)

def _get_siblings(self, feature: Feature): # type: ignore[no-untyped-def]
siblings = feature.multivariate_options.all()
if self.instance:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,237 @@ def test_create_mv_option__valid_data__returns_created( # type: ignore[no-untyp
assert set(data.items()).issubset(set(response.json().items()))


def test_create_mv_option__with_key__returns_created_with_key(
admin_client_new: APIClient,
project: int,
feature: int,
) -> None:
# Given
url = reverse(
"api-v1:projects:feature-mv-options-list",
args=[project, feature],
)
data = {
"type": "unicode",
"feature": feature,
"string_value": "bigger",
"default_percentage_allocation": 50,
"key": "control",
}
# When
response = admin_client_new.post(
url,
data=json.dumps(data),
content_type="application/json",
)
# Then
assert response.status_code == status.HTTP_201_CREATED
assert response.json()["key"] == "control"


def test_create_mv_option__duplicate_key_for_same_feature__returns_bad_request(
admin_client_new: APIClient,
project: int,
feature: int,
) -> None:
# Given
url = reverse(
"api-v1:projects:feature-mv-options-list",
args=[project, feature],
)
existing_option_data = {
"type": "unicode",
"feature": feature,
"string_value": "bigger",
"default_percentage_allocation": 50,
"key": "control",
}
assert (
admin_client_new.post(
url,
data=json.dumps(existing_option_data),
content_type="application/json",
).status_code
== status.HTTP_201_CREATED
)
duplicate_option_data = {
"type": "unicode",
"feature": feature,
"string_value": "biggest",
"default_percentage_allocation": 50,
"key": "control",
}
# When
response = admin_client_new.post(
url,
data=json.dumps(duplicate_option_data),
content_type="application/json",
)
# Then
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.json()["key"] == [
"Multivariate option with this key already exists for the feature."
]


@pytest.mark.parametrize(
"invalid_key",
["has spaces", "emoji🚀", "exclamation!", "trailing space "],
)
def test_create_mv_option__invalid_key_format__returns_bad_request(
admin_client_new: APIClient,
project: int,
feature: int,
invalid_key: str,
) -> None:
# Given
url = reverse(
"api-v1:projects:feature-mv-options-list",
args=[project, feature],
)
data = {
"type": "unicode",
"feature": feature,
"string_value": "bigger",
"default_percentage_allocation": 50,
"key": invalid_key,
}
# When
response = admin_client_new.post(
url,
data=json.dumps(data),
content_type="application/json",
)
# Then
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert "key" in response.json()


def test_create_mv_option__empty_string_key__returns_bad_request(
admin_client_new: APIClient,
project: int,
feature: int,
) -> None:
# Given - an empty string is not a valid key; clients must omit the field
# or send null for "no key"
url = reverse(
"api-v1:projects:feature-mv-options-list",
args=[project, feature],
)
data = {
"type": "unicode",
"feature": feature,
"string_value": "bigger",
"default_percentage_allocation": 50,
"key": "",
}
# When
response = admin_client_new.post(
url,
data=json.dumps(data),
content_type="application/json",
)
# Then
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.json()["key"] == ["This field may not be blank."]


def test_update_mv_option__unchanged_key__returns_ok(
admin_client_new: APIClient,
project: int,
feature: int,
) -> None:
# Given - an option which already has a key
create_url = reverse(
"api-v1:projects:feature-mv-options-list",
args=[project, feature],
)
data = {
"type": "unicode",
"feature": feature,
"string_value": "bigger",
"default_percentage_allocation": 50,
"key": "control",
}
option_id = admin_client_new.post(
create_url,
data=json.dumps(data),
content_type="application/json",
).json()["id"]

# When - the option is updated keeping its own key
update_url = reverse(
"api-v1:projects:feature-mv-options-detail",
args=[project, feature, option_id],
)
response = admin_client_new.put(
update_url,
data=json.dumps({**data, "id": option_id, "string_value": "biggest"}),
content_type="application/json",
)

# Then - the option does not collide with itself
assert response.status_code == status.HTTP_200_OK
assert response.json()["key"] == "control"


def test_update_mv_option__duplicate_sibling_key__returns_bad_request(
admin_client_new: APIClient,
project: int,
feature: int,
) -> None:
# Given - two options, one with a key
create_url = reverse(
"api-v1:projects:feature-mv-options-list",
args=[project, feature],
)
keyed_option_data = {
"type": "unicode",
"feature": feature,
"string_value": "bigger",
"default_percentage_allocation": 50,
"key": "control",
}
unkeyed_option_data = {
"type": "unicode",
"feature": feature,
"string_value": "biggest",
"default_percentage_allocation": 50,
}
assert (
admin_client_new.post(
create_url,
data=json.dumps(keyed_option_data),
content_type="application/json",
).status_code
== status.HTTP_201_CREATED
)
unkeyed_option_id = admin_client_new.post(
create_url,
data=json.dumps(unkeyed_option_data),
content_type="application/json",
).json()["id"]

# When - the unkeyed option is updated to use its sibling's key
update_url = reverse(
"api-v1:projects:feature-mv-options-detail",
args=[project, feature, unkeyed_option_id],
)
response = admin_client_new.put(
update_url,
data=json.dumps(
{**unkeyed_option_data, "id": unkeyed_option_id, "key": "control"}
),
content_type="application/json",
)

# Then
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.json()["key"] == [
"Multivariate option with this key already exists for the feature."
]


@pytest.mark.parametrize(
"client, feature_id",
[
Expand Down
Loading
Loading