From 1d68bc3b22d5e18dca834d78d3e756488b7a7e20 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 26 Aug 2026 10:29:28 +0200 Subject: [PATCH 1/8] feat: auto-connect Flagsmith warehouse for new orgs via experimental_flags value --- api/experimentation/services.py | 46 +++++++++++++++++++++++++++++++-- api/experimentation/views.py | 2 ++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/api/experimentation/services.py b/api/experimentation/services.py index 514246996a84..3b4b70e824b2 100644 --- a/api/experimentation/services.py +++ b/api/experimentation/services.py @@ -13,7 +13,7 @@ from clickhouse_driver.util.helpers import parse_url from django.conf import settings from django.core.cache import cache -from django.db import transaction +from django.db import IntegrityError, transaction from django.db.models import Q from django.utils import timezone from flag_engine.segments.constants import ALL_RULE, PERCENTAGE_SPLIT @@ -58,6 +58,7 @@ ExperimentStatus, MetricAggregation, MetricDirection, + WarehouseConnection, WarehouseConnectionStatus, WarehouseDeliveryLog, WarehouseDeliveryOutcome, @@ -91,7 +92,8 @@ from clickhouse_connect.driver.client import Client as ClickHouseHTTPClient - from experimentation.models import Metric, WarehouseConnection + from environments.models import Environment + from experimentation.models import Metric from experimentation.types import ExposureGranularity from features.feature_states.models import FeatureValueType from features.models import FeatureStateValue @@ -147,6 +149,46 @@ def is_experiment_feature_enabled(organisation: Organisation) -> bool: ) +def get_experiment_flag_config( + organisation: Organisation, +) -> dict[str, object]: + if not is_experiment_feature_enabled(organisation): + return {} + raw = get_openfeature_client().get_string_value( + EXPERIMENT_FLAG, + default_value="{}", + evaluation_context=organisation.openfeature_evaluation_context, + ) + try: + parsed = json.loads(raw) + except (json.JSONDecodeError, TypeError): + return {} + return parsed if isinstance(parsed, dict) else {} + + +def ensure_flagsmith_warehouse_connection( + environment: Environment, +) -> WarehouseConnection | None: + config = get_experiment_flag_config(environment.project.organisation) + if not config.get("auto_connect_warehouse"): + return None + + if WarehouseConnection.objects.filter( + environment=environment, + deleted_at__isnull=True, + ).exists(): + return None + + try: + return WarehouseConnection.objects.create( + environment=environment, + warehouse_type=WarehouseType.FLAGSMITH, + name="Flagsmith", + ) + except IntegrityError: + return None + + @lru_cache(maxsize=2) def _get_clickhouse_client( send_receive_timeout: int = CLICKHOUSE_QUERY_TIMEOUT_SECONDS, diff --git a/api/experimentation/views.py b/api/experimentation/views.py index 5ab97d319c3c..d0ff9afbcc99 100644 --- a/api/experimentation/views.py +++ b/api/experimentation/views.py @@ -67,6 +67,7 @@ create_metric_audit_log, create_warehouse_audit_log, enable_experiment_rollout, + ensure_flagsmith_warehouse_connection, get_warehouse_event_names, mark_warehouse_pending_connection, refresh_warehouse_connection_status, @@ -142,6 +143,7 @@ def perform_destroy(self, instance: WarehouseConnection) -> None: instance.delete() def list(self, request: Request, *args: object, **kwargs: object) -> Response: + ensure_flagsmith_warehouse_connection(self._get_environment()) environment_api_key: str = self.kwargs["environment_api_key"] connections = list(self.filter_queryset(self.get_queryset())) exclude_event_stats = ( From 5020eb63f11d53b84adc51e94b4dd26c83e4ddc6 Mon Sep 17 00:00:00 2001 From: "flagsmith-engineering[bot]" Date: Wed, 26 Aug 2026 08:33:28 +0000 Subject: [PATCH 2/8] chore: Update documentation artefacts --- .../observability/_events-catalogue.md | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index 8c508716727e..23b374eacc71 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -804,7 +804,7 @@ Attributes: ### `warehouse.connection.connected` Logged at `info` from: - - `api/experimentation/services.py:1147` + - `api/experimentation/services.py:1189` Attributes: - `environment.id` @@ -813,8 +813,8 @@ Attributes: ### `warehouse.connection.event_names_failed` Logged at `warning` from: - - `api/experimentation/services.py:224` - - `api/experimentation/services.py:1247` + - `api/experimentation/services.py:266` + - `api/experimentation/services.py:1289` Attributes: - `environment.id` @@ -824,7 +824,7 @@ Attributes: ### `warehouse.connection.event_stats_failed` Logged at `warning` from: - - `api/experimentation/services.py:1210` + - `api/experimentation/services.py:1252` Attributes: - `environment.id` @@ -833,7 +833,7 @@ Attributes: ### `warehouse.connection.test_event_sent` Logged at `info` from: - - `api/experimentation/services.py:921` + - `api/experimentation/services.py:963` Attributes: - `environment.id` @@ -842,7 +842,7 @@ Attributes: ### `warehouse.connection.verification_failed` Logged at `warning` from: - - `api/experimentation/services.py:1122` + - `api/experimentation/services.py:1164` Attributes: - `environment.id` @@ -852,7 +852,7 @@ Attributes: ### `warehouse.connection.verification_succeeded` Logged at `info` from: - - `api/experimentation/services.py:1132` + - `api/experimentation/services.py:1174` Attributes: - `environment.id` @@ -861,7 +861,7 @@ Attributes: ### `warehouse.delivery.all_objects_rejected` Logged at `error` from: - - `api/experimentation/services.py:1077` + - `api/experimentation/services.py:1119` Attributes: - `connection.id` @@ -872,7 +872,7 @@ Attributes: ### `warehouse.delivery.budget_exhausted` Logged at `info` from: - - `api/experimentation/services.py:966` + - `api/experimentation/services.py:1008` Attributes: - `connection.id` @@ -883,7 +883,7 @@ Attributes: ### `warehouse.delivery.completed` Logged at `info` from: - - `api/experimentation/services.py:1087` + - `api/experimentation/services.py:1129` Attributes: - `connection.id` @@ -896,7 +896,7 @@ Attributes: ### `warehouse.delivery.failed` Logged at `error` from: - - `api/experimentation/services.py:1060` + - `api/experimentation/services.py:1102` Attributes: - `connection.id` @@ -907,7 +907,7 @@ Attributes: ### `warehouse.delivery.object_rejected` Logged at `error` from: - - `api/experimentation/services.py:995` + - `api/experimentation/services.py:1037` Attributes: - `connection.id` @@ -919,7 +919,7 @@ Attributes: ### `warehouse.srm.overallocated` Logged at `error` from: - - `api/experimentation/services.py:517` + - `api/experimentation/services.py:559` Attributes: - `environment.id` @@ -929,7 +929,7 @@ Attributes: ### `warehouse.srm.unkeyed_variant` Logged at `error` from: - - `api/experimentation/services.py:503` + - `api/experimentation/services.py:545` Attributes: - `environment.id` From 7e5deede0967ce2133441e29aa848d01805f9946 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 26 Aug 2026 11:25:35 +0200 Subject: [PATCH 3/8] test: add coverage for experiment flag config and auto-connect warehouse --- api/experimentation/services.py | 3 +- .../unit/experimentation/test_services.py | 184 ++++++++++++++++++ api/tests/unit/experimentation/test_views.py | 46 +++++ 3 files changed, 232 insertions(+), 1 deletion(-) diff --git a/api/experimentation/services.py b/api/experimentation/services.py index 3b4b70e824b2..8496f9c3feaa 100644 --- a/api/experimentation/services.py +++ b/api/experimentation/services.py @@ -180,11 +180,12 @@ def ensure_flagsmith_warehouse_connection( return None try: - return WarehouseConnection.objects.create( + connection: WarehouseConnection = WarehouseConnection.objects.create( environment=environment, warehouse_type=WarehouseType.FLAGSMITH, name="Flagsmith", ) + return connection except IntegrityError: return None diff --git a/api/tests/unit/experimentation/test_services.py b/api/tests/unit/experimentation/test_services.py index 9934d65224c3..d25127d3f529 100644 --- a/api/tests/unit/experimentation/test_services.py +++ b/api/tests/unit/experimentation/test_services.py @@ -38,6 +38,8 @@ WarehouseType, ) from experimentation.results_query import _MetricSlot +from django.db import IntegrityError + from experimentation.services import ( annotate_warehouse_event_stats, verify_clickhouse_connection, @@ -2505,3 +2507,185 @@ def test_annotate_warehouse_event_stats__clickhouse_connection__queries_customer # Then get_client.assert_called_once() assert getattr(fresh_connection, "event_stats", None) == expected_stats + + +class TestGetExperimentFlagConfig: + def test__flag_disabled__returns_empty( + self, + organisation: "Organisation", + mocker: MockerFixture, + ) -> None: + # Given + mock_client = MagicMock() + mock_client.get_boolean_value.return_value = False + mocker.patch( + "experimentation.services.get_openfeature_client", + return_value=mock_client, + ) + + # When + result = services.get_experiment_flag_config(organisation) + + # Then + assert result == {} + mock_client.get_string_value.assert_not_called() + + def test__flag_enabled_with_valid_json__returns_parsed( + self, + organisation: "Organisation", + mocker: MockerFixture, + ) -> None: + # Given + mock_client = MagicMock() + mock_client.get_boolean_value.return_value = True + mock_client.get_string_value.return_value = ( + '{"auto_connect_warehouse": true}' + ) + mocker.patch( + "experimentation.services.get_openfeature_client", + return_value=mock_client, + ) + + # When + result = services.get_experiment_flag_config(organisation) + + # Then + assert result == {"auto_connect_warehouse": True} + + @pytest.mark.parametrize( + "raw_value", + ["not-json", "", None], + ids=["invalid-json", "empty-string", "none"], + ) + def test__flag_enabled_with_bad_value__returns_empty( + self, + organisation: "Organisation", + mocker: MockerFixture, + raw_value: str | None, + ) -> None: + # Given + mock_client = MagicMock() + mock_client.get_boolean_value.return_value = True + mock_client.get_string_value.return_value = raw_value + mocker.patch( + "experimentation.services.get_openfeature_client", + return_value=mock_client, + ) + + # When + result = services.get_experiment_flag_config(organisation) + + # Then + assert result == {} + + def test__flag_enabled_with_non_dict_json__returns_empty( + self, + organisation: "Organisation", + mocker: MockerFixture, + ) -> None: + # Given + mock_client = MagicMock() + mock_client.get_boolean_value.return_value = True + mock_client.get_string_value.return_value = '["free"]' + mocker.patch( + "experimentation.services.get_openfeature_client", + return_value=mock_client, + ) + + # When + result = services.get_experiment_flag_config(organisation) + + # Then + assert result == {} + + +@pytest.mark.django_db() +class TestEnsureFlagsmithWarehouseConnection: + def test__auto_connect_disabled__returns_none( + self, + environment: Environment, + mocker: MockerFixture, + ) -> None: + # Given + mocker.patch( + "experimentation.services.get_experiment_flag_config", + return_value={}, + ) + + # When + result = services.ensure_flagsmith_warehouse_connection(environment) + + # Then + assert result is None + assert not WarehouseConnection.objects.filter( + environment=environment, + ).exists() + + def test__auto_connect_enabled__creates_flagsmith_connection( + self, + environment: Environment, + mocker: MockerFixture, + ) -> None: + # Given + mocker.patch( + "experimentation.services.get_experiment_flag_config", + return_value={"auto_connect_warehouse": True}, + ) + + # When + result = services.ensure_flagsmith_warehouse_connection(environment) + + # Then + assert result is not None + assert result.warehouse_type == WarehouseType.FLAGSMITH + assert result.name == "Flagsmith" + assert result.environment == environment + + def test__connection_already_exists__returns_none( + self, + environment: Environment, + warehouse_connection: WarehouseConnection, + mocker: MockerFixture, + ) -> None: + # Given + mocker.patch( + "experimentation.services.get_experiment_flag_config", + return_value={"auto_connect_warehouse": True}, + ) + + # When + result = services.ensure_flagsmith_warehouse_connection(environment) + + # Then + assert result is None + assert WarehouseConnection.objects.filter( + environment=environment, + deleted_at__isnull=True, + ).count() == 1 + + def test__race_condition__handles_integrity_error( + self, + environment: Environment, + mocker: MockerFixture, + ) -> None: + # Given + mocker.patch( + "experimentation.services.get_experiment_flag_config", + return_value={"auto_connect_warehouse": True}, + ) + mocker.patch.object( + WarehouseConnection.objects, + "filter", + return_value=MagicMock(exists=MagicMock(return_value=False)), + ) + mocker.patch.object( + WarehouseConnection.objects, + "create", + side_effect=IntegrityError("duplicate"), + ) + + # When + result = services.ensure_flagsmith_warehouse_connection(environment) + + # Then + assert result is None diff --git a/api/tests/unit/experimentation/test_views.py b/api/tests/unit/experimentation/test_views.py index 543c3b26969e..408cf3000a30 100644 --- a/api/tests/unit/experimentation/test_views.py +++ b/api/tests/unit/experimentation/test_views.py @@ -242,6 +242,52 @@ def test_get__not_exists__returns_200_with_empty_list( assert response.json() == [] +def test_get__auto_connect_enabled__creates_flagsmith_connection( + admin_client: APIClient, + environment: Environment, + enable_features: EnableFeaturesFixture, + warehouse_connection_url: str, + mocker: MockerFixture, +) -> None: + # Given + enable_features("experimentation_warehouse_connection") + mocker.patch( + "experimentation.services.get_experiment_flag_config", + return_value={"auto_connect_warehouse": True}, + ) + + # When + response = admin_client.get(warehouse_connection_url) + + # Then + assert response.status_code == status.HTTP_200_OK + data = response.json() + assert len(data) == 1 + assert data[0]["warehouse_type"] == "flagsmith" + assert data[0]["name"] == "Flagsmith" + + +def test_get__auto_connect_disabled__no_connection_created( + admin_client: APIClient, + enable_features: EnableFeaturesFixture, + warehouse_connection_url: str, + mocker: MockerFixture, +) -> None: + # Given + enable_features("experimentation_warehouse_connection") + mocker.patch( + "experimentation.services.get_experiment_flag_config", + return_value={}, + ) + + # When + response = admin_client.get(warehouse_connection_url) + + # Then + assert response.status_code == status.HTTP_200_OK + assert response.json() == [] + + def test_delete__exists__returns_204( admin_client: APIClient, environment: Environment, From 55a63e1664a7fb7c91f689931db8a36b0e09a8f1 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2026 09:26:11 +0000 Subject: [PATCH 4/8] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- .../unit/experimentation/test_services.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/api/tests/unit/experimentation/test_services.py b/api/tests/unit/experimentation/test_services.py index d25127d3f529..e04becb2b19f 100644 --- a/api/tests/unit/experimentation/test_services.py +++ b/api/tests/unit/experimentation/test_services.py @@ -3,6 +3,7 @@ from unittest.mock import MagicMock import pytest +from django.db import IntegrityError from django.db.models import Q from flag_engine.segments.constants import PERCENTAGE_SPLIT from prometheus_client import REGISTRY @@ -38,8 +39,6 @@ WarehouseType, ) from experimentation.results_query import _MetricSlot -from django.db import IntegrityError - from experimentation.services import ( annotate_warehouse_event_stats, verify_clickhouse_connection, @@ -2538,9 +2537,7 @@ def test__flag_enabled_with_valid_json__returns_parsed( # Given mock_client = MagicMock() mock_client.get_boolean_value.return_value = True - mock_client.get_string_value.return_value = ( - '{"auto_connect_warehouse": true}' - ) + mock_client.get_string_value.return_value = '{"auto_connect_warehouse": true}' mocker.patch( "experimentation.services.get_openfeature_client", return_value=mock_client, @@ -2658,10 +2655,13 @@ def test__connection_already_exists__returns_none( # Then assert result is None - assert WarehouseConnection.objects.filter( - environment=environment, - deleted_at__isnull=True, - ).count() == 1 + assert ( + WarehouseConnection.objects.filter( + environment=environment, + deleted_at__isnull=True, + ).count() + == 1 + ) def test__race_condition__handles_integrity_error( self, From 5eb0cea5ce91f75b141335614ea29e99a1b7c3fb Mon Sep 17 00:00:00 2001 From: "flagsmith-engineering[bot]" Date: Wed, 26 Aug 2026 09:33:00 +0000 Subject: [PATCH 5/8] chore: Update documentation artefacts --- .../observability/_events-catalogue.md | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index 23b374eacc71..fe00b0203b8f 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -804,7 +804,7 @@ Attributes: ### `warehouse.connection.connected` Logged at `info` from: - - `api/experimentation/services.py:1189` + - `api/experimentation/services.py:1190` Attributes: - `environment.id` @@ -813,8 +813,8 @@ Attributes: ### `warehouse.connection.event_names_failed` Logged at `warning` from: - - `api/experimentation/services.py:266` - - `api/experimentation/services.py:1289` + - `api/experimentation/services.py:267` + - `api/experimentation/services.py:1290` Attributes: - `environment.id` @@ -824,7 +824,7 @@ Attributes: ### `warehouse.connection.event_stats_failed` Logged at `warning` from: - - `api/experimentation/services.py:1252` + - `api/experimentation/services.py:1253` Attributes: - `environment.id` @@ -833,7 +833,7 @@ Attributes: ### `warehouse.connection.test_event_sent` Logged at `info` from: - - `api/experimentation/services.py:963` + - `api/experimentation/services.py:964` Attributes: - `environment.id` @@ -842,7 +842,7 @@ Attributes: ### `warehouse.connection.verification_failed` Logged at `warning` from: - - `api/experimentation/services.py:1164` + - `api/experimentation/services.py:1165` Attributes: - `environment.id` @@ -852,7 +852,7 @@ Attributes: ### `warehouse.connection.verification_succeeded` Logged at `info` from: - - `api/experimentation/services.py:1174` + - `api/experimentation/services.py:1175` Attributes: - `environment.id` @@ -861,7 +861,7 @@ Attributes: ### `warehouse.delivery.all_objects_rejected` Logged at `error` from: - - `api/experimentation/services.py:1119` + - `api/experimentation/services.py:1120` Attributes: - `connection.id` @@ -872,7 +872,7 @@ Attributes: ### `warehouse.delivery.budget_exhausted` Logged at `info` from: - - `api/experimentation/services.py:1008` + - `api/experimentation/services.py:1009` Attributes: - `connection.id` @@ -883,7 +883,7 @@ Attributes: ### `warehouse.delivery.completed` Logged at `info` from: - - `api/experimentation/services.py:1129` + - `api/experimentation/services.py:1130` Attributes: - `connection.id` @@ -896,7 +896,7 @@ Attributes: ### `warehouse.delivery.failed` Logged at `error` from: - - `api/experimentation/services.py:1102` + - `api/experimentation/services.py:1103` Attributes: - `connection.id` @@ -907,7 +907,7 @@ Attributes: ### `warehouse.delivery.object_rejected` Logged at `error` from: - - `api/experimentation/services.py:1037` + - `api/experimentation/services.py:1038` Attributes: - `connection.id` @@ -919,7 +919,7 @@ Attributes: ### `warehouse.srm.overallocated` Logged at `error` from: - - `api/experimentation/services.py:559` + - `api/experimentation/services.py:560` Attributes: - `environment.id` @@ -929,7 +929,7 @@ Attributes: ### `warehouse.srm.unkeyed_variant` Logged at `error` from: - - `api/experimentation/services.py:545` + - `api/experimentation/services.py:546` Attributes: - `environment.id` From eae03e50a3bf4dd51bf04094cbab094aa32d766d Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 26 Aug 2026 11:45:38 +0200 Subject: [PATCH 6/8] fix: resolve mypy and lint errors in experimentation tests --- .../unit/experimentation/test_services.py | 331 +++++++++--------- 1 file changed, 165 insertions(+), 166 deletions(-) diff --git a/api/tests/unit/experimentation/test_services.py b/api/tests/unit/experimentation/test_services.py index e04becb2b19f..e318751dd05e 100644 --- a/api/tests/unit/experimentation/test_services.py +++ b/api/tests/unit/experimentation/test_services.py @@ -49,6 +49,7 @@ from features.multivariate.models import MultivariateFeatureOption from features.value_types import STRING from features.versioning.dataclasses import MultivariateValueChangeSet +from organisations.models import Organisation from segments.models import Condition, Segment, SegmentRule from users.models import FFAdminUser from util.mappers import map_environment_to_environment_document @@ -2508,184 +2509,182 @@ def test_annotate_warehouse_event_stats__clickhouse_connection__queries_customer assert getattr(fresh_connection, "event_stats", None) == expected_stats -class TestGetExperimentFlagConfig: - def test__flag_disabled__returns_empty( - self, - organisation: "Organisation", - mocker: MockerFixture, - ) -> None: - # Given - mock_client = MagicMock() - mock_client.get_boolean_value.return_value = False - mocker.patch( - "experimentation.services.get_openfeature_client", - return_value=mock_client, - ) +def test_get_experiment_flag_config__flag_disabled__returns_empty( + organisation: Organisation, + mocker: MockerFixture, +) -> None: + # Given + mock_client = MagicMock() + mock_client.get_boolean_value.return_value = False + mocker.patch( + "experimentation.services.get_openfeature_client", + return_value=mock_client, + ) - # When - result = services.get_experiment_flag_config(organisation) - - # Then - assert result == {} - mock_client.get_string_value.assert_not_called() - - def test__flag_enabled_with_valid_json__returns_parsed( - self, - organisation: "Organisation", - mocker: MockerFixture, - ) -> None: - # Given - mock_client = MagicMock() - mock_client.get_boolean_value.return_value = True - mock_client.get_string_value.return_value = '{"auto_connect_warehouse": true}' - mocker.patch( - "experimentation.services.get_openfeature_client", - return_value=mock_client, - ) + # When + result = services.get_experiment_flag_config(organisation) - # When - result = services.get_experiment_flag_config(organisation) - - # Then - assert result == {"auto_connect_warehouse": True} - - @pytest.mark.parametrize( - "raw_value", - ["not-json", "", None], - ids=["invalid-json", "empty-string", "none"], - ) - def test__flag_enabled_with_bad_value__returns_empty( - self, - organisation: "Organisation", - mocker: MockerFixture, - raw_value: str | None, - ) -> None: - # Given - mock_client = MagicMock() - mock_client.get_boolean_value.return_value = True - mock_client.get_string_value.return_value = raw_value - mocker.patch( - "experimentation.services.get_openfeature_client", - return_value=mock_client, - ) + # Then + assert result == {} + mock_client.get_string_value.assert_not_called() - # When - result = services.get_experiment_flag_config(organisation) - - # Then - assert result == {} - - def test__flag_enabled_with_non_dict_json__returns_empty( - self, - organisation: "Organisation", - mocker: MockerFixture, - ) -> None: - # Given - mock_client = MagicMock() - mock_client.get_boolean_value.return_value = True - mock_client.get_string_value.return_value = '["free"]' - mocker.patch( - "experimentation.services.get_openfeature_client", - return_value=mock_client, - ) - # When - result = services.get_experiment_flag_config(organisation) +def test_get_experiment_flag_config__flag_enabled_with_valid_json__returns_parsed( + organisation: Organisation, + mocker: MockerFixture, +) -> None: + # Given + mock_client = MagicMock() + mock_client.get_boolean_value.return_value = True + mock_client.get_string_value.return_value = '{"auto_connect_warehouse": true}' + mocker.patch( + "experimentation.services.get_openfeature_client", + return_value=mock_client, + ) - # Then - assert result == {} + # When + result = services.get_experiment_flag_config(organisation) + + # Then + assert result == {"auto_connect_warehouse": True} + + +@pytest.mark.parametrize( + "raw_value", + ["not-json", "", None], + ids=["invalid-json", "empty-string", "none"], +) +def test_get_experiment_flag_config__flag_enabled_with_bad_value__returns_empty( + organisation: Organisation, + mocker: MockerFixture, + raw_value: str | None, +) -> None: + # Given + mock_client = MagicMock() + mock_client.get_boolean_value.return_value = True + mock_client.get_string_value.return_value = raw_value + mocker.patch( + "experimentation.services.get_openfeature_client", + return_value=mock_client, + ) + + # When + result = services.get_experiment_flag_config(organisation) + + # Then + assert result == {} + + +def test_get_experiment_flag_config__flag_enabled_with_non_dict_json__returns_empty( + organisation: Organisation, + mocker: MockerFixture, +) -> None: + # Given + mock_client = MagicMock() + mock_client.get_boolean_value.return_value = True + mock_client.get_string_value.return_value = '["free"]' + mocker.patch( + "experimentation.services.get_openfeature_client", + return_value=mock_client, + ) + + # When + result = services.get_experiment_flag_config(organisation) + + # Then + assert result == {} @pytest.mark.django_db() -class TestEnsureFlagsmithWarehouseConnection: - def test__auto_connect_disabled__returns_none( - self, - environment: Environment, - mocker: MockerFixture, - ) -> None: - # Given - mocker.patch( - "experimentation.services.get_experiment_flag_config", - return_value={}, - ) +def test_ensure_flagsmith_warehouse_connection__auto_connect_disabled__returns_none( + environment: Environment, + mocker: MockerFixture, +) -> None: + # Given + mocker.patch( + "experimentation.services.get_experiment_flag_config", + return_value={}, + ) - # When - result = services.ensure_flagsmith_warehouse_connection(environment) + # When + result = services.ensure_flagsmith_warehouse_connection(environment) - # Then - assert result is None - assert not WarehouseConnection.objects.filter( - environment=environment, - ).exists() - - def test__auto_connect_enabled__creates_flagsmith_connection( - self, - environment: Environment, - mocker: MockerFixture, - ) -> None: - # Given - mocker.patch( - "experimentation.services.get_experiment_flag_config", - return_value={"auto_connect_warehouse": True}, - ) + # Then + assert result is None + assert not WarehouseConnection.objects.filter( + environment=environment, + ).exists() - # When - result = services.ensure_flagsmith_warehouse_connection(environment) - - # Then - assert result is not None - assert result.warehouse_type == WarehouseType.FLAGSMITH - assert result.name == "Flagsmith" - assert result.environment == environment - - def test__connection_already_exists__returns_none( - self, - environment: Environment, - warehouse_connection: WarehouseConnection, - mocker: MockerFixture, - ) -> None: - # Given - mocker.patch( - "experimentation.services.get_experiment_flag_config", - return_value={"auto_connect_warehouse": True}, - ) - # When - result = services.ensure_flagsmith_warehouse_connection(environment) - - # Then - assert result is None - assert ( - WarehouseConnection.objects.filter( - environment=environment, - deleted_at__isnull=True, - ).count() - == 1 - ) +@pytest.mark.django_db() +def test_ensure_flagsmith_warehouse_connection__auto_connect_enabled__creates_connection( + environment: Environment, + mocker: MockerFixture, +) -> None: + # Given + mocker.patch( + "experimentation.services.get_experiment_flag_config", + return_value={"auto_connect_warehouse": True}, + ) - def test__race_condition__handles_integrity_error( - self, - environment: Environment, - mocker: MockerFixture, - ) -> None: - # Given - mocker.patch( - "experimentation.services.get_experiment_flag_config", - return_value={"auto_connect_warehouse": True}, - ) - mocker.patch.object( - WarehouseConnection.objects, - "filter", - return_value=MagicMock(exists=MagicMock(return_value=False)), - ) - mocker.patch.object( - WarehouseConnection.objects, - "create", - side_effect=IntegrityError("duplicate"), - ) + # When + result = services.ensure_flagsmith_warehouse_connection(environment) - # When - result = services.ensure_flagsmith_warehouse_connection(environment) + # Then + assert result is not None + assert result.warehouse_type == WarehouseType.FLAGSMITH + assert result.name == "Flagsmith" + assert result.environment == environment - # Then - assert result is None + +@pytest.mark.django_db() +def test_ensure_flagsmith_warehouse_connection__connection_already_exists__returns_none( + environment: Environment, + warehouse_connection: WarehouseConnection, + mocker: MockerFixture, +) -> None: + # Given + mocker.patch( + "experimentation.services.get_experiment_flag_config", + return_value={"auto_connect_warehouse": True}, + ) + + # When + result = services.ensure_flagsmith_warehouse_connection(environment) + + # Then + assert result is None + assert ( + WarehouseConnection.objects.filter( + environment=environment, + deleted_at__isnull=True, + ).count() + == 1 + ) + + +def test_ensure_flagsmith_warehouse_connection__race_condition__handles_integrity_error( + environment: Environment, + mocker: MockerFixture, +) -> None: + # Given + mocker.patch( + "experimentation.services.get_experiment_flag_config", + return_value={"auto_connect_warehouse": True}, + ) + mocker.patch.object( + WarehouseConnection.objects, + "filter", + return_value=MagicMock(exists=MagicMock(return_value=False)), + ) + mocker.patch.object( + WarehouseConnection.objects, + "create", + side_effect=IntegrityError("duplicate"), + ) + + # When + result = services.ensure_flagsmith_warehouse_connection(environment) + + # Then + assert result is None From 1037f7c6c218ca488cfc707b154b6c3afa4d1dd5 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 26 Aug 2026 14:30:52 +0200 Subject: [PATCH 7/8] refactor: move warehouse auto-connect to environment creation signal --- api/experimentation/apps.py | 3 + api/experimentation/signals.py | 19 +++++++ api/experimentation/views.py | 2 - .../unit/experimentation/test_signals.py | 57 ++++++++++++++++++- api/tests/unit/experimentation/test_views.py | 46 --------------- 5 files changed, 78 insertions(+), 49 deletions(-) create mode 100644 api/experimentation/signals.py diff --git a/api/experimentation/apps.py b/api/experimentation/apps.py index 27787df60280..3659299c49a1 100644 --- a/api/experimentation/apps.py +++ b/api/experimentation/apps.py @@ -3,3 +3,6 @@ class ExperimentationConfig(AppConfig): name = "experimentation" + + def ready(self) -> None: + from experimentation import signals # noqa: F401 diff --git a/api/experimentation/signals.py b/api/experimentation/signals.py new file mode 100644 index 000000000000..f777cfe133f0 --- /dev/null +++ b/api/experimentation/signals.py @@ -0,0 +1,19 @@ +from __future__ import annotations + +from django.db.models.signals import post_save +from django.dispatch import receiver + +from environments.models import Environment +from experimentation.services import ensure_flagsmith_warehouse_connection + + +@receiver(post_save, sender=Environment) +def auto_connect_warehouse_on_environment_create( + sender: type[Environment], + instance: Environment, + created: bool, + **kwargs: object, +) -> None: + if not created: + return + ensure_flagsmith_warehouse_connection(instance) diff --git a/api/experimentation/views.py b/api/experimentation/views.py index d0ff9afbcc99..5ab97d319c3c 100644 --- a/api/experimentation/views.py +++ b/api/experimentation/views.py @@ -67,7 +67,6 @@ create_metric_audit_log, create_warehouse_audit_log, enable_experiment_rollout, - ensure_flagsmith_warehouse_connection, get_warehouse_event_names, mark_warehouse_pending_connection, refresh_warehouse_connection_status, @@ -143,7 +142,6 @@ def perform_destroy(self, instance: WarehouseConnection) -> None: instance.delete() def list(self, request: Request, *args: object, **kwargs: object) -> Response: - ensure_flagsmith_warehouse_connection(self._get_environment()) environment_api_key: str = self.kwargs["environment_api_key"] connections = list(self.filter_queryset(self.get_queryset())) exclude_event_stats = ( diff --git a/api/tests/unit/experimentation/test_signals.py b/api/tests/unit/experimentation/test_signals.py index b152952ea0fe..9c79f428108e 100644 --- a/api/tests/unit/experimentation/test_signals.py +++ b/api/tests/unit/experimentation/test_signals.py @@ -1,7 +1,10 @@ +import pytest from pytest_mock import MockerFixture from environments.models import Environment, EnvironmentAPIKey -from experimentation.models import WarehouseConnection +from experimentation.models import WarehouseConnection, WarehouseType +from organisations.models import Organisation +from projects.models import Project def test_environment_api_key__created_with_warehouse__enqueues_write( @@ -94,3 +97,55 @@ def test_environment_api_key__deleted_without_warehouse__does_not_enqueue( # Then mock_task.delay.assert_not_called() + + +@pytest.mark.django_db() +def test_environment_create__auto_connect_enabled__creates_warehouse_connection( + organisation: Organisation, + mocker: MockerFixture, +) -> None: + # Given + mocker.patch( + "experimentation.services.get_experiment_flag_config", + return_value={"auto_connect_warehouse": True}, + ) + project: Project = Project.objects.create( + name="Test Project", + organisation=organisation, + ) + + # When + environment: Environment = Environment.objects.create( + name="Test Environment", + project=project, + ) + + # Then + connection = WarehouseConnection.objects.get(environment=environment) + assert connection.warehouse_type == WarehouseType.FLAGSMITH + assert connection.name == "Flagsmith" + + +@pytest.mark.django_db() +def test_environment_create__auto_connect_disabled__no_warehouse_connection( + organisation: Organisation, + mocker: MockerFixture, +) -> None: + # Given + mocker.patch( + "experimentation.services.get_experiment_flag_config", + return_value={}, + ) + project: Project = Project.objects.create( + name="Test Project", + organisation=organisation, + ) + + # When + environment: Environment = Environment.objects.create( + name="Test Environment", + project=project, + ) + + # Then + assert not WarehouseConnection.objects.filter(environment=environment).exists() diff --git a/api/tests/unit/experimentation/test_views.py b/api/tests/unit/experimentation/test_views.py index 408cf3000a30..543c3b26969e 100644 --- a/api/tests/unit/experimentation/test_views.py +++ b/api/tests/unit/experimentation/test_views.py @@ -242,52 +242,6 @@ def test_get__not_exists__returns_200_with_empty_list( assert response.json() == [] -def test_get__auto_connect_enabled__creates_flagsmith_connection( - admin_client: APIClient, - environment: Environment, - enable_features: EnableFeaturesFixture, - warehouse_connection_url: str, - mocker: MockerFixture, -) -> None: - # Given - enable_features("experimentation_warehouse_connection") - mocker.patch( - "experimentation.services.get_experiment_flag_config", - return_value={"auto_connect_warehouse": True}, - ) - - # When - response = admin_client.get(warehouse_connection_url) - - # Then - assert response.status_code == status.HTTP_200_OK - data = response.json() - assert len(data) == 1 - assert data[0]["warehouse_type"] == "flagsmith" - assert data[0]["name"] == "Flagsmith" - - -def test_get__auto_connect_disabled__no_connection_created( - admin_client: APIClient, - enable_features: EnableFeaturesFixture, - warehouse_connection_url: str, - mocker: MockerFixture, -) -> None: - # Given - enable_features("experimentation_warehouse_connection") - mocker.patch( - "experimentation.services.get_experiment_flag_config", - return_value={}, - ) - - # When - response = admin_client.get(warehouse_connection_url) - - # Then - assert response.status_code == status.HTTP_200_OK - assert response.json() == [] - - def test_delete__exists__returns_204( admin_client: APIClient, environment: Environment, From a9754b5d617b893f2a313f7a02497a0816a8712e Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 26 Aug 2026 14:46:00 +0200 Subject: [PATCH 8/8] refactor: use @hook(AFTER_CREATE) on Environment instead of post_save signal --- api/environments/models.py | 6 ++++++ api/experimentation/apps.py | 3 --- api/experimentation/signals.py | 19 ------------------- 3 files changed, 6 insertions(+), 22 deletions(-) delete mode 100644 api/experimentation/signals.py diff --git a/api/environments/models.py b/api/environments/models.py index 80f79ef08889..feb5e89fc971 100644 --- a/api/environments/models.py +++ b/api/environments/models.py @@ -184,6 +184,12 @@ class Meta: def create_feature_states(self) -> None: FeatureState.create_initial_feature_states_for_environment(environment=self) + @hook(AFTER_CREATE) # type: ignore[misc] + def auto_connect_warehouse(self) -> None: + from experimentation.services import ensure_flagsmith_warehouse_connection + + ensure_flagsmith_warehouse_connection(self) + @hook(AFTER_UPDATE) # type: ignore[misc] def clear_environment_cache(self) -> None: # TODO: this could rebuild the cache itself (using an async task) diff --git a/api/experimentation/apps.py b/api/experimentation/apps.py index 3659299c49a1..27787df60280 100644 --- a/api/experimentation/apps.py +++ b/api/experimentation/apps.py @@ -3,6 +3,3 @@ class ExperimentationConfig(AppConfig): name = "experimentation" - - def ready(self) -> None: - from experimentation import signals # noqa: F401 diff --git a/api/experimentation/signals.py b/api/experimentation/signals.py deleted file mode 100644 index f777cfe133f0..000000000000 --- a/api/experimentation/signals.py +++ /dev/null @@ -1,19 +0,0 @@ -from __future__ import annotations - -from django.db.models.signals import post_save -from django.dispatch import receiver - -from environments.models import Environment -from experimentation.services import ensure_flagsmith_warehouse_connection - - -@receiver(post_save, sender=Environment) -def auto_connect_warehouse_on_environment_create( - sender: type[Environment], - instance: Environment, - created: bool, - **kwargs: object, -) -> None: - if not created: - return - ensure_flagsmith_warehouse_connection(instance)