From 05c82fd73c66ad5ccda5810721f58ac1848cd8dc Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 25 May 2026 11:10:30 +0200 Subject: [PATCH 01/43] feat(experimentation): add Experiment model and CRUD endpoints --- api/audit/related_object_type.py | 1 + api/environments/urls.py | 4 + api/experimentation/constants.py | 1 + api/experimentation/experiment_urls.py | 10 + .../migrations/0004_experiment.py | 94 ++++ api/experimentation/models.py | 49 ++ api/experimentation/permissions.py | 21 +- api/experimentation/serializers.py | 81 +++ api/experimentation/services.py | 31 +- api/experimentation/views.py | 86 ++- api/tests/unit/experimentation/conftest.py | 22 +- .../experimentation/test_experiment_views.py | 532 ++++++++++++++++++ 12 files changed, 924 insertions(+), 8 deletions(-) create mode 100644 api/experimentation/experiment_urls.py create mode 100644 api/experimentation/migrations/0004_experiment.py create mode 100644 api/tests/unit/experimentation/test_experiment_views.py diff --git a/api/audit/related_object_type.py b/api/audit/related_object_type.py index d43c849e574b..853011eb38f4 100644 --- a/api/audit/related_object_type.py +++ b/api/audit/related_object_type.py @@ -13,3 +13,4 @@ class RelatedObjectType(enum.Enum): FEATURE_HEALTH = "Feature health status" RELEASE_PIPELINE = "Release pipeline" WAREHOUSE_CONNECTION = "Warehouse connection" + EXPERIMENT = "Experiment" diff --git a/api/environments/urls.py b/api/environments/urls.py index 2642af9242a5..348594936a45 100644 --- a/api/environments/urls.py +++ b/api/environments/urls.py @@ -177,4 +177,8 @@ "/warehouse-connections/", include("experimentation.urls"), ), + path( + "/experiments/", + include("experimentation.experiment_urls"), + ), ] diff --git a/api/experimentation/constants.py b/api/experimentation/constants.py index a1e79a0fca8e..94f9870f881d 100644 --- a/api/experimentation/constants.py +++ b/api/experimentation/constants.py @@ -1 +1,2 @@ WAREHOUSE_CONNECTION_FLAG = "experimentation_warehouse_connection" +EXPERIMENT_FLAG = "experimental_flags" diff --git a/api/experimentation/experiment_urls.py b/api/experimentation/experiment_urls.py new file mode 100644 index 000000000000..0022909c55dd --- /dev/null +++ b/api/experimentation/experiment_urls.py @@ -0,0 +1,10 @@ +from rest_framework.routers import DefaultRouter + +from experimentation.views import ExperimentViewSet + +app_name = "experiments" + +router = DefaultRouter() +router.register(r"", ExperimentViewSet, basename="experiments") + +urlpatterns = router.urls diff --git a/api/experimentation/migrations/0004_experiment.py b/api/experimentation/migrations/0004_experiment.py new file mode 100644 index 000000000000..89924f708255 --- /dev/null +++ b/api/experimentation/migrations/0004_experiment.py @@ -0,0 +1,94 @@ +# Generated by Django 5.2.14 on 2026-05-25 08:45 + +import django.db.models.deletion +import django_lifecycle.mixins +import uuid +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("environments", "0037_add_uuid_field"), + ("experimentation", "0003_unique_connection_per_environment"), + ("features", "0066_constrain_feature_type"), + ] + + operations = [ + migrations.CreateModel( + name="Experiment", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "deleted_at", + models.DateTimeField( + blank=True, + db_index=True, + default=None, + editable=False, + null=True, + ), + ), + ( + "uuid", + models.UUIDField(default=uuid.uuid4, editable=False, unique=True), + ), + ("name", models.CharField(max_length=255)), + ("hypothesis", models.TextField()), + ( + "status", + models.CharField( + choices=[ + ("created", "Created"), + ("running", "Running"), + ("paused", "Paused"), + ("completed", "Completed"), + ], + default="created", + max_length=50, + ), + ), + ("created_at", models.DateTimeField(auto_now_add=True)), + ("updated_at", models.DateTimeField(auto_now=True)), + ("started_at", models.DateTimeField(blank=True, null=True)), + ("ended_at", models.DateTimeField(blank=True, null=True)), + ( + "environment", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="experiments", + to="environments.environment", + ), + ), + ( + "feature", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="experiments", + to="features.feature", + ), + ), + ], + options={ + "constraints": [ + models.UniqueConstraint( + condition=models.Q( + ("deleted_at__isnull", True), + models.Q(("status", "completed"), _negated=True), + ), + fields=("feature", "environment"), + name="unique_active_experiment_per_feature_env", + ) + ], + }, + bases=(django_lifecycle.mixins.LifecycleModelMixin, models.Model), + ), + ] diff --git a/api/experimentation/models.py b/api/experimentation/models.py index fd3fbf513782..4dd2dcec0ac8 100644 --- a/api/experimentation/models.py +++ b/api/experimentation/models.py @@ -1,4 +1,5 @@ from django.db import models +from django.db.models import Q from django_lifecycle import LifecycleModelMixin # type: ignore[import-untyped] from core.models import SoftDeleteExportableModel @@ -47,3 +48,51 @@ class Meta: name="unique_active_warehouse_per_env", ), ] + + +class ExperimentStatus(models.TextChoices): + CREATED = "created", "Created" + RUNNING = "running", "Running" + PAUSED = "paused", "Paused" + COMPLETED = "completed", "Completed" + + +VALID_STATUS_TRANSITIONS: dict[str, set[str]] = { + ExperimentStatus.CREATED: {ExperimentStatus.RUNNING}, + ExperimentStatus.RUNNING: {ExperimentStatus.PAUSED, ExperimentStatus.COMPLETED}, + ExperimentStatus.PAUSED: {ExperimentStatus.RUNNING, ExperimentStatus.COMPLETED}, + ExperimentStatus.COMPLETED: set(), +} + + +class Experiment(LifecycleModelMixin, SoftDeleteExportableModel): # type: ignore[misc] + environment = models.ForeignKey( + Environment, + on_delete=models.CASCADE, + related_name="experiments", + ) + feature = models.ForeignKey( + "features.Feature", + on_delete=models.CASCADE, + related_name="experiments", + ) + name = models.CharField(max_length=255) + hypothesis = models.TextField() + status = models.CharField( + max_length=50, + choices=ExperimentStatus.choices, + default=ExperimentStatus.CREATED, + ) + created_at = models.DateTimeField(auto_now_add=True) + updated_at = models.DateTimeField(auto_now=True) + started_at = models.DateTimeField(null=True, blank=True) + ended_at = models.DateTimeField(null=True, blank=True) + + class Meta: + constraints = [ + models.UniqueConstraint( + fields=["feature", "environment"], + condition=Q(deleted_at__isnull=True) & ~Q(status="completed"), + name="unique_active_experiment_per_feature_env", + ), + ] diff --git a/api/experimentation/permissions.py b/api/experimentation/permissions.py index 83e1463d1a5c..e1fdc8035101 100644 --- a/api/experimentation/permissions.py +++ b/api/experimentation/permissions.py @@ -3,7 +3,10 @@ from rest_framework.views import APIView from environments.models import Environment -from experimentation.services import is_warehouse_feature_enabled +from experimentation.services import ( + is_experiment_feature_enabled, + is_warehouse_feature_enabled, +) from users.models import FFAdminUser @@ -21,3 +24,19 @@ def has_permission(self, request: Request, view: APIView) -> bool: user: FFAdminUser = request.user # type: ignore[assignment] return user.is_environment_admin(environment) + + +class ExperimentPermission(BasePermission): + def has_permission(self, request: Request, view: APIView) -> bool: + try: + environment = Environment.objects.get( + api_key=view.kwargs.get("environment_api_key") + ) + except Environment.DoesNotExist: + return False + + if not is_experiment_feature_enabled(environment.project.organisation): + return False + + user: FFAdminUser = request.user # type: ignore[assignment] + return user.is_environment_admin(environment) diff --git a/api/experimentation/serializers.py b/api/experimentation/serializers.py index bfece9cc5242..bd93b3591717 100644 --- a/api/experimentation/serializers.py +++ b/api/experimentation/serializers.py @@ -1,14 +1,19 @@ from typing import Any +from django.utils import timezone from rest_framework import serializers from environments.models import Environment from experimentation.models import ( + VALID_STATUS_TRANSITIONS, + Experiment, + ExperimentStatus, WarehouseConnection, WarehouseConnectionStatus, WarehouseType, ) from experimentation.types import SNOWFLAKE_DEFAULTS, SnowflakeConfig +from features.feature_types import MULTIVARIATE class WarehouseConnectionSerializer(serializers.ModelSerializer): # type: ignore[type-arg] @@ -88,3 +93,79 @@ def _validate_snowflake_config(config: dict[str, Any]) -> SnowflakeConfig: **config, # type: ignore[typeddict-item] } return merged + + +class ExperimentSerializer(serializers.ModelSerializer): # type: ignore[type-arg] + class Meta: + model = Experiment + fields = ( + "id", + "feature", + "name", + "hypothesis", + "status", + "created_at", + "updated_at", + "started_at", + "ended_at", + ) + read_only_fields = ( + "id", + "created_at", + "updated_at", + "started_at", + "ended_at", + ) + + def validate_feature(self, feature: Any) -> Any: + if feature.type != MULTIVARIATE: + raise serializers.ValidationError( + "Experiments can only be created for multivariate flags." + ) + view = self.context.get("view") + if view: + environment: Environment = view._get_environment() + if feature.project_id != environment.project_id: + raise serializers.ValidationError( + "Feature does not belong to this environment's project." + ) + return feature + + def validate_status(self, status: str) -> str: + if self.instance is None: + if status != ExperimentStatus.CREATED: + raise serializers.ValidationError( + "Status must be 'created' when creating an experiment." + ) + return status + + current_status: str = self.instance.status # type: ignore[union-attr] + if status == current_status: + return status + + valid_targets = VALID_STATUS_TRANSITIONS.get(current_status, set()) + if status not in valid_targets: + raise serializers.ValidationError( + f"Cannot transition from '{current_status}' to '{status}'." + ) + return status + + def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: + if self.instance is not None and "feature" in attrs: + raise serializers.ValidationError( + {"feature": "Cannot change the feature of an existing experiment."} + ) + return attrs + + def update( + self, + instance: Experiment, + validated_data: dict[str, Any], + ) -> Experiment: + new_status = validated_data.get("status") + if new_status == ExperimentStatus.RUNNING and instance.started_at is None: + validated_data["started_at"] = timezone.now() + elif new_status == ExperimentStatus.COMPLETED: + validated_data["ended_at"] = timezone.now() + result: Experiment = super().update(instance, validated_data) + return result diff --git a/api/experimentation/services.py b/api/experimentation/services.py index fc44b501db1e..562361b38f0e 100644 --- a/api/experimentation/services.py +++ b/api/experimentation/services.py @@ -4,11 +4,11 @@ from audit.models import AuditLog from audit.related_object_type import RelatedObjectType -from experimentation.constants import WAREHOUSE_CONNECTION_FLAG +from experimentation.constants import EXPERIMENT_FLAG, WAREHOUSE_CONNECTION_FLAG from integrations.flagsmith.client import get_openfeature_client if typing.TYPE_CHECKING: - from experimentation.models import WarehouseConnection + from experimentation.models import Experiment, WarehouseConnection from organisations.models import Organisation from users.models import FFAdminUser @@ -21,6 +21,14 @@ def is_warehouse_feature_enabled(organisation: Organisation) -> bool: ) +def is_experiment_feature_enabled(organisation: Organisation) -> bool: + return get_openfeature_client().get_boolean_value( + EXPERIMENT_FLAG, + default_value=False, + evaluation_context=organisation.openfeature_evaluation_context, + ) + + def create_warehouse_audit_log( connection: WarehouseConnection, user: FFAdminUser, @@ -38,3 +46,22 @@ def create_warehouse_audit_log( f"{connection.environment.name}" ), ) + + +def create_experiment_audit_log( + experiment: Experiment, + user: FFAdminUser, + *, + action: str, +) -> None: + AuditLog.objects.create( + environment=experiment.environment, + project=experiment.environment.project, + author=user, + related_object_id=experiment.id, + related_object_type=RelatedObjectType.EXPERIMENT.name, + log=( + f"Experiment '{experiment.name}' {action} for environment " + f"{experiment.environment.name}" + ), + ) diff --git a/api/experimentation/views.py b/api/experimentation/views.py index 5f2968c25b02..a9b902ceb1c1 100644 --- a/api/experimentation/views.py +++ b/api/experimentation/views.py @@ -1,3 +1,4 @@ +from django.db.models import QuerySet from rest_framework import mixins from rest_framework.permissions import IsAuthenticated from rest_framework.request import Request @@ -5,10 +6,19 @@ from rest_framework.serializers import BaseSerializer from environments.views import NestedEnvironmentViewSet -from experimentation.models import WarehouseConnection -from experimentation.permissions import WarehouseConnectionPermission -from experimentation.serializers import WarehouseConnectionSerializer -from experimentation.services import create_warehouse_audit_log +from experimentation.models import Experiment, ExperimentStatus, WarehouseConnection +from experimentation.permissions import ( + ExperimentPermission, + WarehouseConnectionPermission, +) +from experimentation.serializers import ( + ExperimentSerializer, + WarehouseConnectionSerializer, +) +from experimentation.services import ( + create_experiment_audit_log, + create_warehouse_audit_log, +) from users.models import FFAdminUser @@ -68,3 +78,71 @@ def create(self, request: Request, *args: object, **kwargs: object) -> Response: @staticmethod def _get_user(request: Request) -> FFAdminUser: return request.user # type: ignore[return-value] + + +class ExperimentViewSet( + NestedEnvironmentViewSet[Experiment], + mixins.ListModelMixin, + mixins.CreateModelMixin, + mixins.RetrieveModelMixin, + mixins.UpdateModelMixin, + mixins.DestroyModelMixin, +): + serializer_class = ExperimentSerializer + pagination_class = None + permission_classes = [IsAuthenticated, ExperimentPermission] + model_class = Experiment + lookup_field = "id" + lookup_url_kwarg = "experiment_id" + filterset_fields: list[str] = [] + + def get_queryset(self) -> "QuerySet[Experiment]": + qs = super().get_queryset() + status_filter = self.request.query_params.get("status") + if status_filter: + qs = qs.filter(status=status_filter) + return qs + + def create(self, request: Request, *args: object, **kwargs: object) -> Response: + serializer = self.get_serializer(data=request.data) + serializer.is_valid(raise_exception=True) + + feature = serializer.validated_data["feature"] + environment = self._get_environment() + if ( + Experiment.objects.filter( + feature=feature, + environment=environment, + ) + .exclude(status=ExperimentStatus.COMPLETED) + .exists() + ): + return Response( + {"detail": "An active experiment already exists for this feature."}, + status=400, + ) + + self.perform_create(serializer) + return Response(serializer.data, status=201) + + def perform_create(self, serializer: BaseSerializer[Experiment]) -> None: + experiment: Experiment = serializer.save(environment=self._get_environment()) + create_experiment_audit_log( + experiment, self._get_user(self.request), action="created" + ) + + def perform_update(self, serializer: BaseSerializer[Experiment]) -> None: + experiment: Experiment = serializer.save() + create_experiment_audit_log( + experiment, self._get_user(self.request), action="updated" + ) + + def perform_destroy(self, instance: Experiment) -> None: + create_experiment_audit_log( + instance, self._get_user(self.request), action="deleted" + ) + instance.delete() + + @staticmethod + def _get_user(request: Request) -> FFAdminUser: + return request.user # type: ignore[return-value] diff --git a/api/tests/unit/experimentation/conftest.py b/api/tests/unit/experimentation/conftest.py index 127286d3016f..f81c029df0cd 100644 --- a/api/tests/unit/experimentation/conftest.py +++ b/api/tests/unit/experimentation/conftest.py @@ -2,7 +2,13 @@ from django.urls import reverse from environments.models import Environment -from experimentation.models import WarehouseConnection, WarehouseType +from experimentation.models import ( + Experiment, + ExperimentStatus, + WarehouseConnection, + WarehouseType, +) +from features.models import Feature @pytest.fixture() @@ -21,3 +27,17 @@ def warehouse_connection_url(environment: Environment) -> str: "api-v1:environments:experimentation:warehouse-connections-list", args=[environment.api_key], ) + + +@pytest.fixture() +def experiment( + environment: Environment, + multivariate_feature: Feature, +) -> Experiment: + return Experiment.objects.create( + environment=environment, + feature=multivariate_feature, + name="Test Experiment", + hypothesis="Test hypothesis", + status=ExperimentStatus.CREATED, + ) diff --git a/api/tests/unit/experimentation/test_experiment_views.py b/api/tests/unit/experimentation/test_experiment_views.py new file mode 100644 index 000000000000..d436d535a0e6 --- /dev/null +++ b/api/tests/unit/experimentation/test_experiment_views.py @@ -0,0 +1,532 @@ +import pytest +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APIClient + +from audit.models import AuditLog +from audit.related_object_type import RelatedObjectType +from environments.models import Environment +from experimentation.models import Experiment, ExperimentStatus +from features.models import Feature +from tests.types import EnableFeaturesFixture + +pytestmark = pytest.mark.django_db + +EXPERIMENT_FLAG = "experimental_flags" + + +def _list_url(environment: Environment) -> str: + return reverse( + "api-v1:environments:experiments:experiments-list", + args=[environment.api_key], + ) + + +def _detail_url(environment: Environment, experiment: Experiment) -> str: + return reverse( + "api-v1:environments:experiments:experiments-detail", + args=[environment.api_key, experiment.id], + ) + + +# -- Create ------------------------------------------------------------------ + + +def test_post__valid_multivariate_feature__returns_201( + admin_client: APIClient, + environment: Environment, + multivariate_feature: Feature, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = admin_client.post( + _list_url(environment), + data={ + "feature": multivariate_feature.id, + "name": "My experiment", + "hypothesis": "It will work", + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_201_CREATED + data = response.json() + assert data["feature"] == multivariate_feature.id + assert data["name"] == "My experiment" + assert data["status"] == "created" + assert data["started_at"] is None + assert data["ended_at"] is None + + +def test_post__non_multivariate_feature__returns_400( + admin_client: APIClient, + environment: Environment, + feature: Feature, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = admin_client.post( + _list_url(environment), + data={ + "feature": feature.id, + "name": "Bad experiment", + "hypothesis": "Nope", + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + + +def test_post__feature_from_different_project__returns_400( + admin_client: APIClient, + environment: Environment, + enable_features: EnableFeaturesFixture, + organisation_one_project_one_feature_one: Feature, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = admin_client.post( + _list_url(environment), + data={ + "feature": organisation_one_project_one_feature_one.id, + "name": "Wrong project", + "hypothesis": "Nope", + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + + +def test_post__active_experiment_exists__returns_400( + admin_client: APIClient, + environment: Environment, + experiment: Experiment, + multivariate_feature: Feature, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = admin_client.post( + _list_url(environment), + data={ + "feature": multivariate_feature.id, + "name": "Duplicate", + "hypothesis": "Should fail", + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + + +def test_post__completed_experiment_exists__returns_201( + admin_client: APIClient, + environment: Environment, + experiment: Experiment, + multivariate_feature: Feature, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + experiment.status = ExperimentStatus.COMPLETED + experiment.save() + + # When + response = admin_client.post( + _list_url(environment), + data={ + "feature": multivariate_feature.id, + "name": "New experiment", + "hypothesis": "Should work", + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_201_CREATED + + +# -- Permissions -------------------------------------------------------------- + + +def test_post__non_admin__returns_403( + staff_client: APIClient, + environment: Environment, + multivariate_feature: Feature, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = staff_client.post( + _list_url(environment), + data={ + "feature": multivariate_feature.id, + "name": "No access", + "hypothesis": "Nope", + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_403_FORBIDDEN + + +def test_post__feature_flag_disabled__returns_403( + admin_client: APIClient, + environment: Environment, + multivariate_feature: Feature, +) -> None: + # When + response = admin_client.post( + _list_url(environment), + data={ + "feature": multivariate_feature.id, + "name": "No flag", + "hypothesis": "Nope", + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_403_FORBIDDEN + + +# -- List & Retrieve --------------------------------------------------------- + + +def test_get_list__with_experiments__returns_all( + admin_client: APIClient, + environment: Environment, + experiment: Experiment, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = admin_client.get(_list_url(environment)) + + # Then + assert response.status_code == status.HTTP_200_OK + assert len(response.json()) == 1 + assert response.json()[0]["id"] == experiment.id + + +def test_get_list__empty__returns_200( + admin_client: APIClient, + environment: Environment, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = admin_client.get(_list_url(environment)) + + # Then + assert response.status_code == status.HTTP_200_OK + assert response.json() == [] + + +@pytest.mark.parametrize( + "filter_status, expected_count", + [ + ("created", 1), + ("running", 0), + ], +) +def test_get_list__filter_by_status__returns_filtered( + admin_client: APIClient, + environment: Environment, + experiment: Experiment, + enable_features: EnableFeaturesFixture, + filter_status: str, + expected_count: int, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = admin_client.get( + _list_url(environment), {"status": filter_status} + ) + + # Then + assert response.status_code == status.HTTP_200_OK + assert len(response.json()) == expected_count + + +def test_get_detail__exists__returns_200( + admin_client: APIClient, + environment: Environment, + experiment: Experiment, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = admin_client.get(_detail_url(environment, experiment)) + + # Then + assert response.status_code == status.HTTP_200_OK + assert response.json()["id"] == experiment.id + + +# -- Update ------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "field, value", + [ + ("name", "Updated name"), + ("hypothesis", "Updated hypothesis"), + ], +) +def test_patch__update_field__returns_200( + admin_client: APIClient, + environment: Environment, + experiment: Experiment, + enable_features: EnableFeaturesFixture, + field: str, + value: str, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = admin_client.patch( + _detail_url(environment, experiment), + data={field: value}, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_200_OK + assert response.json()[field] == value + + +@pytest.mark.parametrize( + "from_status, to_status", + [ + (ExperimentStatus.CREATED, ExperimentStatus.RUNNING), + (ExperimentStatus.RUNNING, ExperimentStatus.PAUSED), + (ExperimentStatus.RUNNING, ExperimentStatus.COMPLETED), + (ExperimentStatus.PAUSED, ExperimentStatus.RUNNING), + (ExperimentStatus.PAUSED, ExperimentStatus.COMPLETED), + ], +) +def test_patch__valid_status_transition__returns_200( + admin_client: APIClient, + environment: Environment, + experiment: Experiment, + enable_features: EnableFeaturesFixture, + from_status: str, + to_status: str, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + experiment.status = from_status + experiment.save() + + # When + response = admin_client.patch( + _detail_url(environment, experiment), + data={"status": to_status}, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_200_OK + assert response.json()["status"] == to_status + + +@pytest.mark.parametrize( + "from_status, to_status", + [ + (ExperimentStatus.CREATED, ExperimentStatus.PAUSED), + (ExperimentStatus.CREATED, ExperimentStatus.COMPLETED), + (ExperimentStatus.COMPLETED, ExperimentStatus.RUNNING), + (ExperimentStatus.COMPLETED, ExperimentStatus.CREATED), + ], +) +def test_patch__invalid_status_transition__returns_400( + admin_client: APIClient, + environment: Environment, + experiment: Experiment, + enable_features: EnableFeaturesFixture, + from_status: str, + to_status: str, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + experiment.status = from_status + experiment.save() + + # When + response = admin_client.patch( + _detail_url(environment, experiment), + data={"status": to_status}, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + + +def test_patch__change_feature__returns_400( + admin_client: APIClient, + environment: Environment, + experiment: Experiment, + feature: Feature, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = admin_client.patch( + _detail_url(environment, experiment), + data={"feature": feature.id}, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + + +def test_patch__transition_to_running__sets_started_at( + admin_client: APIClient, + environment: Environment, + experiment: Experiment, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = admin_client.patch( + _detail_url(environment, experiment), + data={"status": "running"}, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_200_OK + assert response.json()["started_at"] is not None + + +def test_patch__transition_to_completed__sets_ended_at( + admin_client: APIClient, + environment: Environment, + experiment: Experiment, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + experiment.status = ExperimentStatus.RUNNING + experiment.save() + + # When + response = admin_client.patch( + _detail_url(environment, experiment), + data={"status": "completed"}, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_200_OK + assert response.json()["ended_at"] is not None + + +# -- Delete ------------------------------------------------------------------- + + +def test_delete__exists__returns_204_and_soft_deletes( + admin_client: APIClient, + environment: Environment, + experiment: Experiment, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = admin_client.delete(_detail_url(environment, experiment)) + + # Then + assert response.status_code == status.HTTP_204_NO_CONTENT + assert not Experiment.objects.filter(id=experiment.id).exists() + assert ( + Experiment.objects.all_with_deleted().filter(id=experiment.id).exists() + ) + + +# -- Audit ------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "method, action_label", + [ + ("post", "created"), + ("patch", "updated"), + ("delete", "deleted"), + ], +) +def test_crud__creates_audit_log( + admin_client: APIClient, + environment: Environment, + experiment: Experiment, + multivariate_feature: Feature, + enable_features: EnableFeaturesFixture, + method: str, + action_label: str, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + if method == "post": + experiment.delete() + admin_client.post( + _list_url(environment), + data={ + "feature": multivariate_feature.id, + "name": "Audit test", + "hypothesis": "Check audit", + }, + format="json", + ) + elif method == "patch": + admin_client.patch( + _detail_url(environment, experiment), + data={"name": "Renamed"}, + format="json", + ) + else: + admin_client.delete(_detail_url(environment, experiment)) + + # Then + audit = AuditLog.objects.filter( + related_object_type=RelatedObjectType.EXPERIMENT.name + ).last() + assert audit is not None + assert action_label in audit.log From b27797d47b9969aaf1df4ceed064b25282682685 Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 25 May 2026 11:47:22 +0200 Subject: [PATCH 02/43] feat(experimentation): reworked-tests --- .../experimentation/test_experiment_views.py | 90 +++++-------------- 1 file changed, 22 insertions(+), 68 deletions(-) diff --git a/api/tests/unit/experimentation/test_experiment_views.py b/api/tests/unit/experimentation/test_experiment_views.py index d436d535a0e6..a9d74bfb4a3e 100644 --- a/api/tests/unit/experimentation/test_experiment_views.py +++ b/api/tests/unit/experimentation/test_experiment_views.py @@ -29,9 +29,6 @@ def _detail_url(environment: Environment, experiment: Experiment) -> str: ) -# -- Create ------------------------------------------------------------------ - - def test_post__valid_multivariate_feature__returns_201( admin_client: APIClient, environment: Environment, @@ -162,9 +159,6 @@ def test_post__completed_experiment_exists__returns_201( assert response.status_code == status.HTTP_201_CREATED -# -- Permissions -------------------------------------------------------------- - - def test_post__non_admin__returns_403( staff_client: APIClient, environment: Environment, @@ -194,7 +188,7 @@ def test_post__feature_flag_disabled__returns_403( environment: Environment, multivariate_feature: Feature, ) -> None: - # When + # Given / When response = admin_client.post( _list_url(environment), data={ @@ -209,9 +203,6 @@ def test_post__feature_flag_disabled__returns_403( assert response.status_code == status.HTTP_403_FORBIDDEN -# -- List & Retrieve --------------------------------------------------------- - - def test_get_list__with_experiments__returns_all( admin_client: APIClient, environment: Environment, @@ -251,6 +242,8 @@ def test_get_list__empty__returns_200( [ ("created", 1), ("running", 0), + ("paused", 0), + ("completed", 0), ], ) def test_get_list__filter_by_status__returns_filtered( @@ -265,9 +258,7 @@ def test_get_list__filter_by_status__returns_filtered( enable_features(EXPERIMENT_FLAG) # When - response = admin_client.get( - _list_url(environment), {"status": filter_status} - ) + response = admin_client.get(_list_url(environment), {"status": filter_status}) # Then assert response.status_code == status.HTTP_200_OK @@ -291,9 +282,6 @@ def test_get_detail__exists__returns_200( assert response.json()["id"] == experiment.id -# -- Update ------------------------------------------------------------------- - - @pytest.mark.parametrize( "field, value", [ @@ -325,22 +313,28 @@ def test_patch__update_field__returns_200( @pytest.mark.parametrize( - "from_status, to_status", + "from_status, to_status, expected_status_code", [ - (ExperimentStatus.CREATED, ExperimentStatus.RUNNING), - (ExperimentStatus.RUNNING, ExperimentStatus.PAUSED), - (ExperimentStatus.RUNNING, ExperimentStatus.COMPLETED), - (ExperimentStatus.PAUSED, ExperimentStatus.RUNNING), - (ExperimentStatus.PAUSED, ExperimentStatus.COMPLETED), + (ExperimentStatus.CREATED, ExperimentStatus.RUNNING, 200), + (ExperimentStatus.RUNNING, ExperimentStatus.PAUSED, 200), + (ExperimentStatus.RUNNING, ExperimentStatus.COMPLETED, 200), + (ExperimentStatus.PAUSED, ExperimentStatus.RUNNING, 200), + (ExperimentStatus.PAUSED, ExperimentStatus.COMPLETED, 200), + (ExperimentStatus.CREATED, ExperimentStatus.PAUSED, 400), + (ExperimentStatus.CREATED, ExperimentStatus.COMPLETED, 400), + (ExperimentStatus.COMPLETED, ExperimentStatus.RUNNING, 400), + (ExperimentStatus.COMPLETED, ExperimentStatus.CREATED, 400), + (ExperimentStatus.RUNNING, ExperimentStatus.CREATED, 400), ], ) -def test_patch__valid_status_transition__returns_200( +def test_patch__status_transition__returns_expected( admin_client: APIClient, environment: Environment, experiment: Experiment, enable_features: EnableFeaturesFixture, from_status: str, to_status: str, + expected_status_code: int, ) -> None: # Given enable_features(EXPERIMENT_FLAG) @@ -355,41 +349,9 @@ def test_patch__valid_status_transition__returns_200( ) # Then - assert response.status_code == status.HTTP_200_OK - assert response.json()["status"] == to_status - - -@pytest.mark.parametrize( - "from_status, to_status", - [ - (ExperimentStatus.CREATED, ExperimentStatus.PAUSED), - (ExperimentStatus.CREATED, ExperimentStatus.COMPLETED), - (ExperimentStatus.COMPLETED, ExperimentStatus.RUNNING), - (ExperimentStatus.COMPLETED, ExperimentStatus.CREATED), - ], -) -def test_patch__invalid_status_transition__returns_400( - admin_client: APIClient, - environment: Environment, - experiment: Experiment, - enable_features: EnableFeaturesFixture, - from_status: str, - to_status: str, -) -> None: - # Given - enable_features(EXPERIMENT_FLAG) - experiment.status = from_status - experiment.save() - - # When - response = admin_client.patch( - _detail_url(environment, experiment), - data={"status": to_status}, - format="json", - ) - - # Then - assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.status_code == expected_status_code + if expected_status_code == 200: + assert response.json()["status"] == to_status def test_patch__change_feature__returns_400( @@ -457,9 +419,6 @@ def test_patch__transition_to_completed__sets_ended_at( assert response.json()["ended_at"] is not None -# -- Delete ------------------------------------------------------------------- - - def test_delete__exists__returns_204_and_soft_deletes( admin_client: APIClient, environment: Environment, @@ -475,12 +434,7 @@ def test_delete__exists__returns_204_and_soft_deletes( # Then assert response.status_code == status.HTTP_204_NO_CONTENT assert not Experiment.objects.filter(id=experiment.id).exists() - assert ( - Experiment.objects.all_with_deleted().filter(id=experiment.id).exists() - ) - - -# -- Audit ------------------------------------------------------------------- + assert Experiment.objects.all_with_deleted().filter(id=experiment.id).exists() @pytest.mark.parametrize( @@ -491,7 +445,7 @@ def test_delete__exists__returns_204_and_soft_deletes( ("delete", "deleted"), ], ) -def test_crud__creates_audit_log( +def test_crud__any_method__creates_audit_log( admin_client: APIClient, environment: Environment, experiment: Experiment, From 43b089717a2c5bbcef63a0ee2b80026fd4dccbc8 Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 25 May 2026 11:52:15 +0200 Subject: [PATCH 03/43] feat(experimentation): type-linting --- api/experimentation/migrations/0004_experiment.py | 2 +- api/tests/unit/experimentation/conftest.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/api/experimentation/migrations/0004_experiment.py b/api/experimentation/migrations/0004_experiment.py index 89924f708255..f87c9016f289 100644 --- a/api/experimentation/migrations/0004_experiment.py +++ b/api/experimentation/migrations/0004_experiment.py @@ -1,7 +1,7 @@ # Generated by Django 5.2.14 on 2026-05-25 08:45 import django.db.models.deletion -import django_lifecycle.mixins +import django_lifecycle.mixins # type: ignore[import-untyped] import uuid from django.db import migrations, models diff --git a/api/tests/unit/experimentation/conftest.py b/api/tests/unit/experimentation/conftest.py index 365384c14d47..fbc21d8f6974 100644 --- a/api/tests/unit/experimentation/conftest.py +++ b/api/tests/unit/experimentation/conftest.py @@ -42,10 +42,11 @@ def experiment( environment: Environment, multivariate_feature: Feature, ) -> Experiment: - return Experiment.objects.create( + experiment: Experiment = Experiment.objects.create( environment=environment, feature=multivariate_feature, name="Test Experiment", hypothesis="Test hypothesis", status=ExperimentStatus.CREATED, ) + return experiment From 5d55695e1d141bce4e9747001f85a389b0969afc Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 25 May 2026 12:06:14 +0200 Subject: [PATCH 04/43] feat(experimentation): changed return status when experiment exists and use hooks to set timestamps --- api/experimentation/models.py | 20 +++++++++++++++++++ api/experimentation/serializers.py | 14 ------------- api/experimentation/views.py | 2 +- .../experimentation/test_experiment_views.py | 4 ++-- 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/api/experimentation/models.py b/api/experimentation/models.py index c2c1f9712b1e..bfe74338b652 100644 --- a/api/experimentation/models.py +++ b/api/experimentation/models.py @@ -1,8 +1,10 @@ from django.db import models from django.db.models import Q +from django.utils import timezone from django_lifecycle import ( # type: ignore[import-untyped] AFTER_CREATE, AFTER_DELETE, + BEFORE_UPDATE, LifecycleModelMixin, hook, ) @@ -117,3 +119,21 @@ class Meta: name="unique_active_experiment_per_feature_env", ), ] + + @hook( + BEFORE_UPDATE, + when="status", + was=ExperimentStatus.CREATED, + is_now=ExperimentStatus.RUNNING, + ) # type: ignore[misc] + def set_started_at(self) -> None: + if not self.started_at: + self.started_at = timezone.now() + + @hook( + BEFORE_UPDATE, + when="status", + is_now=ExperimentStatus.COMPLETED, + ) # type: ignore[misc] + def set_ended_at(self) -> None: + self.ended_at = timezone.now() diff --git a/api/experimentation/serializers.py b/api/experimentation/serializers.py index bd93b3591717..50387e23773e 100644 --- a/api/experimentation/serializers.py +++ b/api/experimentation/serializers.py @@ -1,6 +1,5 @@ from typing import Any -from django.utils import timezone from rest_framework import serializers from environments.models import Environment @@ -156,16 +155,3 @@ def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: {"feature": "Cannot change the feature of an existing experiment."} ) return attrs - - def update( - self, - instance: Experiment, - validated_data: dict[str, Any], - ) -> Experiment: - new_status = validated_data.get("status") - if new_status == ExperimentStatus.RUNNING and instance.started_at is None: - validated_data["started_at"] = timezone.now() - elif new_status == ExperimentStatus.COMPLETED: - validated_data["ended_at"] = timezone.now() - result: Experiment = super().update(instance, validated_data) - return result diff --git a/api/experimentation/views.py b/api/experimentation/views.py index a9b902ceb1c1..d7a80a6ade75 100644 --- a/api/experimentation/views.py +++ b/api/experimentation/views.py @@ -119,7 +119,7 @@ def create(self, request: Request, *args: object, **kwargs: object) -> Response: ): return Response( {"detail": "An active experiment already exists for this feature."}, - status=400, + status=409, ) self.perform_create(serializer) diff --git a/api/tests/unit/experimentation/test_experiment_views.py b/api/tests/unit/experimentation/test_experiment_views.py index a9d74bfb4a3e..3550cde7f182 100644 --- a/api/tests/unit/experimentation/test_experiment_views.py +++ b/api/tests/unit/experimentation/test_experiment_views.py @@ -107,7 +107,7 @@ def test_post__feature_from_different_project__returns_400( assert response.status_code == status.HTTP_400_BAD_REQUEST -def test_post__active_experiment_exists__returns_400( +def test_post__active_experiment_exists__returns_409( admin_client: APIClient, environment: Environment, experiment: Experiment, @@ -129,7 +129,7 @@ def test_post__active_experiment_exists__returns_400( ) # Then - assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.status_code == status.HTTP_409_CONFLICT def test_post__completed_experiment_exists__returns_201( From 4269a8a5b41ff73c2226e06360b64231cb63920d Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 25 May 2026 12:18:13 +0200 Subject: [PATCH 05/43] feat(experimentation): added test coverage --- .../experimentation/test_experiment_views.py | 102 +++++++++++++++--- 1 file changed, 90 insertions(+), 12 deletions(-) diff --git a/api/tests/unit/experimentation/test_experiment_views.py b/api/tests/unit/experimentation/test_experiment_views.py index 3550cde7f182..3d509aeada55 100644 --- a/api/tests/unit/experimentation/test_experiment_views.py +++ b/api/tests/unit/experimentation/test_experiment_views.py @@ -1,3 +1,7 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + import pytest from django.urls import reverse from rest_framework import status @@ -7,9 +11,13 @@ from audit.related_object_type import RelatedObjectType from environments.models import Environment from experimentation.models import Experiment, ExperimentStatus +from features.feature_types import MULTIVARIATE from features.models import Feature from tests.types import EnableFeaturesFixture +if TYPE_CHECKING: + from projects.models import Project + pytestmark = pytest.mark.django_db EXPERIMENT_FLAG = "experimental_flags" @@ -87,16 +95,22 @@ def test_post__feature_from_different_project__returns_400( admin_client: APIClient, environment: Environment, enable_features: EnableFeaturesFixture, - organisation_one_project_one_feature_one: Feature, + organisation_one_project_one: "Project", ) -> None: # Given enable_features(EXPERIMENT_FLAG) + other_feature = Feature.objects.create( + project=organisation_one_project_one, + name="other_mv_feature", + type=MULTIVARIATE, + initial_value="control", + ) # When response = admin_client.post( _list_url(environment), data={ - "feature": organisation_one_project_one_feature_one.id, + "feature": other_feature.id, "name": "Wrong project", "hypothesis": "Nope", }, @@ -159,8 +173,8 @@ def test_post__completed_experiment_exists__returns_201( assert response.status_code == status.HTTP_201_CREATED -def test_post__non_admin__returns_403( - staff_client: APIClient, +def test_post__explicit_non_created_status__returns_400( + admin_client: APIClient, environment: Environment, multivariate_feature: Feature, enable_features: EnableFeaturesFixture, @@ -169,31 +183,74 @@ def test_post__non_admin__returns_403( enable_features(EXPERIMENT_FLAG) # When - response = staff_client.post( + response = admin_client.post( _list_url(environment), data={ "feature": multivariate_feature.id, - "name": "No access", + "name": "Forced status", "hypothesis": "Nope", + "status": "running", }, format="json", ) # Then - assert response.status_code == status.HTTP_403_FORBIDDEN + assert response.status_code == status.HTTP_400_BAD_REQUEST -def test_post__feature_flag_disabled__returns_403( - admin_client: APIClient, +@pytest.mark.parametrize( + "client_fixture, enable_flag, expected_status", + [ + ("staff_client", True, status.HTTP_403_FORBIDDEN), + ("admin_client", False, status.HTTP_403_FORBIDDEN), + ], +) +def test_post__insufficient_permissions__returns_403( + request: pytest.FixtureRequest, environment: Environment, multivariate_feature: Feature, + enable_features: EnableFeaturesFixture, + client_fixture: str, + enable_flag: bool, + expected_status: int, ) -> None: - # Given / When - response = admin_client.post( + # Given + api_client: APIClient = request.getfixturevalue(client_fixture) + if enable_flag: + enable_features(EXPERIMENT_FLAG) + + # When + response = api_client.post( _list_url(environment), data={ "feature": multivariate_feature.id, - "name": "No flag", + "name": "No access", + "hypothesis": "Nope", + }, + format="json", + ) + + # Then + assert response.status_code == expected_status + + +def test_post__nonexistent_environment__returns_403( + admin_client: APIClient, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + url = reverse( + "api-v1:environments:experiments:experiments-list", + args=["bad_api_key"], + ) + + # When + response = admin_client.post( + url, + data={ + "feature": 999, + "name": "Bad env", "hypothesis": "Nope", }, format="json", @@ -354,6 +411,27 @@ def test_patch__status_transition__returns_expected( assert response.json()["status"] == to_status +def test_patch__same_status__returns_200( + admin_client: APIClient, + environment: Environment, + experiment: Experiment, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = admin_client.patch( + _detail_url(environment, experiment), + data={"status": "created"}, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_200_OK + assert response.json()["status"] == "created" + + def test_patch__change_feature__returns_400( admin_client: APIClient, environment: Environment, From 52fa070f94d68668c687ea2bb7464da3614c7993 Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 25 May 2026 12:35:34 +0200 Subject: [PATCH 06/43] feat(experimentation): added test coverage for existing mv features --- .../experimentation/test_experiment_views.py | 29 ++++++++++++++----- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/api/tests/unit/experimentation/test_experiment_views.py b/api/tests/unit/experimentation/test_experiment_views.py index 3d509aeada55..903cde88c57f 100644 --- a/api/tests/unit/experimentation/test_experiment_views.py +++ b/api/tests/unit/experimentation/test_experiment_views.py @@ -173,11 +173,20 @@ def test_post__completed_experiment_exists__returns_201( assert response.status_code == status.HTTP_201_CREATED -def test_post__explicit_non_created_status__returns_400( +@pytest.mark.parametrize( + "explicit_status, expected_status_code", + [ + ("created", 201), + ("running", 400), + ], +) +def test_post__explicit_status__returns_expected( admin_client: APIClient, environment: Environment, multivariate_feature: Feature, enable_features: EnableFeaturesFixture, + explicit_status: str, + expected_status_code: int, ) -> None: # Given enable_features(EXPERIMENT_FLAG) @@ -187,15 +196,15 @@ def test_post__explicit_non_created_status__returns_400( _list_url(environment), data={ "feature": multivariate_feature.id, - "name": "Forced status", - "hypothesis": "Nope", - "status": "running", + "name": "Explicit status", + "hypothesis": "Testing", + "status": explicit_status, }, format="json", ) # Then - assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.status_code == expected_status_code @pytest.mark.parametrize( @@ -436,16 +445,22 @@ def test_patch__change_feature__returns_400( admin_client: APIClient, environment: Environment, experiment: Experiment, - feature: Feature, + project: "Project", enable_features: EnableFeaturesFixture, ) -> None: # Given enable_features(EXPERIMENT_FLAG) + other_feature = Feature.objects.create( + project=project, + name="other_mv_feature", + type=MULTIVARIATE, + initial_value="control", + ) # When response = admin_client.patch( _detail_url(environment, experiment), - data={"feature": feature.id}, + data={"feature": other_feature.id}, format="json", ) From 683fc699edd7b3a49cad5663cce755c7f6b17c52 Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 25 May 2026 14:37:00 +0200 Subject: [PATCH 07/43] feat(experimentation): extracted status actions --- api/experimentation/serializers.py | 32 +-- api/experimentation/views.py | 48 +++- .../experimentation/test_experiment_views.py | 233 ++++++++---------- 3 files changed, 159 insertions(+), 154 deletions(-) diff --git a/api/experimentation/serializers.py b/api/experimentation/serializers.py index 50387e23773e..2380b8c33871 100644 --- a/api/experimentation/serializers.py +++ b/api/experimentation/serializers.py @@ -4,9 +4,7 @@ from environments.models import Environment from experimentation.models import ( - VALID_STATUS_TRANSITIONS, Experiment, - ExperimentStatus, WarehouseConnection, WarehouseConnectionStatus, WarehouseType, @@ -110,6 +108,7 @@ class Meta: ) read_only_fields = ( "id", + "status", "created_at", "updated_at", "started_at", @@ -121,33 +120,12 @@ def validate_feature(self, feature: Any) -> Any: raise serializers.ValidationError( "Experiments can only be created for multivariate flags." ) - view = self.context.get("view") - if view: - environment: Environment = view._get_environment() - if feature.project_id != environment.project_id: - raise serializers.ValidationError( - "Feature does not belong to this environment's project." - ) - return feature - - def validate_status(self, status: str) -> str: - if self.instance is None: - if status != ExperimentStatus.CREATED: - raise serializers.ValidationError( - "Status must be 'created' when creating an experiment." - ) - return status - - current_status: str = self.instance.status # type: ignore[union-attr] - if status == current_status: - return status - - valid_targets = VALID_STATUS_TRANSITIONS.get(current_status, set()) - if status not in valid_targets: + environment: Environment | None = self.context.get("environment") + if environment and feature.project_id != environment.project_id: raise serializers.ValidationError( - f"Cannot transition from '{current_status}' to '{status}'." + "Feature does not belong to this environment's project." ) - return status + return feature def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: if self.instance is not None and "feature" in attrs: diff --git a/api/experimentation/views.py b/api/experimentation/views.py index d7a80a6ade75..b7f73cc0e406 100644 --- a/api/experimentation/views.py +++ b/api/experimentation/views.py @@ -1,12 +1,20 @@ +from typing import Any + from django.db.models import QuerySet from rest_framework import mixins +from rest_framework.decorators import action from rest_framework.permissions import IsAuthenticated from rest_framework.request import Request from rest_framework.response import Response from rest_framework.serializers import BaseSerializer from environments.views import NestedEnvironmentViewSet -from experimentation.models import Experiment, ExperimentStatus, WarehouseConnection +from experimentation.models import ( + VALID_STATUS_TRANSITIONS, + Experiment, + ExperimentStatus, + WarehouseConnection, +) from experimentation.permissions import ( ExperimentPermission, WarehouseConnectionPermission, @@ -96,6 +104,11 @@ class ExperimentViewSet( lookup_url_kwarg = "experiment_id" filterset_fields: list[str] = [] + def get_serializer_context(self) -> dict[str, Any]: + context = super().get_serializer_context() + context["environment"] = self._get_environment() + return context + def get_queryset(self) -> "QuerySet[Experiment]": qs = super().get_queryset() status_filter = self.request.query_params.get("status") @@ -143,6 +156,39 @@ def perform_destroy(self, instance: Experiment) -> None: ) instance.delete() + @action(detail=True, methods=["post"]) + def start(self, request: Request, **kwargs: object) -> Response: + return self._transition_status(ExperimentStatus.RUNNING) + + @action(detail=True, methods=["post"]) + def pause(self, request: Request, **kwargs: object) -> Response: + return self._transition_status(ExperimentStatus.PAUSED) + + @action(detail=True, methods=["post"]) + def complete(self, request: Request, **kwargs: object) -> Response: + return self._transition_status(ExperimentStatus.COMPLETED) + + def _transition_status(self, target_status: str) -> Response: + experiment: Experiment = self.get_object() + valid_targets = VALID_STATUS_TRANSITIONS.get(experiment.status, set()) + if target_status not in valid_targets: + return Response( + { + "detail": ( + f"Cannot transition from '{experiment.status}' " + f"to '{target_status}'." + ) + }, + status=400, + ) + experiment.status = target_status + experiment.save() + create_experiment_audit_log( + experiment, self._get_user(self.request), action=target_status + ) + serializer = self.get_serializer(experiment) + return Response(serializer.data) + @staticmethod def _get_user(request: Request) -> FFAdminUser: return request.user # type: ignore[return-value] diff --git a/api/tests/unit/experimentation/test_experiment_views.py b/api/tests/unit/experimentation/test_experiment_views.py index 903cde88c57f..1454e55873b3 100644 --- a/api/tests/unit/experimentation/test_experiment_views.py +++ b/api/tests/unit/experimentation/test_experiment_views.py @@ -37,6 +37,13 @@ def _detail_url(environment: Environment, experiment: Experiment) -> str: ) +def _action_url(environment: Environment, experiment: Experiment, action: str) -> str: + return reverse( + f"api-v1:environments:experiments:experiments-{action}", + args=[environment.api_key, experiment.id], + ) + + def test_post__valid_multivariate_feature__returns_201( admin_client: APIClient, environment: Environment, @@ -95,7 +102,7 @@ def test_post__feature_from_different_project__returns_400( admin_client: APIClient, environment: Environment, enable_features: EnableFeaturesFixture, - organisation_one_project_one: "Project", + organisation_one_project_one: Project, ) -> None: # Given enable_features(EXPERIMENT_FLAG) @@ -173,40 +180,6 @@ def test_post__completed_experiment_exists__returns_201( assert response.status_code == status.HTTP_201_CREATED -@pytest.mark.parametrize( - "explicit_status, expected_status_code", - [ - ("created", 201), - ("running", 400), - ], -) -def test_post__explicit_status__returns_expected( - admin_client: APIClient, - environment: Environment, - multivariate_feature: Feature, - enable_features: EnableFeaturesFixture, - explicit_status: str, - expected_status_code: int, -) -> None: - # Given - enable_features(EXPERIMENT_FLAG) - - # When - response = admin_client.post( - _list_url(environment), - data={ - "feature": multivariate_feature.id, - "name": "Explicit status", - "hypothesis": "Testing", - "status": explicit_status, - }, - format="json", - ) - - # Then - assert response.status_code == expected_status_code - - @pytest.mark.parametrize( "client_fixture, enable_flag, expected_status", [ @@ -378,28 +351,55 @@ def test_patch__update_field__returns_200( assert response.json()[field] == value +def test_patch__change_feature__returns_400( + admin_client: APIClient, + environment: Environment, + experiment: Experiment, + project: Project, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + other_feature = Feature.objects.create( + project=project, + name="other_mv_feature", + type=MULTIVARIATE, + initial_value="control", + ) + + # When + response = admin_client.patch( + _detail_url(environment, experiment), + data={"feature": other_feature.id}, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + + @pytest.mark.parametrize( - "from_status, to_status, expected_status_code", + "from_status, action_name, expected_status_code", [ - (ExperimentStatus.CREATED, ExperimentStatus.RUNNING, 200), - (ExperimentStatus.RUNNING, ExperimentStatus.PAUSED, 200), - (ExperimentStatus.RUNNING, ExperimentStatus.COMPLETED, 200), - (ExperimentStatus.PAUSED, ExperimentStatus.RUNNING, 200), - (ExperimentStatus.PAUSED, ExperimentStatus.COMPLETED, 200), - (ExperimentStatus.CREATED, ExperimentStatus.PAUSED, 400), - (ExperimentStatus.CREATED, ExperimentStatus.COMPLETED, 400), - (ExperimentStatus.COMPLETED, ExperimentStatus.RUNNING, 400), - (ExperimentStatus.COMPLETED, ExperimentStatus.CREATED, 400), - (ExperimentStatus.RUNNING, ExperimentStatus.CREATED, 400), + (ExperimentStatus.CREATED, "start", 200), + (ExperimentStatus.RUNNING, "pause", 200), + (ExperimentStatus.RUNNING, "complete", 200), + (ExperimentStatus.PAUSED, "start", 200), + (ExperimentStatus.PAUSED, "complete", 200), + (ExperimentStatus.CREATED, "pause", 400), + (ExperimentStatus.CREATED, "complete", 400), + (ExperimentStatus.COMPLETED, "start", 400), + (ExperimentStatus.COMPLETED, "pause", 400), + (ExperimentStatus.RUNNING, "start", 400), ], ) -def test_patch__status_transition__returns_expected( +def test_action__status_transition__returns_expected( admin_client: APIClient, environment: Environment, experiment: Experiment, enable_features: EnableFeaturesFixture, from_status: str, - to_status: str, + action_name: str, expected_status_code: int, ) -> None: # Given @@ -408,19 +408,13 @@ def test_patch__status_transition__returns_expected( experiment.save() # When - response = admin_client.patch( - _detail_url(environment, experiment), - data={"status": to_status}, - format="json", - ) + response = admin_client.post(_action_url(environment, experiment, action_name)) # Then assert response.status_code == expected_status_code - if expected_status_code == 200: - assert response.json()["status"] == to_status -def test_patch__same_status__returns_200( +def test_action__start__sets_started_at( admin_client: APIClient, environment: Environment, experiment: Experiment, @@ -430,45 +424,33 @@ def test_patch__same_status__returns_200( enable_features(EXPERIMENT_FLAG) # When - response = admin_client.patch( - _detail_url(environment, experiment), - data={"status": "created"}, - format="json", - ) + response = admin_client.post(_action_url(environment, experiment, "start")) # Then assert response.status_code == status.HTTP_200_OK - assert response.json()["status"] == "created" + assert response.json()["started_at"] is not None -def test_patch__change_feature__returns_400( +def test_action__complete__sets_ended_at( admin_client: APIClient, environment: Environment, experiment: Experiment, - project: "Project", enable_features: EnableFeaturesFixture, ) -> None: # Given enable_features(EXPERIMENT_FLAG) - other_feature = Feature.objects.create( - project=project, - name="other_mv_feature", - type=MULTIVARIATE, - initial_value="control", - ) + experiment.status = ExperimentStatus.RUNNING + experiment.save() # When - response = admin_client.patch( - _detail_url(environment, experiment), - data={"feature": other_feature.id}, - format="json", - ) + response = admin_client.post(_action_url(environment, experiment, "complete")) # Then - assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.status_code == status.HTTP_200_OK + assert response.json()["ended_at"] is not None -def test_patch__transition_to_running__sets_started_at( +def test_delete__exists__returns_204_and_soft_deletes( admin_client: APIClient, environment: Environment, experiment: Experiment, @@ -478,18 +460,43 @@ def test_patch__transition_to_running__sets_started_at( enable_features(EXPERIMENT_FLAG) # When - response = admin_client.patch( - _detail_url(environment, experiment), - data={"status": "running"}, + response = admin_client.delete(_detail_url(environment, experiment)) + + # Then + assert response.status_code == status.HTTP_204_NO_CONTENT + assert not Experiment.objects.filter(id=experiment.id).exists() + assert Experiment.objects.all_with_deleted().filter(id=experiment.id).exists() + + +def test_post__valid_create__creates_audit_log( + admin_client: APIClient, + environment: Environment, + multivariate_feature: Feature, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + admin_client.post( + _list_url(environment), + data={ + "feature": multivariate_feature.id, + "name": "Audit test", + "hypothesis": "Check audit", + }, format="json", ) # Then - assert response.status_code == status.HTTP_200_OK - assert response.json()["started_at"] is not None + audit = AuditLog.objects.filter( + related_object_type=RelatedObjectType.EXPERIMENT.name + ).last() + assert audit is not None + assert "created" in audit.log -def test_patch__transition_to_completed__sets_ended_at( +def test_patch__valid_update__creates_audit_log( admin_client: APIClient, environment: Environment, experiment: Experiment, @@ -497,22 +504,23 @@ def test_patch__transition_to_completed__sets_ended_at( ) -> None: # Given enable_features(EXPERIMENT_FLAG) - experiment.status = ExperimentStatus.RUNNING - experiment.save() # When - response = admin_client.patch( + admin_client.patch( _detail_url(environment, experiment), - data={"status": "completed"}, + data={"name": "Renamed"}, format="json", ) # Then - assert response.status_code == status.HTTP_200_OK - assert response.json()["ended_at"] is not None + audit = AuditLog.objects.filter( + related_object_type=RelatedObjectType.EXPERIMENT.name + ).last() + assert audit is not None + assert "updated" in audit.log -def test_delete__exists__returns_204_and_soft_deletes( +def test_action__start__creates_audit_log( admin_client: APIClient, environment: Environment, experiment: Experiment, @@ -522,58 +530,31 @@ def test_delete__exists__returns_204_and_soft_deletes( enable_features(EXPERIMENT_FLAG) # When - response = admin_client.delete(_detail_url(environment, experiment)) + admin_client.post(_action_url(environment, experiment, "start")) # Then - assert response.status_code == status.HTTP_204_NO_CONTENT - assert not Experiment.objects.filter(id=experiment.id).exists() - assert Experiment.objects.all_with_deleted().filter(id=experiment.id).exists() + audit = AuditLog.objects.filter( + related_object_type=RelatedObjectType.EXPERIMENT.name + ).last() + assert audit is not None + assert "running" in audit.log -@pytest.mark.parametrize( - "method, action_label", - [ - ("post", "created"), - ("patch", "updated"), - ("delete", "deleted"), - ], -) -def test_crud__any_method__creates_audit_log( +def test_delete__valid_delete__creates_audit_log( admin_client: APIClient, environment: Environment, experiment: Experiment, - multivariate_feature: Feature, enable_features: EnableFeaturesFixture, - method: str, - action_label: str, ) -> None: # Given enable_features(EXPERIMENT_FLAG) # When - if method == "post": - experiment.delete() - admin_client.post( - _list_url(environment), - data={ - "feature": multivariate_feature.id, - "name": "Audit test", - "hypothesis": "Check audit", - }, - format="json", - ) - elif method == "patch": - admin_client.patch( - _detail_url(environment, experiment), - data={"name": "Renamed"}, - format="json", - ) - else: - admin_client.delete(_detail_url(environment, experiment)) + admin_client.delete(_detail_url(environment, experiment)) # Then audit = AuditLog.objects.filter( related_object_type=RelatedObjectType.EXPERIMENT.name ).last() assert audit is not None - assert action_label in audit.log + assert "deleted" in audit.log From 7eef7bf44a3a9001cb5b078d1b09073be49407ab Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 25 May 2026 17:15:18 +0200 Subject: [PATCH 08/43] feat(experimentation): add Experiment types to frontend --- frontend/common/types/requests.ts | 5 +++++ frontend/common/types/responses.ts | 16 ++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/frontend/common/types/requests.ts b/frontend/common/types/requests.ts index 7dd713129e41..0677227f4702 100644 --- a/frontend/common/types/requests.ts +++ b/frontend/common/types/requests.ts @@ -984,5 +984,10 @@ export type Req = { name?: string config?: Record } + getExperiments: { environmentId: string } + createExperiment: { + environmentId: string + body: { name: string; hypothesis: string; feature: number } + } // END OF TYPES } diff --git a/frontend/common/types/responses.ts b/frontend/common/types/responses.ts index b3edece5bd24..66cd73e331f7 100644 --- a/frontend/common/types/responses.ts +++ b/frontend/common/types/responses.ts @@ -571,6 +571,20 @@ export type MultivariateOption = { export type FeatureType = 'STANDARD' | 'MULTIVARIATE' +export type ExperimentStatus = 'created' | 'running' | 'paused' | 'completed' + +export type Experiment = { + id: number + name: string + hypothesis: string + feature: number + status: ExperimentStatus + created_at: string + updated_at: string + started_at: string | null + ended_at: string | null +} + export enum TagStrategy { INTERSECTION = 'INTERSECTION', UNION = 'UNION', @@ -1343,5 +1357,7 @@ export type Res = { gitlabIssues: PagedResponse gitlabMergeRequests: PagedResponse warehouseConnections: WarehouseConnection[] + experiments: Experiment[] + experiment: Experiment // END OF TYPES } From c835318f4cba28916bcd7040246f00720040193f Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 25 May 2026 17:17:42 +0200 Subject: [PATCH 09/43] feat(experimentation): add RTK Query service for experiments --- frontend/common/services/useExperiment.ts | 30 +++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 frontend/common/services/useExperiment.ts diff --git a/frontend/common/services/useExperiment.ts b/frontend/common/services/useExperiment.ts new file mode 100644 index 000000000000..9c0536d99d76 --- /dev/null +++ b/frontend/common/services/useExperiment.ts @@ -0,0 +1,30 @@ +import { Res } from 'common/types/responses' +import { Req } from 'common/types/requests' +import { service } from 'common/service' + +export const experimentService = service + .enhanceEndpoints({ addTagTypes: ['Experiment'] }) + .injectEndpoints({ + endpoints: (builder) => ({ + createExperiment: builder.mutation< + Res['experiment'], + Req['createExperiment'] + >({ + invalidatesTags: [{ id: 'LIST', type: 'Experiment' }], + query: ({ body, environmentId }) => ({ + body, + method: 'POST', + url: `environments/${environmentId}/experiments/`, + }), + }), + getExperiments: builder.query({ + providesTags: [{ id: 'LIST', type: 'Experiment' }], + query: ({ environmentId }) => ({ + url: `environments/${environmentId}/experiments/`, + }), + }), + }), + }) + +export const { useCreateExperimentMutation, useGetExperimentsQuery } = + experimentService From 17360b04fbaec51e6a806f34cd7c79f6c6feec02 Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 25 May 2026 17:20:08 +0200 Subject: [PATCH 10/43] feat(experimentation): add wizard utility components --- .../experiments/LivePreviewPanel.tsx | 18 ++++ .../experiments/WizardNavButtons.tsx | 49 +++++++++ .../components/experiments/WizardStepper.tsx | 101 ++++++++++++++++++ 3 files changed, 168 insertions(+) create mode 100644 frontend/web/components/experiments/LivePreviewPanel.tsx create mode 100644 frontend/web/components/experiments/WizardNavButtons.tsx create mode 100644 frontend/web/components/experiments/WizardStepper.tsx diff --git a/frontend/web/components/experiments/LivePreviewPanel.tsx b/frontend/web/components/experiments/LivePreviewPanel.tsx new file mode 100644 index 000000000000..5a0ae932482b --- /dev/null +++ b/frontend/web/components/experiments/LivePreviewPanel.tsx @@ -0,0 +1,18 @@ +import { FC } from 'react' + +const LivePreviewPanel: FC = () => { + return ( +
+
+
+ Live Preview +
+
+
+ ) +} + +export default LivePreviewPanel diff --git a/frontend/web/components/experiments/WizardNavButtons.tsx b/frontend/web/components/experiments/WizardNavButtons.tsx new file mode 100644 index 000000000000..ee28f673a036 --- /dev/null +++ b/frontend/web/components/experiments/WizardNavButtons.tsx @@ -0,0 +1,49 @@ +import { FC } from 'react' +import Button from 'components/base/forms/Button' +import { IonIcon } from '@ionic/react' +import { arrowBack, rocketOutline } from 'ionicons/icons' + +type WizardNavButtonsProps = { + currentStep: number + totalSteps: number + canContinue: boolean + isSubmitting?: boolean + onBack: () => void + onContinue: () => void + onLaunch: () => void +} + +const WizardNavButtons: FC = ({ + canContinue, + currentStep, + isSubmitting, + onBack, + onContinue, + onLaunch, + totalSteps, +}) => { + const isLastStep = currentStep === totalSteps - 1 + + return ( +
+ {currentStep > 0 && ( + + )} + {isLastStep ? ( + + ) : ( + + )} +
+ ) +} + +export default WizardNavButtons diff --git a/frontend/web/components/experiments/WizardStepper.tsx b/frontend/web/components/experiments/WizardStepper.tsx new file mode 100644 index 000000000000..17d3686cc38b --- /dev/null +++ b/frontend/web/components/experiments/WizardStepper.tsx @@ -0,0 +1,101 @@ +import { FC } from 'react' +import { IonIcon } from '@ionic/react' +import { checkmarkCircle } from 'ionicons/icons' +import cn from 'classnames' + +type StepDef = { + title: string + subtitle: string +} + +const STEPS: StepDef[] = [ + { + subtitle: 'Name, hypothesis, and the flag to experiment on', + title: 'Setup', + }, + { + subtitle: 'Choose who is exposed and how traffic is split', + title: 'Audience & Traffic', + }, + { subtitle: 'Pick the metrics that determine success', title: 'Measurement' }, + { + subtitle: 'Review your configuration and launch', + title: 'Review & Launch', + }, +] + +type WizardStepperProps = { + currentStep: number + completedSteps: Set + onStepClick: (step: number) => void +} + +const WizardStepper: FC = ({ + completedSteps, + currentStep, + onStepClick, +}) => { + return ( +
+ {STEPS.map((step, index) => { + const isActive = index === currentStep + const isCompleted = completedSteps.has(index) + const isClickable = isCompleted || index < currentStep + + return ( + + ) + })} +
+ ) +} + +export default WizardStepper From c7d0bc2a3ee097112c228fc1c85f5f2fe6490870 Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 25 May 2026 17:23:11 +0200 Subject: [PATCH 11/43] feat(experimentation): add SetupStep with feature flag selector --- .../experiments/steps/SetupStep.tsx | 183 ++++++++++++++++++ 1 file changed, 183 insertions(+) create mode 100644 frontend/web/components/experiments/steps/SetupStep.tsx diff --git a/frontend/web/components/experiments/steps/SetupStep.tsx b/frontend/web/components/experiments/steps/SetupStep.tsx new file mode 100644 index 000000000000..e2c8d49e7dc6 --- /dev/null +++ b/frontend/web/components/experiments/steps/SetupStep.tsx @@ -0,0 +1,183 @@ +import { FC, useMemo } from 'react' +import { ProjectFlag } from 'common/types/responses' +import { useGetFeatureListQuery } from 'common/services/useProjectFlag' +import useDebouncedSearch from 'common/useDebouncedSearch' +import Utils from 'common/utils/utils' +import Panel from 'components/base/grid/Panel' + +type SetupStepProps = { + name: string + hypothesis: string + selectedFeature: ProjectFlag | null + projectId: number + environmentId: string + onNameChange: (name: string) => void + onHypothesisChange: (hypothesis: string) => void + onFeatureSelect: (feature: ProjectFlag) => void +} + +const SetupStep: FC = ({ + environmentId, + hypothesis, + name, + onFeatureSelect, + onHypothesisChange, + onNameChange, + projectId, + selectedFeature, +}) => { + const { search, setSearchInput } = useDebouncedSearch('') + + const { data: featureList, isLoading: isFeaturesLoading } = + useGetFeatureListQuery({ + environmentId, + page: 1, + page_size: 50, + projectId, + search: search || undefined, + }) + + const multivariateFeatures = useMemo(() => { + if (!featureList?.results) return [] + return featureList.results.filter((f) => f.type === 'MULTIVARIATE') + }, [featureList]) + + const getVariationValue = ( + mv: ProjectFlag['multivariate_options'][number], + ) => { + if (mv.type === 'unicode') return mv.string_value + if (mv.type === 'int') return String(mv.integer_value ?? '') + if (mv.type === 'bool') return String(mv.boolean_value ?? '') + return '' + } + + return ( +
+ +

+ Name the experiment and capture what you're trying to learn before + picking a flag. +

+ + + + onNameChange(Utils.safeParseEventValue(e)) + } + placeholder='e.g. Checkout Button Redesign' + /> + + + +