From 443071c60cc371c7942bae208da9dd08479674ba Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Thu, 17 Sep 2026 21:34:21 -0300 Subject: [PATCH 1/2] =?UTF-8?q?adventures=20in=20feature=20versioning=20la?= =?UTF-8?q?nd=20=F0=9F=8D=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- api/features/versioning/managers.py | 25 +-- api/segments/serializers.py | 4 +- api/segments/services.py | 43 ++-- api/segments/views.py | 8 +- .../segments/test_unit_segments_services.py | 191 +++++++++++++++++- 5 files changed, 224 insertions(+), 47 deletions(-) diff --git a/api/features/versioning/managers.py b/api/features/versioning/managers.py index ce73a4086dde..0d486997c425 100644 --- a/api/features/versioning/managers.py +++ b/api/features/versioning/managers.py @@ -1,8 +1,7 @@ import typing -from datetime import datetime from pathlib import Path -from django.db.models import OuterRef +from django.db.models import Exists, OuterRef from django.db.models.query import QuerySet, RawQuerySet from django.utils import timezone from softdelete.models import SoftDeleteManager # type: ignore[import-untyped] @@ -16,21 +15,17 @@ class EnvironmentFeatureVersionManager(SoftDeleteManager): # type: ignore[misc] - def get_versions_live_since( - self, - feature_id: int | OuterRef, - environment_id: int | OuterRef, - live_from: datetime | OuterRef, - ) -> QuerySet["EnvironmentFeatureVersion"]: - """ - Get the published versions of a flag that went live between the provided `live_from` and now. - """ - return self.filter( # type: ignore[no-any-return] - feature_id=feature_id, - environment_id=environment_id, + def get_live_or_scheduled(self) -> QuerySet["EnvironmentFeatureVersion"]: + """Get the published versions that are live now or scheduled to go live.""" + superseding_versions = self.filter( + feature_id=OuterRef("feature_id"), + environment_id=OuterRef("environment_id"), published_at__isnull=False, + live_from__gt=OuterRef("live_from"), live_from__lte=timezone.now(), - live_from__gt=live_from, + ) + return self.filter(published_at__isnull=False).exclude( # type: ignore[no-any-return] + Exists(superseding_versions) ) def get_latest_versions_by_environment_id(self, environment_id: int) -> RawQuerySet: # type: ignore[type-arg] diff --git a/api/segments/serializers.py b/api/segments/serializers.py index 4364967a3b3e..dee190c21376 100644 --- a/api/segments/serializers.py +++ b/api/segments/serializers.py @@ -16,7 +16,7 @@ from segment_membership.models import SegmentMembershipCount from segment_membership.services import enqueue_membership_refresh from segments.models import Condition, Segment, SegmentRule, WhitelistedSegment -from segments.services import get_overrides_in_effect +from segments.services import get_all_live_or_scheduled_overrides from segments.types import ( LegacySegmentRule, ) @@ -191,7 +191,7 @@ def get_has_overrides(self, segment: Segment) -> bool: # is serialized outside that queryset. if (has_overrides := getattr(segment, "has_overrides", None)) is not None: return bool(has_overrides) - return get_overrides_in_effect().filter(segment=segment).exists() + return get_all_live_or_scheduled_overrides().filter(segment=segment).exists() def to_internal_value(self, data: dict[str, Any]) -> Any: self._validate_rules_depth(data.get("rules", [])) diff --git a/api/segments/services.py b/api/segments/services.py index c7518cf46963..18615e652839 100644 --- a/api/segments/services.py +++ b/api/segments/services.py @@ -6,43 +6,34 @@ from django.utils import timezone from core.dataclasses import AuthorData -from features.models import FeatureSegment +from features.models import FeatureSegment, FeatureState from features.versioning.models import EnvironmentFeatureVersion if typing.TYPE_CHECKING: from segments.models import Segment -def get_overrides_in_effect() -> "QuerySet[FeatureSegment]": - """ - Get the feature overrides that are live now or scheduled to go live. - - Returns a global queryset; narrow the result down with additional filters. - """ - # Without v2 versioning, a feature segment is the current state, so it - # counts unless every feature state on it is held by an open change request. - without_v2_versioning = models.Q( - models.Q(environment__use_v2_feature_versioning=False), - models.Q(feature_states__change_request__isnull=True) - | models.Q(feature_states__change_request__committed_at__isnull=False), - ) +def get_all_live_or_scheduled_overrides() -> "QuerySet[FeatureSegment]": + """Get the feature overrides that are live now or scheduled to go live.""" + no_change_request = models.Q(change_request__isnull=True) + committed_change_request = models.Q(change_request__committed_at__isnull=False) + with_feature_versioning_v1 = models.Q( + environment__use_v2_feature_versioning=False, + ) & (no_change_request | committed_change_request) - # With v2 versioning, a published version counts unless another published - # version has gone live since, which covers both the version that is live - # now and any version scheduled to go live later. - with_v2_versioning = models.Q( + with_feature_versioning_v2 = models.Q( environment__use_v2_feature_versioning=True, - environment_feature_version__published_at__isnull=False, - ) & ~models.Exists( - EnvironmentFeatureVersion.objects.get_versions_live_since( - feature_id=models.OuterRef("feature_id"), - environment_id=models.OuterRef("environment_id"), - live_from=models.OuterRef("environment_feature_version__live_from"), - ) + environment_feature_version__in=( + EnvironmentFeatureVersion.objects.get_live_or_scheduled() + ), ) + live_or_scheduled_feature_states = FeatureState.objects.filter( + with_feature_versioning_v1 | with_feature_versioning_v2, + feature_segment_id=models.OuterRef("pk"), + ) return FeatureSegment.objects.filter( # type: ignore[no-any-return] - without_v2_versioning | with_v2_versioning + models.Exists(live_or_scheduled_feature_states) ) diff --git a/api/segments/views.py b/api/segments/views.py index b7d4aeb47279..79a005b68351 100644 --- a/api/segments/views.py +++ b/api/segments/views.py @@ -46,7 +46,7 @@ SegmentMembersResponseSerializer, SegmentSerializer, ) -from .services import delete_segment, get_overrides_in_effect +from .services import delete_segment, get_all_live_or_scheduled_overrides if TYPE_CHECKING: from users.models import FFAdminUser @@ -106,7 +106,9 @@ def get_queryset(self): # type: ignore[no-untyped-def] project=project, is_system_segment=False ).annotate( has_overrides=models.Exists( - get_overrides_in_effect().filter(segment_id=models.OuterRef("pk")) + get_all_live_or_scheduled_overrides().filter( + segment_id=models.OuterRef("pk") + ) ) ) @@ -268,7 +270,7 @@ def _check_segment_is_deletable(self, segment: Segment) -> None: """ if not segment.project.is_workflow_enabled: return - if not get_overrides_in_effect().filter(segment=segment).exists(): + if not get_all_live_or_scheduled_overrides().filter(segment=segment).exists(): return api_error = ChangeRequestsEnabledError( "Cannot delete a segment with feature overrides in a project with " diff --git a/api/tests/unit/segments/test_unit_segments_services.py b/api/tests/unit/segments/test_unit_segments_services.py index c6ff3289e58c..fdde18c7f2e9 100644 --- a/api/tests/unit/segments/test_unit_segments_services.py +++ b/api/tests/unit/segments/test_unit_segments_services.py @@ -1,7 +1,10 @@ +from datetime import timedelta from typing import cast +import pytest from django.db import connection, reset_queries from django.test.utils import CaptureQueriesContext +from django.utils import timezone from flag_engine.segments.constants import EQUAL from api_keys.models import MasterAPIKey @@ -11,10 +14,12 @@ from core.dataclasses import AuthorData from environments.models import Environment from features.models import Feature, FeatureSegment, FeatureState +from features.versioning.models import EnvironmentFeatureVersion +from features.workflows.core.models import ChangeRequest from organisations.models import Organisation from projects.models import Project from segments.models import Condition, Segment, SegmentRule -from segments.services import delete_segment +from segments.services import delete_segment, get_all_live_or_scheduled_overrides from users.models import FFAdminUser @@ -363,3 +368,187 @@ def test_copy_rules_and_conditions_from__varying_segment_sizes__query_count_is_c # Then - query count should be the same (O(depth) not O(n)) assert small_query_count == large_query_count == 10 + + +def test_get_all_live_or_scheduled_overrides__feature_versioning_v1_uncommitted_change_request__returns_no_overrides( + feature_segment: FeatureSegment, + feature: Feature, + environment: Environment, + change_request: ChangeRequest, +) -> None: + # Given + FeatureState.objects.create( + feature_segment=feature_segment, + feature=feature, + environment=environment, + change_request=change_request, + version=None, + ) + + # When + overrides = list(get_all_live_or_scheduled_overrides()) + + # Then + assert overrides == [] + + +@pytest.mark.usefixtures("segment_featurestate") +def test_get_all_live_or_scheduled_overrides__feature_versioning_v1_committed_change_request__returns_distinct_overrides( + feature_segment: FeatureSegment, + feature: Feature, + environment: Environment, + change_request: ChangeRequest, + admin_user: FFAdminUser, +) -> None: + # Given + FeatureState.objects.create( + feature_segment=feature_segment, + feature=feature, + environment=environment, + change_request=change_request, + version=None, + ) + change_request.commit(admin_user) + + # When + overrides = list(get_all_live_or_scheduled_overrides()) + + # Then + assert overrides == [feature_segment] + + +@pytest.mark.usefixtures("segment_featurestate") +def test_get_all_live_or_scheduled_overrides__feature_versioning_v1_scheduled_feature_change__returns_distinct_overrides( + feature_segment: FeatureSegment, + feature: Feature, + environment: Environment, + change_request: ChangeRequest, + admin_user: FFAdminUser, +) -> None: + # Given + FeatureState.objects.create( + feature_segment=feature_segment, + feature=feature, + environment=environment, + change_request=change_request, + live_from=timezone.now() + timedelta(days=1), + version=None, + ) + change_request.commit(admin_user) + + # When + overrides = list(get_all_live_or_scheduled_overrides()) + + # Then + assert overrides == [feature_segment] + + +def test_get_all_live_or_scheduled_overrides__feature_versioning_v2_uncommitted_change_request__returns_no_overrides( + environment_v2_versioning: Environment, + feature: Feature, + segment: Segment, + change_request: ChangeRequest, +) -> None: + # Given + version = EnvironmentFeatureVersion.objects.create( + environment=environment_v2_versioning, + feature=feature, + change_request=change_request, + ) + FeatureState.objects.create( + feature_segment=FeatureSegment.objects.create( + feature=feature, + segment=segment, + environment=environment_v2_versioning, + environment_feature_version=version, + ), + feature=feature, + environment=environment_v2_versioning, + environment_feature_version=version, + ) + + # When + overrides = list(get_all_live_or_scheduled_overrides()) + + # Then + assert overrides == [] + + +def test_get_all_live_or_scheduled_overrides__feature_versioning_v2_committed_change_request__returns_distinct_overrides( + environment_v2_versioning: Environment, + feature: Feature, + segment: Segment, + change_request: ChangeRequest, + admin_user: FFAdminUser, +) -> None: + # Given + live_version = EnvironmentFeatureVersion.objects.get( + environment=environment_v2_versioning, feature=feature + ) + live_version.live_from = timezone.now() - timedelta(days=1) + live_version.save() + FeatureState.objects.create( + feature_segment=FeatureSegment.objects.create( + feature=feature, + segment=segment, + environment=environment_v2_versioning, + environment_feature_version=live_version, + ), + feature=feature, + environment=environment_v2_versioning, + environment_feature_version=live_version, + ) + version = EnvironmentFeatureVersion.objects.create( + environment=environment_v2_versioning, + feature=feature, + change_request=change_request, + ) + committed_override = FeatureSegment.objects.get(environment_feature_version=version) + change_request.commit(admin_user) + + # When + overrides = list(get_all_live_or_scheduled_overrides()) + + # Then + assert overrides == [committed_override] + + +def test_get_all_live_or_scheduled_overrides__feature_versioning_v2_scheduled_feature_change__returns_distinct_overrides( + environment_v2_versioning: Environment, + feature: Feature, + segment: Segment, + change_request: ChangeRequest, + admin_user: FFAdminUser, +) -> None: + # Given + live_version = EnvironmentFeatureVersion.objects.get( + environment=environment_v2_versioning, feature=feature + ) + live_override = FeatureSegment.objects.create( + feature=feature, + segment=segment, + environment=environment_v2_versioning, + environment_feature_version=live_version, + ) + FeatureState.objects.create( + feature_segment=live_override, + feature=feature, + environment=environment_v2_versioning, + environment_feature_version=live_version, + ) + scheduled_version = EnvironmentFeatureVersion.objects.create( + environment=environment_v2_versioning, + feature=feature, + change_request=change_request, + live_from=timezone.now() + timedelta(days=1), + ) + scheduled_override = FeatureSegment.objects.get( + environment_feature_version=scheduled_version + ) + change_request.commit(admin_user) + + # When + overrides = list(get_all_live_or_scheduled_overrides().order_by("id")) + + # Then + assert overrides == [live_override, scheduled_override] From a5a15f4a5bd0db47e044e008901a04659ef95847 Mon Sep 17 00:00:00 2001 From: "flagsmith-engineering[bot]" Date: Fri, 18 Sep 2026 00:53:48 +0000 Subject: [PATCH 2/2] chore: Update documentation artefacts --- .../observability/_events-catalogue.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index 428096849b15..6185878c46b4 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -639,7 +639,7 @@ Attributes: ### `segments.delete_rejected` Logged at `warning` from: - - `api/segments/views.py:277` + - `api/segments/views.py:279` Attributes: - `organisation.id` @@ -659,7 +659,7 @@ Attributes: ### `segments.update_rejected` Logged at `warning` from: - - `api/segments/views.py:253` + - `api/segments/views.py:255` Attributes: - `organisation.id`