From 846e4b09ab2c9fc70b9115790ec51d9a7b6bf9ba Mon Sep 17 00:00:00 2001 From: wadii Date: Fri, 12 Jun 2026 15:02:47 +0200 Subject: [PATCH 01/13] feat: accept-inline-metrics-on-experiment-create --- api/experimentation/serializers.py | 56 +++++ api/experimentation/views.py | 3 +- api/tests/unit/experimentation/conftest.py | 11 + .../test_experiment_metric_views.py | 10 - .../experimentation/test_experiment_views.py | 212 +++++++++++++++++- 5 files changed, 280 insertions(+), 12 deletions(-) diff --git a/api/experimentation/serializers.py b/api/experimentation/serializers.py index 00fbbb99fbb6..212e35803922 100644 --- a/api/experimentation/serializers.py +++ b/api/experimentation/serializers.py @@ -1,11 +1,13 @@ from typing import Any +from django.db import transaction from rest_framework import serializers from environments.models import Environment from experimentation.dataclasses import WarehouseEventStats from experimentation.metric_definitions import validate_metric_definition from experimentation.models import ( + ExpectedDirection, Experiment, ExperimentMetric, ExperimentStatus, @@ -189,7 +191,22 @@ def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: return attrs +class ExperimentMetricInlineSerializer(serializers.Serializer): # type: ignore[type-arg] + metric = serializers.PrimaryKeyRelatedField( # type: ignore[var-annotated] + queryset=Metric.objects.all(), + ) + expected_direction = serializers.ChoiceField(choices=ExpectedDirection.choices) + + class ExperimentSerializer(serializers.ModelSerializer): # type: ignore[type-arg] + # Annotated with the common base type so ExperimentListSerializer can + # override the field with a read-only representation. + metrics: serializers.BaseSerializer[Any] = ExperimentMetricInlineSerializer( + many=True, + required=False, + write_only=True, + ) + class Meta: model = Experiment fields = ( @@ -198,6 +215,7 @@ class Meta: "name", "hypothesis", "status", + "metrics", "created_at", "updated_at", "started_at", @@ -229,8 +247,41 @@ def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: raise serializers.ValidationError( {"feature": "Cannot change the feature of an existing experiment."} ) + if self.instance is not None and "metrics" in attrs: + raise serializers.ValidationError( + {"metrics": "Cannot change the metrics of an existing experiment."} + ) + self._validate_metrics(attrs.get("metrics") or []) return attrs + def _validate_metrics(self, metrics: list[dict[str, Any]]) -> None: + metric_ids = [entry["metric"].id for entry in metrics] + if len(metric_ids) != len(set(metric_ids)): + raise serializers.ValidationError( + {"metrics": "Metric can only be attached once per experiment."} + ) + environment: Environment | None = self.context.get("environment") + if environment and any( + entry["metric"].environment_id != environment.id for entry in metrics + ): + raise serializers.ValidationError( + {"metrics": "Metric must belong to the experiment's environment."} + ) + + def create(self, validated_data: dict[str, Any]) -> Experiment: + metrics: list[dict[str, Any]] = validated_data.pop("metrics", []) + with transaction.atomic(): + experiment: Experiment = super().create(validated_data) + ExperimentMetric.objects.bulk_create( + ExperimentMetric( + experiment=experiment, + metric=entry["metric"], + expected_direction=entry["expected_direction"], + ) + for entry in metrics + ) + return experiment + class ExperimentFeatureSerializer(serializers.ModelSerializer): # type: ignore[type-arg] multivariate_options = NestedMultivariateFeatureOptionSerializer( @@ -245,3 +296,8 @@ class Meta: class ExperimentListSerializer(ExperimentSerializer): feature = ExperimentFeatureSerializer(read_only=True) + metrics = ExperimentMetricSerializer( + source="experiment_metrics", + many=True, + read_only=True, + ) diff --git a/api/experimentation/views.py b/api/experimentation/views.py index e501197d3031..d5fb48aac7ec 100644 --- a/api/experimentation/views.py +++ b/api/experimentation/views.py @@ -164,7 +164,8 @@ def get_queryset(self) -> "QuerySet[Experiment]": qs = super().get_queryset() if self.action in ("list", "retrieve"): qs = qs.select_related("feature").prefetch_related( - "feature__multivariate_options" + "feature__multivariate_options", + "experiment_metrics__metric", ) status_filter = self.request.query_params.get("status") if status_filter: diff --git a/api/tests/unit/experimentation/conftest.py b/api/tests/unit/experimentation/conftest.py index fbc21d8f6974..16cb83242203 100644 --- a/api/tests/unit/experimentation/conftest.py +++ b/api/tests/unit/experimentation/conftest.py @@ -7,6 +7,7 @@ from experimentation.models import ( Experiment, ExperimentStatus, + Metric, WarehouseConnection, WarehouseType, ) @@ -37,6 +38,16 @@ def warehouse_connection_url(environment: Environment) -> str: ) +@pytest.fixture() +def metric(environment: Environment) -> Metric: + metric: Metric = Metric.objects.create( + environment=environment, + name="Sessions per User", + definition={"version": 1, "event": "session_started"}, + ) + return metric + + @pytest.fixture() def experiment( environment: Environment, diff --git a/api/tests/unit/experimentation/test_experiment_metric_views.py b/api/tests/unit/experimentation/test_experiment_metric_views.py index 7d3091401c1c..8b2880728f92 100644 --- a/api/tests/unit/experimentation/test_experiment_metric_views.py +++ b/api/tests/unit/experimentation/test_experiment_metric_views.py @@ -40,16 +40,6 @@ def _detail_url( ) -@pytest.fixture() -def metric(environment: Environment) -> Metric: - metric: Metric = Metric.objects.create( - environment=environment, - name="Sessions per User", - definition={"version": 1, "event": "session_started"}, - ) - return metric - - def test_attach_metric__valid__returns_201( admin_client_new: APIClient, environment: Environment, diff --git a/api/tests/unit/experimentation/test_experiment_views.py b/api/tests/unit/experimentation/test_experiment_views.py index 6910544876fd..430b8bcaff12 100644 --- a/api/tests/unit/experimentation/test_experiment_views.py +++ b/api/tests/unit/experimentation/test_experiment_views.py @@ -13,7 +13,13 @@ from audit.related_object_type import RelatedObjectType from environments.models import Environment from experimentation.constants import EXPERIMENT_FLAG -from experimentation.models import Experiment, ExperimentStatus +from experimentation.models import ( + ExpectedDirection, + Experiment, + ExperimentMetric, + ExperimentStatus, + Metric, +) from features.feature_types import MULTIVARIATE from features.models import Feature from tests.types import EnableFeaturesFixture @@ -820,6 +826,210 @@ def test_patch__no_change__skips_audit_log( assert audit_count_after == audit_count_before +def test_post__inline_metric__creates_experiment_metric( + admin_client_new: APIClient, + environment: Environment, + multivariate_feature: Feature, + metric: Metric, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = admin_client_new.post( + _list_url(environment), + data={ + "feature": multivariate_feature.id, + "name": "With metric", + "hypothesis": "It will work", + "metrics": [ + {"metric": metric.id, "expected_direction": "increase"}, + ], + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_201_CREATED + experiment_metric = ExperimentMetric.objects.get( + experiment_id=response.json()["id"] + ) + assert experiment_metric.metric == metric + assert experiment_metric.expected_direction == ExpectedDirection.INCREASE + + +def test_post__metric_from_other_environment__returns_400( + admin_client_new: APIClient, + environment: Environment, + multivariate_feature: Feature, + enable_features: EnableFeaturesFixture, + project: Project, +) -> None: + # Given a metric defined in a sibling environment + enable_features(EXPERIMENT_FLAG) + other_env = Environment.objects.create(name="Other", project=project) + foreign = Metric.objects.create( + environment=other_env, + name="foreign", + definition={"version": 1, "event": "x"}, + ) + + # When + response = admin_client_new.post( + _list_url(environment), + data={ + "feature": multivariate_feature.id, + "name": "Foreign metric", + "hypothesis": "Should fail", + "metrics": [ + {"metric": foreign.id, "expected_direction": "increase"}, + ], + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert not Experiment.objects.filter(name="Foreign metric").exists() + + +def test_post__duplicate_metrics__returns_400( + admin_client_new: APIClient, + environment: Environment, + multivariate_feature: Feature, + metric: Metric, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = admin_client_new.post( + _list_url(environment), + data={ + "feature": multivariate_feature.id, + "name": "Duplicate metrics", + "hypothesis": "Should fail", + "metrics": [ + {"metric": metric.id, "expected_direction": "increase"}, + {"metric": metric.id, "expected_direction": "decrease"}, + ], + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert not Experiment.objects.filter(name="Duplicate metrics").exists() + + +def test_post__invalid_expected_direction__returns_400( + admin_client_new: APIClient, + environment: Environment, + multivariate_feature: Feature, + metric: Metric, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = admin_client_new.post( + _list_url(environment), + data={ + "feature": multivariate_feature.id, + "name": "Bad direction", + "hypothesis": "Should fail", + "metrics": [ + {"metric": metric.id, "expected_direction": "sideways"}, + ], + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + + +def test_patch__metrics__returns_400( + admin_client_new: APIClient, + environment: Environment, + experiment: Experiment, + metric: Metric, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = admin_client_new.patch( + _detail_url(environment, experiment), + data={ + "metrics": [ + {"metric": metric.id, "expected_direction": "increase"}, + ], + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert not experiment.experiment_metrics.exists() + + +def test_get_list__with_attached_metric__returns_metrics( + admin_client_new: APIClient, + environment: Environment, + experiment: Experiment, + metric: Metric, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + ExperimentMetric.objects.create( + experiment=experiment, + metric=metric, + expected_direction=ExpectedDirection.INCREASE, + ) + + # When + response = admin_client_new.get(_list_url(environment)) + + # Then + assert response.status_code == status.HTTP_200_OK + metrics_data = response.json()["results"][0]["metrics"] + assert len(metrics_data) == 1 + assert metrics_data[0]["metric"] == metric.id + assert metrics_data[0]["metric_name"] == metric.name + assert metrics_data[0]["expected_direction"] == "increase" + + +def test_get_detail__with_attached_metric__returns_metrics( + admin_client_new: APIClient, + environment: Environment, + experiment: Experiment, + metric: Metric, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + ExperimentMetric.objects.create( + experiment=experiment, + metric=metric, + expected_direction=ExpectedDirection.INCREASE, + ) + + # When + response = admin_client_new.get(_detail_url(environment, experiment)) + + # Then + assert response.status_code == status.HTTP_200_OK + metrics_data = response.json()["metrics"] + assert len(metrics_data) == 1 + assert metrics_data[0]["metric"] == metric.id + + def test_post__concurrent_create_race__returns_409( admin_client_new: APIClient, environment: Environment, From d9e4669391b6e33f8d3a19b95e18ea0d000fcc61 Mon Sep 17 00:00:00 2001 From: wadii Date: Fri, 12 Jun 2026 17:16:57 +0200 Subject: [PATCH 02/13] feat: add exclude_event_stats param to warehouse connections list --- api/experimentation/views.py | 5 ++-- api/tests/unit/experimentation/test_views.py | 24 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/api/experimentation/views.py b/api/experimentation/views.py index d5fb48aac7ec..039fecdbee0e 100644 --- a/api/experimentation/views.py +++ b/api/experimentation/views.py @@ -86,8 +86,9 @@ def perform_destroy(self, instance: WarehouseConnection) -> None: def list(self, request: Request, *args: object, **kwargs: object) -> Response: environment_api_key: str = self.kwargs["environment_api_key"] connections = list(self.filter_queryset(self.get_queryset())) - for connection in connections: - annotate_warehouse_event_stats(connection, environment_api_key) + if request.query_params.get("exclude_event_stats") not in ("true", "True"): + for connection in connections: + annotate_warehouse_event_stats(connection, environment_api_key) serializer = self.get_serializer(connections, many=True) return Response(serializer.data) diff --git a/api/tests/unit/experimentation/test_views.py b/api/tests/unit/experimentation/test_views.py index 078a9faca9fa..dc6068edb1d5 100644 --- a/api/tests/unit/experimentation/test_views.py +++ b/api/tests/unit/experimentation/test_views.py @@ -1072,6 +1072,30 @@ def test_get__clickhouse_unconfigured__returns_200_without_stats( stats_spy.assert_not_called() +def test_get__exclude_event_stats__returns_200_without_querying_warehouse( + admin_client: APIClient, + environment: Environment, + enable_features: EnableFeaturesFixture, + warehouse_connection: WarehouseConnection, + warehouse_connection_url: str, + mocker: MockerFixture, +) -> None: + # Given + enable_features("experimentation_warehouse_connection") + annotate_spy = mocker.patch("experimentation.views.annotate_warehouse_event_stats") + + # When + response = admin_client.get(f"{warehouse_connection_url}?exclude_event_stats=true") + + # Then + assert response.status_code == status.HTTP_200_OK + data = response.json()[0] + assert data["id"] == warehouse_connection.id + assert data["total_events_received"] is None + assert data["unique_events_count"] is None + annotate_spy.assert_not_called() + + def test_get__clickhouse_errors__returns_200_without_stats( admin_client: APIClient, environment: Environment, From 9d2ed50fff4980d46ea347836608b0069d6ec09a Mon Sep 17 00:00:00 2001 From: wadii Date: Fri, 12 Jun 2026 16:21:41 +0200 Subject: [PATCH 03/13] feat: experiment wizard primary metric selection --- frontend/common/types/requests.ts | 8 +- frontend/common/types/responses.ts | 16 ++ .../base/CenteredModal/CenteredModal.scss | 9 + .../base/CenteredModal/CenteredModal.tsx | 32 ++++ .../components/base/CenteredModal/index.ts | 1 + .../experiments/CreateExperimentWizard.tsx | 49 +++++- .../experiments/CreateMetricInModal.tsx | 51 ++++++ .../MetricSelectList/MetricSelectList.scss | 21 +++ .../MetricSelectList/MetricSelectList.tsx | 161 ++++++++++++++++++ .../experiments/MetricSelectList/index.ts | 1 + .../web/components/experiments/constants.ts | 30 +++- .../experiments/steps/MeasurementStep.tsx | 85 +++++++-- .../experiments/steps/ReviewStep.tsx | 54 +++++- 13 files changed, 492 insertions(+), 26 deletions(-) create mode 100644 frontend/web/components/base/CenteredModal/CenteredModal.scss create mode 100644 frontend/web/components/base/CenteredModal/CenteredModal.tsx create mode 100644 frontend/web/components/base/CenteredModal/index.ts create mode 100644 frontend/web/components/experiments/CreateMetricInModal.tsx create mode 100644 frontend/web/components/experiments/MetricSelectList/MetricSelectList.scss create mode 100644 frontend/web/components/experiments/MetricSelectList/MetricSelectList.tsx create mode 100644 frontend/web/components/experiments/MetricSelectList/index.ts diff --git a/frontend/common/types/requests.ts b/frontend/common/types/requests.ts index 5d31e8488f60..72c8eae6bdb7 100644 --- a/frontend/common/types/requests.ts +++ b/frontend/common/types/requests.ts @@ -24,6 +24,7 @@ import { StageActionType, StageActionBody, ChangeRequest, + ExpectedDirection, ExperimentStatus, MetricAggregation, MetricDirection, @@ -1031,7 +1032,12 @@ export type Req = { }> createExperiment: { environmentId: string - body: { name: string; hypothesis: string; feature: number } + body: { + name: string + hypothesis: string + feature: number + metrics: { metric: number; expected_direction: ExpectedDirection }[] + } } experimentAction: { environmentId: string; experimentId: number } deleteExperiment: { environmentId: string; experimentId: number } diff --git a/frontend/common/types/responses.ts b/frontend/common/types/responses.ts index 73b6c5607024..45da7caadbf4 100644 --- a/frontend/common/types/responses.ts +++ b/frontend/common/types/responses.ts @@ -608,6 +608,21 @@ export type Metric = { updated_at: string } +export type ExpectedDirection = + | 'increase' + | 'decrease' + | 'not_increase' + | 'not_decrease' + +export type ExperimentMetric = { + id: number + metric: number + metric_name: string + aggregation: MetricAggregation + expected_direction: ExpectedDirection + created_at: string +} + export type ExperimentFeature = { id: number name: string @@ -622,6 +637,7 @@ export type Experiment = { hypothesis: string feature: ExperimentFeature status: ExperimentStatus + metrics: ExperimentMetric[] created_at: string updated_at: string started_at: string | null diff --git a/frontend/web/components/base/CenteredModal/CenteredModal.scss b/frontend/web/components/base/CenteredModal/CenteredModal.scss new file mode 100644 index 000000000000..5389401edc7c --- /dev/null +++ b/frontend/web/components/base/CenteredModal/CenteredModal.scss @@ -0,0 +1,9 @@ +.centered-modal { + width: 80vw; + max-width: 80vw; + + .modal-body { + max-height: 75vh; + overflow-y: auto; + } +} diff --git a/frontend/web/components/base/CenteredModal/CenteredModal.tsx b/frontend/web/components/base/CenteredModal/CenteredModal.tsx new file mode 100644 index 000000000000..5d02a41c803a --- /dev/null +++ b/frontend/web/components/base/CenteredModal/CenteredModal.tsx @@ -0,0 +1,32 @@ +import { FC, ReactNode } from 'react' +import { Modal, ModalBody } from 'reactstrap' +import ModalHeader from 'components/modals/base/ModalHeader' +import './CenteredModal.scss' + +type CenteredModalProps = { + isOpen: boolean + title: ReactNode + onClose: () => void + children: ReactNode + className?: string +} + +const CenteredModal: FC = ({ + children, + className, + isOpen, + onClose, + title, +}) => ( + + {title} + {children} + +) + +export default CenteredModal diff --git a/frontend/web/components/base/CenteredModal/index.ts b/frontend/web/components/base/CenteredModal/index.ts new file mode 100644 index 000000000000..26d12e96c80b --- /dev/null +++ b/frontend/web/components/base/CenteredModal/index.ts @@ -0,0 +1 @@ +export { default } from './CenteredModal' diff --git a/frontend/web/components/experiments/CreateExperimentWizard.tsx b/frontend/web/components/experiments/CreateExperimentWizard.tsx index 040be0fc5cd6..d753b8cadab2 100644 --- a/frontend/web/components/experiments/CreateExperimentWizard.tsx +++ b/frontend/web/components/experiments/CreateExperimentWizard.tsx @@ -1,5 +1,5 @@ import { FC, useCallback, useMemo, useState } from 'react' -import { ProjectFlag } from 'common/types/responses' +import { ExpectedDirection, Metric, ProjectFlag } from 'common/types/responses' import { useCreateExperimentMutation } from 'common/services/useExperiment' import WizardStepper from './WizardStepper' import WizardNavButtons from './WizardNavButtons' @@ -10,6 +10,7 @@ import MeasurementStep from './steps/MeasurementStep' import ReviewStep from './steps/ReviewStep' const TOTAL_STEPS = 4 +const MEASUREMENT_STEP = 2 type CreateExperimentWizardProps = { environmentId: string @@ -28,6 +29,9 @@ const CreateExperimentWizard: FC = ({ const [selectedFeature, setSelectedFeature] = useState( null, ) + const [selectedMetric, setSelectedMetric] = useState(null) + const [expectedDirection, setExpectedDirection] = + useState(null) const [completedSteps, setCompletedSteps] = useState>(new Set()) const [createExperiment, { isLoading: isSubmitting }] = @@ -41,7 +45,21 @@ const CreateExperimentWizard: FC = ({ [name, hypothesis, selectedFeature], ) - const canContinue = currentStep === 0 ? isStep1Valid : true + const isMeasurementValid = + selectedMetric !== null && expectedDirection !== null + + const stepValidity: Record = { + 0: isStep1Valid, + [MEASUREMENT_STEP]: isMeasurementValid, + } + const canContinue = stepValidity[currentStep] ?? true + + const handleMetricSelect = useCallback((metric: Metric) => { + setSelectedMetric(metric) + // The metric's own direction and the experiment's expected direction + // are independent concepts — picking a metric never implies a direction. + setExpectedDirection(null) + }, []) const handleContinue = useCallback(() => { if (currentStep < TOTAL_STEPS - 1) { @@ -66,12 +84,18 @@ const CreateExperimentWizard: FC = ({ ) const doCreate = useCallback(async () => { - if (!selectedFeature) return + if (!selectedFeature || !selectedMetric || !expectedDirection) return try { await createExperiment({ body: { feature: selectedFeature.id, hypothesis: hypothesis.trim(), + metrics: [ + { + expected_direction: expectedDirection, + metric: selectedMetric.id, + }, + ], name: name.trim(), }, environmentId, @@ -84,14 +108,16 @@ const CreateExperimentWizard: FC = ({ }, [ createExperiment, environmentId, + expectedDirection, hypothesis, name, onCreated, selectedFeature, + selectedMetric, ]) const handleLaunch = useCallback(() => { - if (!selectedFeature) return + if (!selectedFeature || !isMeasurementValid) return openConfirm({ body: ( @@ -106,7 +132,7 @@ const CreateExperimentWizard: FC = ({ title: 'Create experiment?', yesText: 'Create', }) - }, [selectedFeature, doCreate]) + }, [selectedFeature, isMeasurementValid, doCreate]) const renderStep = () => { switch (currentStep) { @@ -126,14 +152,25 @@ const CreateExperimentWizard: FC = ({ case 1: return case 2: - return + return ( + + ) case 3: return ( setCurrentStep(0)} + onEditMeasurement={() => setCurrentStep(MEASUREMENT_STEP)} /> ) default: diff --git a/frontend/web/components/experiments/CreateMetricInModal.tsx b/frontend/web/components/experiments/CreateMetricInModal.tsx new file mode 100644 index 000000000000..478a9f0b5f18 --- /dev/null +++ b/frontend/web/components/experiments/CreateMetricInModal.tsx @@ -0,0 +1,51 @@ +import { FC } from 'react' +import { Metric } from 'common/types/responses' +import { useCreateMetricMutation } from 'common/services/useMetric' +import Utils from 'common/utils/utils' +import CreateMetricForm from './CreateMetricForm' +import { + buildMetricPayload, + DEFAULT_METRIC_DEFINITION_VERSION, + MetricFormState, +} from './CreateMetricForm/utils' + +type CreateMetricInModalProps = { + environmentId: string + onClose: () => void + onCreated: (metric: Metric) => void +} + +const CreateMetricInModal: FC = ({ + environmentId, + onClose, + onCreated, +}) => { + const [createMetric, { isLoading: isSaving }] = useCreateMetricMutation() + const version = + Number(Utils.getFlagsmithValue('experiment_metrics')) || + DEFAULT_METRIC_DEFINITION_VERSION + + const handleSubmit = async (state: MetricFormState) => { + try { + const metric = await createMetric({ + body: buildMetricPayload(state, version), + environmentId, + }).unwrap() + toast('Metric created successfully') + onClose() + onCreated(metric) + } catch { + toast('Failed to create metric', 'danger') + } + } + + return ( + + ) +} + +export default CreateMetricInModal diff --git a/frontend/web/components/experiments/MetricSelectList/MetricSelectList.scss b/frontend/web/components/experiments/MetricSelectList/MetricSelectList.scss new file mode 100644 index 000000000000..937d5fed2673 --- /dev/null +++ b/frontend/web/components/experiments/MetricSelectList/MetricSelectList.scss @@ -0,0 +1,21 @@ +.metric-select-card { + border: 1px solid var(--color-border-default); + background: none; + width: 100%; + transition: border-color var(--duration-fast) ease, + background-color var(--duration-fast) ease; + + &:hover { + border-color: var(--color-border-action); + } + + &--selected { + border-color: var(--color-border-action); + background-color: var(--color-surface-action-subtle); + } + + &__name { + font-weight: 500; + color: var(--color-text-default); + } +} diff --git a/frontend/web/components/experiments/MetricSelectList/MetricSelectList.tsx b/frontend/web/components/experiments/MetricSelectList/MetricSelectList.tsx new file mode 100644 index 000000000000..767141adb4b7 --- /dev/null +++ b/frontend/web/components/experiments/MetricSelectList/MetricSelectList.tsx @@ -0,0 +1,161 @@ +import { ChangeEvent, FC, useEffect, useState } from 'react' +import { Metric } from 'common/types/responses' +import { useGetMetricsQuery } from 'common/services/useMetric' +import useDebouncedSearch from 'common/useDebouncedSearch' +import Utils from 'common/utils/utils' +import Button from 'components/base/forms/Button' +import Icon from 'components/icons/Icon' +import Paging from 'components/Paging' +import { METRIC_DIRECTION_LABELS } from 'components/experiments/constants' +import './MetricSelectList.scss' + +const PAGE_SIZE = 5 + +type MetricSelectListProps = { + environmentId: string + selectedMetric: Metric | null + onSelect: (metric: Metric) => void + onCreateClick: () => void +} + +const MetricSelectList: FC = ({ + environmentId, + onCreateClick, + onSelect, + selectedMetric, +}) => { + const { search, searchInput, setSearchInput } = useDebouncedSearch() + const [page, setPage] = useState(1) + + useEffect(() => { + setPage(1) + }, [search]) + + const { data: metricsData, isLoading } = useGetMetricsQuery( + { + environmentId, + page, + page_size: PAGE_SIZE, + q: search || undefined, + }, + { skip: !environmentId }, + ) + + const metrics = metricsData?.results + const metricCount = metricsData?.count ?? 0 + const hasActiveSearch = !!search + + if (isLoading) { + return ( +
+ +
+ ) + } + + if (metricCount === 0 && !hasActiveSearch) { + return ( +
+ +
No metrics yet
+

+ Create your first metric to measure experiment outcomes. +

+ +
+ ) + } + + return ( +
+
+
+ ) => + setSearchInput(Utils.safeParseEventValue(e)) + } + placeholder='Search metrics...' + search + size='small' + /> +
+ +
+ + {metrics?.length ? ( + <> +
+ {metrics.map((metric) => { + const isSelected = selectedMetric?.id === metric.id + return ( + + ) + })} +
+ setPage(page + 1)} + prevPage={() => setPage(page - 1)} + goToPage={(p: number) => setPage(p)} + isLoading={isLoading} + /> + + ) : ( +
+

No metrics match your search.

+
+ )} +
+ ) +} + +export default MetricSelectList diff --git a/frontend/web/components/experiments/MetricSelectList/index.ts b/frontend/web/components/experiments/MetricSelectList/index.ts new file mode 100644 index 000000000000..3140eb163e6f --- /dev/null +++ b/frontend/web/components/experiments/MetricSelectList/index.ts @@ -0,0 +1 @@ +export { default } from './MetricSelectList' diff --git a/frontend/web/components/experiments/constants.ts b/frontend/web/components/experiments/constants.ts index 2d9f301c5e44..b4f4f286e0af 100644 --- a/frontend/web/components/experiments/constants.ts +++ b/frontend/web/components/experiments/constants.ts @@ -1,4 +1,8 @@ -import { ExperimentStatus } from 'common/types/responses' +import { + ExpectedDirection, + ExperimentStatus, + MetricDirection, +} from 'common/types/responses' export const EXPERIMENT_STATUS_LABELS: Record = { completed: 'Completed', @@ -21,3 +25,27 @@ export const TAB_ORDER: FilterTab[] = [ 'paused', 'completed', ] + +export const METRIC_DIRECTION_LABELS: Record = { + down: '↓ Lower is better', + informational: 'Informational', + up: '↑ Higher is better', +} + +export type ExpectedDirectionOption = { + value: ExpectedDirection + label: string +} + +export const EXPECTED_DIRECTION_OPTIONS: ExpectedDirectionOption[] = [ + { label: 'Increase', value: 'increase' }, + { label: 'Decrease', value: 'decrease' }, + { label: 'Should not increase', value: 'not_increase' }, + { label: 'Should not decrease', value: 'not_decrease' }, +] + +export const getExpectedDirectionLabel = ( + direction: ExpectedDirection, +): string => + EXPECTED_DIRECTION_OPTIONS.find((option) => option.value === direction) + ?.label ?? direction diff --git a/frontend/web/components/experiments/steps/MeasurementStep.tsx b/frontend/web/components/experiments/steps/MeasurementStep.tsx index 9f11203023ea..01e4b0facc67 100644 --- a/frontend/web/components/experiments/steps/MeasurementStep.tsx +++ b/frontend/web/components/experiments/steps/MeasurementStep.tsx @@ -1,28 +1,79 @@ -import { FC } from 'react' -import Button from 'components/base/forms/Button' -import Icon from 'components/icons/Icon' +import { FC, useState } from 'react' +import { ExpectedDirection, Metric } from 'common/types/responses' import ContentCard from 'components/base/grid/ContentCard' +import CenteredModal from 'components/base/CenteredModal' +import CreateMetricInModal from 'components/experiments/CreateMetricInModal' +import MetricSelectList from 'components/experiments/MetricSelectList' +import { + EXPECTED_DIRECTION_OPTIONS, + ExpectedDirectionOption, +} from 'components/experiments/constants' + +type MeasurementStepProps = { + environmentId: string + selectedMetric: Metric | null + expectedDirection: ExpectedDirection | null + onMetricSelect: (metric: Metric) => void + onExpectedDirectionChange: (direction: ExpectedDirection) => void +} + +const MeasurementStep: FC = ({ + environmentId, + expectedDirection, + onExpectedDirectionChange, + onMetricSelect, + selectedMetric, +}) => { + const [isCreateMetricOpen, setIsCreateMetricOpen] = useState(false) -const MeasurementStep: FC = () => { return (
-
-
- + Select the primary metric this experiment will be judged on. +

+ + setIsCreateMetricOpen(true)} + /> + + {selectedMetric && ( +
+ + +
+
+ + +
+
+ + +
+
+ + ), +} + +export const ScrollableBody: Story = { + render: () => ( + {}}> +
+ {Array.from({ length: 30 }, (_, i) => ( +

+ Long content block {i + 1} — the modal body scrolls once content + exceeds 75% of the viewport height. +

+ ))} +
+ + ), +} + +const InteractiveExample = () => { + const [isOpen, setIsOpen] = useState(false) + return ( + <> + + setIsOpen(false)} + > +

+ Close me via the header button or by clicking the backdrop. +

+ + + ) +} + +export const Interactive: Story = { + parameters: { chromatic: { disableSnapshot: true } }, + render: () => , +} From 49536cdf8a4bb012ab8ae5a72a81307d2b985069 Mon Sep 17 00:00:00 2001 From: wadii Date: Fri, 12 Jun 2026 16:33:00 +0200 Subject: [PATCH 05/13] fix: warehouse enable loading and instant connection display --- frontend/common/services/useWarehouseConnection.ts | 10 ++++++++++ .../environment-settings/tabs/warehouse-tab/index.tsx | 9 +++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/frontend/common/services/useWarehouseConnection.ts b/frontend/common/services/useWarehouseConnection.ts index 017fe642a5a3..84f4473512cc 100644 --- a/frontend/common/services/useWarehouseConnection.ts +++ b/frontend/common/services/useWarehouseConnection.ts @@ -11,6 +11,16 @@ export const warehouseConnectionService = service Req['createWarehouseConnection'] >({ invalidatesTags: [{ id: 'LIST', type: 'WarehouseConnection' }], + async onQueryStarted({ environmentId }, { dispatch, queryFulfilled }) { + const { data } = await queryFulfilled + dispatch( + warehouseConnectionService.util.updateQueryData( + 'getWarehouseConnections', + { environmentId }, + () => [data], + ), + ) + }, query: ({ environmentId, ...body }) => ({ body, method: 'POST', diff --git a/frontend/web/components/pages/environment-settings/tabs/warehouse-tab/index.tsx b/frontend/web/components/pages/environment-settings/tabs/warehouse-tab/index.tsx index 1d8d5f71836b..fd26ed628785 100644 --- a/frontend/web/components/pages/environment-settings/tabs/warehouse-tab/index.tsx +++ b/frontend/web/components/pages/environment-settings/tabs/warehouse-tab/index.tsx @@ -20,6 +20,7 @@ type WarehouseTabProps = { const WarehouseTab: FC = ({ environmentId }) => { const [editing, setEditing] = useState(false) + const [isEnabling, setIsEnabling] = useState(false) const { data: connections, @@ -55,10 +56,14 @@ const WarehouseTab: FC = ({ environmentId }) => { openConfirm({ body: 'This will enable a Flagsmith Warehouse connection for this environment. Are you sure you want to proceed?', onYes: () => { + setIsEnabling(true) createConnection({ environmentId, warehouse_type: 'flagsmith' }) .unwrap() .then(() => toast('Warehouse connection created')) - .catch(() => toast('Failed to create warehouse connection', 'danger')) + .catch(() => { + setIsEnabling(false) + toast('Failed to create warehouse connection', 'danger') + }) }, title: 'Connect Flagsmith Warehouse', }) @@ -142,7 +147,7 @@ const WarehouseTab: FC = ({ environmentId }) => {
) From 949c0adf4d14d394ce51763029aed25a9ccadbd6 Mon Sep 17 00:00:00 2001 From: wadii Date: Fri, 12 Jun 2026 17:36:01 +0200 Subject: [PATCH 06/13] fix: experiment wizard metric cards and review styling per design --- .../SelectableCard}/SelectableCard.scss | 20 ++- .../SelectableCard}/SelectableCard.tsx | 12 ++ .../components/base/SelectableCard/index.ts | 1 + .../base/grid/ContentCard/ContentCard.scss | 12 ++ .../base/grid/ContentCard/ContentCard.tsx | 13 +- .../experiments/CreateExperimentWizard.tsx | 4 +- .../MetricSelectList/MetricSelectList.scss | 32 ++--- .../MetricSelectList/MetricSelectList.tsx | 116 ++++++++---------- .../web/components/experiments/constants.ts | 6 +- .../experiments/steps/AudienceStep.tsx | 11 +- .../experiments/steps/MeasurementStep.tsx | 9 +- .../experiments/steps/ReviewStep.scss | 43 +++++++ .../experiments/steps/ReviewStep.tsx | 41 +++---- .../experiments/steps/SetupStep.tsx | 20 ++- .../tabs/warehouse-tab/WarehouseSetup.tsx | 2 +- .../warehouse-tab/WarehouseSetupSkeleton.tsx | 2 +- 16 files changed, 202 insertions(+), 142 deletions(-) rename frontend/web/components/{pages/environment-settings/tabs/warehouse-tab => base/SelectableCard}/SelectableCard.scss (80%) rename frontend/web/components/{pages/environment-settings/tabs/warehouse-tab => base/SelectableCard}/SelectableCard.tsx (80%) create mode 100644 frontend/web/components/base/SelectableCard/index.ts diff --git a/frontend/web/components/pages/environment-settings/tabs/warehouse-tab/SelectableCard.scss b/frontend/web/components/base/SelectableCard/SelectableCard.scss similarity index 80% rename from frontend/web/components/pages/environment-settings/tabs/warehouse-tab/SelectableCard.scss rename to frontend/web/components/base/SelectableCard/SelectableCard.scss index f390b4ca83ba..cf76f233d77a 100644 --- a/frontend/web/components/pages/environment-settings/tabs/warehouse-tab/SelectableCard.scss +++ b/frontend/web/components/base/SelectableCard/SelectableCard.scss @@ -50,7 +50,7 @@ &__title { font-size: var(--font-body-size); - font-weight: var(--font-weight-semibold); + font-weight: var(--font-weight-medium); color: var(--color-text-default); } @@ -59,6 +59,22 @@ color: var(--color-text-secondary); } + &__tags { + display: flex; + gap: 6px; + flex-wrap: wrap; + margin-top: 4px; + } + + &__tag { + font-size: var(--font-caption-xs-size); + font-weight: var(--font-weight-regular); + padding: 2px 8px; + border-radius: var(--radius-sm); + background: var(--color-surface-emphasis); + color: var(--color-text-secondary); + } + &__aside { display: flex; align-items: center; @@ -68,7 +84,7 @@ &__badge { font-size: var(--font-caption-xs-size); - font-weight: var(--font-weight-semibold); + font-weight: var(--font-weight-medium); padding: 4px 12px; border-radius: var(--radius-full); flex-shrink: 0; diff --git a/frontend/web/components/pages/environment-settings/tabs/warehouse-tab/SelectableCard.tsx b/frontend/web/components/base/SelectableCard/SelectableCard.tsx similarity index 80% rename from frontend/web/components/pages/environment-settings/tabs/warehouse-tab/SelectableCard.tsx rename to frontend/web/components/base/SelectableCard/SelectableCard.tsx index 48d7c6537a35..9ea537b066f3 100644 --- a/frontend/web/components/pages/environment-settings/tabs/warehouse-tab/SelectableCard.tsx +++ b/frontend/web/components/base/SelectableCard/SelectableCard.tsx @@ -10,6 +10,8 @@ type SelectableCardProps = { title: string description: string badge?: { label: string; variant: BadgeVariant } + /** Small info tags rendered under the description (e.g. measurement type). */ + tags?: string[] disabled?: boolean } @@ -20,6 +22,7 @@ const SelectableCard: FC = ({ icon, onClick, selected, + tags, title, }) => { const handleKeyDown = (e: React.KeyboardEvent) => { @@ -43,6 +46,15 @@ const SelectableCard: FC = ({ {icon &&
{icon}
} {title} {description} + {!!tags?.length && ( +
+ {tags.map((tag) => ( + + {tag} + + ))} +
+ )}
{badge && (
diff --git a/frontend/web/components/base/SelectableCard/index.ts b/frontend/web/components/base/SelectableCard/index.ts new file mode 100644 index 000000000000..da1acff49da5 --- /dev/null +++ b/frontend/web/components/base/SelectableCard/index.ts @@ -0,0 +1 @@ +export { default } from './SelectableCard' diff --git a/frontend/web/components/base/grid/ContentCard/ContentCard.scss b/frontend/web/components/base/grid/ContentCard/ContentCard.scss index e6c78a3a0457..c6c6afead62e 100644 --- a/frontend/web/components/base/grid/ContentCard/ContentCard.scss +++ b/frontend/web/components/base/grid/ContentCard/ContentCard.scss @@ -8,11 +8,23 @@ border-radius: var(--radius-lg); &__header { + display: flex; + flex-direction: column; + gap: 4px; + } + + &__heading { display: flex; align-items: center; justify-content: space-between; } + &__description { + font-size: var(--font-caption-size); + color: var(--color-text-secondary); + margin: 0; + } + .input-container { display: block; } diff --git a/frontend/web/components/base/grid/ContentCard/ContentCard.tsx b/frontend/web/components/base/grid/ContentCard/ContentCard.tsx index 2cb5136a3f88..66113cc24d95 100644 --- a/frontend/web/components/base/grid/ContentCard/ContentCard.tsx +++ b/frontend/web/components/base/grid/ContentCard/ContentCard.tsx @@ -4,6 +4,7 @@ import './ContentCard.scss' type ContentCardProps = { title?: string + description?: string action?: ReactNode className?: string children: ReactNode @@ -13,14 +14,20 @@ const ContentCard: FC = ({ action, children, className, + description, title, }) => { return (
- {(title || action) && ( + {(title || action || description) && (
- {title &&

{title}

} - {action} +
+ {title &&

{title}

} + {action} +
+ {description && ( +

{description}

+ )}
)} {children} diff --git a/frontend/web/components/experiments/CreateExperimentWizard.tsx b/frontend/web/components/experiments/CreateExperimentWizard.tsx index d753b8cadab2..d3126ec9ea15 100644 --- a/frontend/web/components/experiments/CreateExperimentWizard.tsx +++ b/frontend/web/components/experiments/CreateExperimentWizard.tsx @@ -11,6 +11,8 @@ import ReviewStep from './steps/ReviewStep' const TOTAL_STEPS = 4 const MEASUREMENT_STEP = 2 +// Hidden until the preview content is wired up to the wizard state +const SHOW_LIVE_PREVIEW = false type CreateExperimentWizardProps = { environmentId: string @@ -197,7 +199,7 @@ const CreateExperimentWizard: FC = ({ onLaunch={handleLaunch} />
- + {SHOW_LIVE_PREVIEW && }
) } diff --git a/frontend/web/components/experiments/MetricSelectList/MetricSelectList.scss b/frontend/web/components/experiments/MetricSelectList/MetricSelectList.scss index 937d5fed2673..35da89037a00 100644 --- a/frontend/web/components/experiments/MetricSelectList/MetricSelectList.scss +++ b/frontend/web/components/experiments/MetricSelectList/MetricSelectList.scss @@ -1,21 +1,25 @@ -.metric-select-card { - border: 1px solid var(--color-border-default); - background: none; - width: 100%; - transition: border-color var(--duration-fast) ease, - background-color var(--duration-fast) ease; +.metric-select-list { + display: flex; + flex-direction: column; + gap: 16px; - &:hover { - border-color: var(--color-border-action); + &__toolbar { + display: flex; + align-items: center; + gap: 12px; } - &--selected { - border-color: var(--color-border-action); - background-color: var(--color-surface-action-subtle); + &__search { + flex: 1; + + .input-container { + width: 100%; + } } - &__name { - font-weight: 500; - color: var(--color-text-default); + &__cards { + display: flex; + flex-direction: column; + gap: 8px; } } diff --git a/frontend/web/components/experiments/MetricSelectList/MetricSelectList.tsx b/frontend/web/components/experiments/MetricSelectList/MetricSelectList.tsx index 767141adb4b7..abebcde51f3d 100644 --- a/frontend/web/components/experiments/MetricSelectList/MetricSelectList.tsx +++ b/frontend/web/components/experiments/MetricSelectList/MetricSelectList.tsx @@ -4,6 +4,8 @@ import { useGetMetricsQuery } from 'common/services/useMetric' import useDebouncedSearch from 'common/useDebouncedSearch' import Utils from 'common/utils/utils' import Button from 'components/base/forms/Button' +import SelectableCard from 'components/base/SelectableCard' +import EmptyState from 'components/EmptyState' import Icon from 'components/icons/Icon' import Paging from 'components/Paging' import { METRIC_DIRECTION_LABELS } from 'components/experiments/constants' @@ -55,28 +57,24 @@ const MetricSelectList: FC = ({ if (metricCount === 0 && !hasActiveSearch) { return ( -
- -
No metrics yet
-

- Create your first metric to measure experiment outcomes. -

- -
+ + + Create Metric + + } + /> ) } return ( -
-
-
+
+
+
) => @@ -95,64 +93,50 @@ const MetricSelectList: FC = ({ {metrics?.length ? ( <> -
+
{metrics.map((metric) => { const isSelected = selectedMetric?.id === metric.id return ( - + /> ) })}
- setPage(page + 1)} - prevPage={() => setPage(page - 1)} - goToPage={(p: number) => setPage(p)} - isLoading={isLoading} - /> + {metricCount > PAGE_SIZE && ( + setPage(page + 1)} + prevPage={() => setPage(page - 1)} + goToPage={(p: number) => setPage(p)} + isLoading={isLoading} + /> + )} ) : ( -
-

No metrics match your search.

-
+ )}
) diff --git a/frontend/web/components/experiments/constants.ts b/frontend/web/components/experiments/constants.ts index b4f4f286e0af..9a10cb32600b 100644 --- a/frontend/web/components/experiments/constants.ts +++ b/frontend/web/components/experiments/constants.ts @@ -27,9 +27,9 @@ export const TAB_ORDER: FilterTab[] = [ ] export const METRIC_DIRECTION_LABELS: Record = { - down: '↓ Lower is better', - informational: 'Informational', - up: '↑ Higher is better', + down: '↓ lower is better', + informational: 'informational', + up: '↑ higher is better', } export type ExpectedDirectionOption = { diff --git a/frontend/web/components/experiments/steps/AudienceStep.tsx b/frontend/web/components/experiments/steps/AudienceStep.tsx index d3687d9ff0fc..e3b76cdc8681 100644 --- a/frontend/web/components/experiments/steps/AudienceStep.tsx +++ b/frontend/web/components/experiments/steps/AudienceStep.tsx @@ -5,13 +5,10 @@ import ContentCard from 'components/base/grid/ContentCard' const AudienceStep: FC = () => { return (
- -

- Define who is eligible for the experiment using attribute conditions. - Conditions are AND-joined. Leave empty to run on all identities in the - environment. Conditions are frozen at launch. Later edits to existing - Segments cannot drift the experiment audience. -

+
= ({ return (
- -

- Select the primary metric this experiment will be judged on. -

- + = ({ Edit @@ -76,34 +73,24 @@ const ReviewStep: FC = ({ } > {selectedMetric ? ( - <> -
- Primary metric - {selectedMetric.name} +
+
+ + {selectedMetric.name} + {!!selectedMetric.description && ( - + {selectedMetric.description} )} -
- - {selectedMetric.definition.event}:{' '} - {selectedMetric.aggregation} - - - {METRIC_DIRECTION_LABELS[selectedMetric.direction]} - -
-
- {expectedDirection && ( -
- Expected direction - + {expectedDirection && ( + {getExpectedDirectionLabel(expectedDirection)} -
- )} - + )} +
+ Primary +
) : (

No metric selected.

)} diff --git a/frontend/web/components/experiments/steps/SetupStep.tsx b/frontend/web/components/experiments/steps/SetupStep.tsx index d69c071d3364..906a75af0f5f 100644 --- a/frontend/web/components/experiments/steps/SetupStep.tsx +++ b/frontend/web/components/experiments/steps/SetupStep.tsx @@ -51,12 +51,10 @@ const SetupStep: FC = ({ return (
- -

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

- + = ({
- -

- The flag you're experimenting on. Variations are read-only, - defined on the flag itself. -

- +