diff --git a/api/environments/urls.py b/api/environments/urls.py index 2b2019516561..4921177c7e1f 100644 --- a/api/environments/urls.py +++ b/api/environments/urls.py @@ -8,7 +8,11 @@ EdgeIdentityWithIdentifierFeatureStateView, get_edge_identity_overrides, ) -from features.dependencies.views import FeatureDependencyAPIView +from features.dependencies.views import ( + FeatureDependenciesAPIView, + FeatureDependencyAPIView, + FeatureDependentsAPIView, +) from features.views import ( EnvironmentFeatureStateViewSet, IdentityFeatureStateViewSet, @@ -170,6 +174,16 @@ FeatureDependencyAPIView.as_view(), name="feature-dependency", ), + path( + "/features//dependencies/", + FeatureDependenciesAPIView.as_view(), + name="feature-dependencies", + ), + path( + "/features//dependents/", + FeatureDependentsAPIView.as_view(), + name="feature-dependents", + ), path( "/edge-identity-overrides", get_edge_identity_overrides, diff --git a/api/features/dependencies/exceptions.py b/api/features/dependencies/exceptions.py index 42fd5aed65a2..b645f08735f7 100644 --- a/api/features/dependencies/exceptions.py +++ b/api/features/dependencies/exceptions.py @@ -96,6 +96,20 @@ def get_message(self, path: DependencyPath) -> str: ) +class EnvironmentNotFoundError(NotFound): + """Raised where an environment key does not exist.""" + + default_code = "environment_not_found" + + def __init__(self, environment_api_key: str) -> None: + super().__init__( + { + "code": self.default_code, + "message": f"Environment key '{environment_api_key}' does not exist.", + } + ) + + class FeatureNotFoundError(NotFound): """Raised where a feature ID is not in the environment's project.""" diff --git a/api/features/dependencies/services.py b/api/features/dependencies/services.py index babdd22052bf..c489e0c30b86 100644 --- a/api/features/dependencies/services.py +++ b/api/features/dependencies/services.py @@ -4,6 +4,7 @@ import structlog from django.db import transaction +from django.db.models import QuerySet from flag_engine.segments import constants from ordered_model.models import OrderedModelQuerySet # type: ignore[import-untyped] @@ -27,16 +28,14 @@ from features.dependencies.models import SegmentFlagReference from features.dependencies.types import ( DependencyEdge, + DependencyList, DependencyPath, FeatureName, ReferencingEnvironment, ) from features.models import Feature, FeatureSegment from segments.models import Segment -from segments.services import ( - get_all_live_or_scheduled_overrides, - write_segment_rules, -) +from segments.services import get_live_overrides, write_segment_rules from segments.types import SegmentCondition, SegmentRule from users.models import FFAdminUser @@ -132,13 +131,15 @@ def validate_segment_flag_dependencies(segment: "Segment") -> None: return edges_by_environment_id: dict[int, dict[FeatureName, list[DependencyEdge]]] = {} for override in ( - get_all_live_or_scheduled_overrides() + get_live_overrides(include_scheduled=True) .filter(segment=segment) .select_related("environment", "feature") ): if override.environment_id not in edges_by_environment_id: edges_by_environment_id[override.environment_id] = _get_dependency_edges( - override.environment + get_live_overrides(include_scheduled=True).filter( + environment=override.environment + ) ) edges = edges_by_environment_id[override.environment_id] pending: list[DependencyPath] = [ @@ -172,7 +173,7 @@ def validate_segment_flag_dependencies(segment: "Segment") -> None: def _get_dependency_edges( - environment: Environment, + overrides: "QuerySet[FeatureSegment]", ) -> dict[FeatureName, list[DependencyEdge]]: edges: dict[FeatureName, list[DependencyEdge]] = defaultdict(list) for ( @@ -185,20 +186,16 @@ def _get_dependency_edges( segment_rules, condition_json_path, is_system_segment, - ) in ( - get_all_live_or_scheduled_overrides() - .filter(environment=environment, segment__flag_references__isnull=False) - .values_list( - "feature__id", - "feature__name", - "segment__flag_references__prerequisite_feature__id", - "segment__flag_references__prerequisite_feature__name", - "segment__id", - "segment__name", - "segment__rules_data", - "segment__flag_references__condition_json_path", - "segment__is_system_segment", - ) + ) in overrides.filter(segment__flag_references__isnull=False).values_list( + "feature__id", + "feature__name", + "segment__flag_references__prerequisite_feature__id", + "segment__flag_references__prerequisite_feature__name", + "segment__id", + "segment__name", + "segment__rules_data", + "segment__flag_references__condition_json_path", + "segment__is_system_segment", ): assert segment_rules is not None edges[feature_name].append( @@ -220,6 +217,33 @@ def _get_dependency_edges( return edges +def list_flag_dependencies( + *, + environment: Environment, + feature: Feature, +) -> DependencyList: + """List the features the feature depends on in the environment.""" + edges = _get_dependency_edges(get_live_overrides().filter(environment=environment)) + return {"results": edges[feature.name]} + + +def list_flag_dependents( + *, + environment: Environment, + feature: Feature, +) -> DependencyList: + """List the features depending on the feature in the environment.""" + edges = _get_dependency_edges(get_live_overrides().filter(environment=environment)) + return { + "results": [ + edge + for feature_edges in edges.values() + for edge in feature_edges + if edge["prerequisite"]["id"] == feature.id + ] + } + + def create_flag_dependency( *, environment: Environment, @@ -254,7 +278,9 @@ def create_flag_dependency( } segment_name = f"{feature.name}-dependencies-{environment.api_key}" with transaction.atomic(): - edges = _get_dependency_edges(environment) + edges = _get_dependency_edges( + get_live_overrides(include_scheduled=True).filter(environment=environment) + ) if existing_edges := [ edge for edge in edges[feature.name] @@ -298,11 +324,9 @@ def create_flag_dependency( ) write_segment_rules(segment, rules) index_segment_flag_references(segment) - overrides: OrderedModelQuerySet = ( - get_all_live_or_scheduled_overrides().filter( - environment=environment, feature=feature - ) - ) + overrides: OrderedModelQuerySet = get_live_overrides( + include_scheduled=True + ).filter(environment=environment, feature=feature) update_flag( environment=environment, feature=feature, diff --git a/api/features/dependencies/types.py b/api/features/dependencies/types.py index 12bebe2a3b48..f5068a68ce9c 100644 --- a/api/features/dependencies/types.py +++ b/api/features/dependencies/types.py @@ -38,3 +38,9 @@ class DependencyEdge(TypedDict): DependencyPath = list[DependencyEdge] + + +class DependencyList(TypedDict): + """The live dependencies a feature takes part in within an environment.""" + + results: list[DependencyEdge] diff --git a/api/features/dependencies/views.py b/api/features/dependencies/views.py index 2f0d888a7482..ec9275e0a56a 100644 --- a/api/features/dependencies/views.py +++ b/api/features/dependencies/views.py @@ -1,4 +1,3 @@ -from django.shortcuts import get_object_or_404 from drf_spectacular.utils import PolymorphicProxySerializer, extend_schema from rest_framework import status from rest_framework.permissions import IsAuthenticated @@ -10,14 +9,27 @@ from features.dependencies.exceptions import ( DependencyConflictDetail, DependencyErrorDetail, + EnvironmentNotFoundError, FeatureNotFoundError, ) from features.dependencies.permissions import check_manage_permissions -from features.dependencies.services import create_flag_dependency -from features.dependencies.types import DependencyEdge +from features.dependencies.services import ( + create_flag_dependency, + list_flag_dependencies, + list_flag_dependents, +) +from features.dependencies.types import DependencyEdge, DependencyList +from features.future.permissions import check_read_permissions from features.models import Feature +def _get_environment(environment_api_key: str) -> Environment: + try: + return Environment.objects.get(api_key=environment_api_key) # type: ignore[no-any-return] + except Environment.DoesNotExist: + raise EnvironmentNotFoundError(environment_api_key) from None + + def _get_feature(environment: Environment, feature_id: int) -> Feature: try: return Feature.objects.get( # type: ignore[no-any-return] @@ -51,7 +63,7 @@ def post( feature_id: int, prerequisite_feature_id: int, ) -> Response: - environment = get_object_or_404(Environment, api_key=environment_api_key) + environment = _get_environment(environment_api_key) check_manage_permissions(request.user, environment) feature = _get_feature(environment, feature_id) prerequisite_feature = _get_feature(environment, prerequisite_feature_id) @@ -64,3 +76,47 @@ def post( ), status=status.HTTP_201_CREATED, ) + + +class FeatureDependenciesAPIView(APIView): + """List the features a feature depends on in an environment.""" + + permission_classes = [IsAuthenticated] + + @extend_schema( + responses={200: DependencyList, 404: DependencyErrorDetail}, + description="List the features the feature depends on in the environment.", + ) + def get( + self, + request: AuthenticatedRequest, + environment_api_key: str, + feature_id: int, + ) -> Response: + environment = _get_environment(environment_api_key) + check_read_permissions(request.user, environment) + feature = _get_feature(environment, feature_id) + return Response( + list_flag_dependencies(environment=environment, feature=feature) + ) + + +class FeatureDependentsAPIView(APIView): + """List the features depending on a feature in an environment.""" + + permission_classes = [IsAuthenticated] + + @extend_schema( + responses={200: DependencyList, 404: DependencyErrorDetail}, + description="List the features depending on the feature in the environment.", + ) + def get( + self, + request: AuthenticatedRequest, + environment_api_key: str, + feature_id: int, + ) -> Response: + environment = _get_environment(environment_api_key) + check_read_permissions(request.user, environment) + feature = _get_feature(environment, feature_id) + return Response(list_flag_dependents(environment=environment, feature=feature)) diff --git a/api/segments/serializers.py b/api/segments/serializers.py index 45221c128347..1248b16c4db1 100644 --- a/api/segments/serializers.py +++ b/api/segments/serializers.py @@ -20,7 +20,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_all_live_or_scheduled_overrides +from segments.services import get_live_overrides from segments.types import ( LegacySegmentRule, ) @@ -195,7 +195,9 @@ 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_all_live_or_scheduled_overrides().filter(segment=segment).exists() + return ( + get_live_overrides(include_scheduled=True).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 8015881f5743..b0225256fbd8 100644 --- a/api/segments/services.py +++ b/api/segments/services.py @@ -17,13 +17,17 @@ from segments.models import Segment, SegmentRule -def get_all_live_or_scheduled_overrides() -> "QuerySet[FeatureSegment]": - """Get the feature overrides that are live now or scheduled to go live.""" +def get_live_overrides( + *, include_scheduled: bool = False +) -> "QuerySet[FeatureSegment]": + """Get the feature overrides that are live now, or also 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) + if not include_scheduled: + with_feature_versioning_v1 &= models.Q(live_from__lte=timezone.now()) superseding_versions = EnvironmentFeatureVersion.objects.filter( environment_id=models.OuterRef("environment_id"), @@ -34,10 +38,17 @@ def get_all_live_or_scheduled_overrides() -> "QuerySet[FeatureSegment]": ) # Filtering on not superseded is the same as filtering on the latest # live EFV but uses the index on feature, environment. - with_feature_versioning_v2 = models.Q( + published_v2_versions = models.Q( environment__use_v2_feature_versioning=True, environment_feature_version__published_at__isnull=False, - ) & ~models.Exists(superseding_versions) + ) + if not include_scheduled: + published_v2_versions &= models.Q( + environment_feature_version__live_from__lte=timezone.now(), + ) + with_feature_versioning_v2 = published_v2_versions & ~models.Exists( + superseding_versions + ) live_or_scheduled_feature_states = FeatureState.objects.filter( with_feature_versioning_v1 | with_feature_versioning_v2, diff --git a/api/segments/views.py b/api/segments/views.py index 79a005b68351..f6d1582b7b2a 100644 --- a/api/segments/views.py +++ b/api/segments/views.py @@ -46,7 +46,7 @@ SegmentMembersResponseSerializer, SegmentSerializer, ) -from .services import delete_segment, get_all_live_or_scheduled_overrides +from .services import delete_segment, get_live_overrides if TYPE_CHECKING: from users.models import FFAdminUser @@ -106,7 +106,7 @@ def get_queryset(self): # type: ignore[no-untyped-def] project=project, is_system_segment=False ).annotate( has_overrides=models.Exists( - get_all_live_or_scheduled_overrides().filter( + get_live_overrides(include_scheduled=True).filter( segment_id=models.OuterRef("pk") ) ) @@ -270,7 +270,11 @@ def _check_segment_is_deletable(self, segment: Segment) -> None: """ if not segment.project.is_workflow_enabled: return - if not get_all_live_or_scheduled_overrides().filter(segment=segment).exists(): + if ( + not get_live_overrides(include_scheduled=True) + .filter(segment=segment) + .exists() + ): return api_error = ChangeRequestsEnabledError( "Cannot delete a segment with feature overrides in a project with " diff --git a/api/tests/integration/features/conftest.py b/api/tests/integration/features/conftest.py index 89e995c231fd..67b90304a3bf 100644 --- a/api/tests/integration/features/conftest.py +++ b/api/tests/integration/features/conftest.py @@ -1,8 +1,15 @@ +from datetime import datetime + import pytest from rest_framework.test import APIClient from environments.models import Environment +from features.models import FeatureSegment, FeatureState +from features.versioning.models import EnvironmentFeatureVersion from features.versioning.tasks import enable_v2_versioning +from features.workflows.core.models import ChangeRequest +from tests.types import CreateChangeRequestSegmentOverrideFixture +from users.models import FFAdminUser @pytest.fixture() @@ -34,3 +41,50 @@ def versioned_environment( if request.param == "feature_versioning_v2": enable_v2_versioning(environment_id=environment) return Environment.objects.get(id=environment) # type: ignore[no-any-return] + + +@pytest.fixture() +def create_change_request_segment_override( + admin_user: FFAdminUser, +) -> CreateChangeRequestSegmentOverrideFixture: + """Return a callable putting a segment override through a change request, whichever versioning is in use.""" + + def _create_change_request_segment_override( + environment: Environment, + feature_id: int, + segment_id: int, + committed: bool = False, + live_from: datetime | None = None, + ) -> None: + change_request = ChangeRequest.objects.create( + environment=environment, title="Pending", user=admin_user + ) + version = ( + EnvironmentFeatureVersion.objects.create( + environment=environment, + feature_id=feature_id, + change_request=change_request, + live_from=live_from, + ) + if environment.use_v2_feature_versioning + else None + ) + feature_segment = FeatureSegment.objects.create( + environment=environment, + feature_id=feature_id, + segment_id=segment_id, + environment_feature_version=version, + ) + FeatureState.objects.create( + environment=environment, + feature_id=feature_id, + feature_segment=feature_segment, + environment_feature_version=version, + change_request=None if version else change_request, + live_from=live_from, + enabled=True, + ) + if committed: + change_request.commit(committed_by=admin_user) + + return _create_change_request_segment_override diff --git a/api/tests/integration/features/dependencies/test_dependency_management.py b/api/tests/integration/features/dependencies/test_dependency_management.py index 0dcf3e26d4c4..096466a5ea52 100644 --- a/api/tests/integration/features/dependencies/test_dependency_management.py +++ b/api/tests/integration/features/dependencies/test_dependency_management.py @@ -1,15 +1,21 @@ +from datetime import timedelta + import pytest from common.environments.permissions import UPDATE_FEATURE_STATE +from django.utils import timezone from pytest_structlog import StructuredLogCapture from rest_framework.test import APIClient from audit.models import AuditLog +from environments.models import Environment from features.dependencies.models import SegmentFlagReference from features.dependencies.types import DependencyEdge from features.models import Feature from organisations.models import Organisation from segments.models import Segment +from segments.types import SegmentCondition, SegmentRule from tests.types import ( + CreateChangeRequestSegmentOverrideFixture, CreateSegmentOverrideFixture, WithEnvironmentPermissionsCallable, ) @@ -735,3 +741,503 @@ def test_add_feature_dependency__missing_environment_permission__responds_403_wi assert not SegmentFlagReference.objects.exists() assert list(AuditLog.objects.filter(related_object_type="FEATURE")) == [] assert not log.has("dependencies.created") + + +def test_list_feature_dependencies__feature_has_prerequisites__responds_200_with_prerequisites( + admin_client: APIClient, + create_segment_override: CreateSegmentOverrideFixture, + environment_api_key: str, + project: int, +) -> None: + # Given + feature = Feature.objects.create(name="checkout", project_id=project) + payments = Feature.objects.create(name="payments", project_id=project) + inventory = Feature.objects.create(name="inventory", project_id=project) + dependency_via_system_segment = admin_client.post( + f"/api/v1/environments/{environment_api_key}/features/{feature.id}/dependencies/{payments.id}/", + ).json() + user_segment_id = admin_client.post( + f"/api/v1/projects/{project}/segments/", + data={ + "name": "stocked", + "rules": ( + needs_inventory := [ + SegmentRule( + type="ALL", + conditions=[ + SegmentCondition( + property="$.flags.inventory.enabled", + operator="EQUAL", + value="true", + description=None, + ), + ], + rules=[], + ), + ] + ), + }, + format="json", + ).json()["id"] + create_segment_override( + environment_api_key, feature.id, user_segment_id, priority=1 + ) + + # When + response = admin_client.get( + f"/api/v1/environments/{environment_api_key}/features/{feature.id}/dependencies/", + ) + + # Then + assert response.status_code == 200 + dependency_via_user_segment = DependencyEdge( + { + "feature": {"id": feature.id, "name": "checkout"}, + "prerequisite": {"id": inventory.id, "name": "inventory"}, + "segment": { + "id": user_segment_id, + "name": "stocked", + "rules": needs_inventory, + "condition_json_path": "$[0].conditions[0]", + "is_system": False, + }, + } + ) + assert response.json() == { + "results": [dependency_via_system_segment, dependency_via_user_segment] + } + + +def test_list_feature_dependencies__feature_has_no_prerequisites__responds_200_with_empty_list( + admin_client: APIClient, + environment_api_key: str, + project: int, +) -> None: + # Given + feature = Feature.objects.create(name="checkout", project_id=project) + dependent = Feature.objects.create(name="storefront", project_id=project) + admin_client.post( + f"/api/v1/environments/{environment_api_key}/features/{dependent.id}/dependencies/{feature.id}/", + ) + + # When + response = admin_client.get( + f"/api/v1/environments/{environment_api_key}/features/{feature.id}/dependencies/", + ) + + # Then + assert response.status_code == 200 + assert response.json() == {"results": []} + + +def test_list_feature_dependencies__uncommitted_prerequisite__responds_200_without_it( + admin_client: APIClient, + create_change_request_segment_override: CreateChangeRequestSegmentOverrideFixture, + environment_api_key: str, + project: int, + versioned_environment: Environment, +) -> None: + # Given + checkout = Feature.objects.create(name="checkout", project_id=project) + payments = Feature.objects.create(name="payments", project_id=project) + Feature.objects.create(name="inventory", project_id=project) + dependency_via_system_segment = admin_client.post( + f"/api/v1/environments/{environment_api_key}/features/{checkout.id}/dependencies/{payments.id}/", + ).json() + user_segment_id = admin_client.post( + f"/api/v1/projects/{project}/segments/", + data={ + "name": "stocked", + "rules": [ + { + "type": "ALL", + "conditions": [ + { + "property": "$.flags.inventory.enabled", + "operator": "EQUAL", + "value": "true", + "description": None, + } + ], + "rules": [], + } + ], + }, + format="json", + ).json()["id"] + create_change_request_segment_override( + versioned_environment, checkout.id, user_segment_id + ) + + # When + response = admin_client.get( + f"/api/v1/environments/{environment_api_key}/features/{checkout.id}/dependencies/", + ) + + # Then + assert response.status_code == 200 + assert response.json() == {"results": [dependency_via_system_segment]} + + +def test_list_feature_dependencies__committed_but_scheduled_prerequisite__responds_200_without_it( + admin_client: APIClient, + create_change_request_segment_override: CreateChangeRequestSegmentOverrideFixture, + environment_api_key: str, + project: int, + versioned_environment: Environment, +) -> None: + # Given + checkout = Feature.objects.create(name="checkout", project_id=project) + payments = Feature.objects.create(name="payments", project_id=project) + Feature.objects.create(name="inventory", project_id=project) + dependency_via_system_segment = admin_client.post( + f"/api/v1/environments/{environment_api_key}/features/{checkout.id}/dependencies/{payments.id}/", + ).json() + user_segment_id = admin_client.post( + f"/api/v1/projects/{project}/segments/", + data={ + "name": "stocked", + "rules": [ + { + "type": "ALL", + "conditions": [ + { + "property": "$.flags.inventory.enabled", + "operator": "EQUAL", + "value": "true", + "description": None, + } + ], + "rules": [], + } + ], + }, + format="json", + ).json()["id"] + create_change_request_segment_override( + versioned_environment, + checkout.id, + user_segment_id, + committed=True, + live_from=timezone.now() + timedelta(days=1), + ) + + # When + response = admin_client.get( + f"/api/v1/environments/{environment_api_key}/features/{checkout.id}/dependencies/", + ) + + # Then + assert response.status_code == 200 + assert response.json() == {"results": [dependency_via_system_segment]} + + +def test_list_feature_dependencies__another_environment__responds_200_without_it( + admin_client: APIClient, + environment_api_key: str, + other_environment: Environment, + project: int, +) -> None: + # Given + checkout = Feature.objects.create(name="checkout", project_id=project) + payments = Feature.objects.create(name="payments", project_id=project) + inventory = Feature.objects.create(name="inventory", project_id=project) + dependency_via_system_segment = admin_client.post( + f"/api/v1/environments/{environment_api_key}/features/{checkout.id}/dependencies/{payments.id}/", + ).json() + admin_client.post( + f"/api/v1/environments/{other_environment.api_key}/features/{checkout.id}/dependencies/{inventory.id}/", + ) + + # When + response = admin_client.get( + f"/api/v1/environments/{environment_api_key}/features/{checkout.id}/dependencies/", + ) + + # Then + assert response.status_code == 200 + assert response.json() == {"results": [dependency_via_system_segment]} + + +def test_list_feature_dependents__feature_is_prerequisite__responds_200_with_dependents( + admin_client: APIClient, + create_segment_override: CreateSegmentOverrideFixture, + environment_api_key: str, + other_environment: Environment, + project: int, +) -> None: + # Given + feature = Feature.objects.create(name="payments", project_id=project) + checkout = Feature.objects.create(name="checkout", project_id=project) + storefront = Feature.objects.create(name="storefront", project_id=project) + dependency_via_system_segment = admin_client.post( + f"/api/v1/environments/{environment_api_key}/features/{checkout.id}/dependencies/{feature.id}/", + ).json() + user_segment_id = admin_client.post( + f"/api/v1/projects/{project}/segments/", + data={ + "name": "paying", + "rules": ( + needs_payments := [ + SegmentRule( + type="ALL", + conditions=[ + SegmentCondition( + property="$.flags.payments.enabled", + operator="EQUAL", + value="true", + description=None, + ), + ], + rules=[], + ), + ] + ), + }, + format="json", + ).json()["id"] + create_segment_override( + environment_api_key, storefront.id, user_segment_id, priority=1 + ) + admin_client.post( + f"/api/v1/environments/{other_environment.api_key}/features/{storefront.id}/dependencies/{feature.id}/", + ) + + # When + response = admin_client.get( + f"/api/v1/environments/{environment_api_key}/features/{feature.id}/dependents/", + ) + + # Then + assert response.status_code == 200 + dependency_via_user_segment = DependencyEdge( + { + "feature": {"id": storefront.id, "name": "storefront"}, + "prerequisite": {"id": feature.id, "name": "payments"}, + "segment": { + "id": user_segment_id, + "name": "paying", + "rules": needs_payments, + "condition_json_path": "$[0].conditions[0]", + "is_system": False, + }, + } + ) + assert response.json() == { + "results": [dependency_via_system_segment, dependency_via_user_segment] + } + + +def test_list_feature_dependents__feature_is_not_prerequisite__responds_200_with_empty_list( + admin_client: APIClient, + environment_api_key: str, + project: int, +) -> None: + # Given + feature = Feature.objects.create(name="checkout", project_id=project) + prerequisite = Feature.objects.create(name="payments", project_id=project) + admin_client.post( + f"/api/v1/environments/{environment_api_key}/features/{feature.id}/dependencies/{prerequisite.id}/", + ) + + # When + response = admin_client.get( + f"/api/v1/environments/{environment_api_key}/features/{feature.id}/dependents/", + ) + + # Then + assert response.status_code == 200 + assert response.json() == {"results": []} + + +def test_list_feature_dependents__uncommitted_dependent__responds_200_without_it( + admin_client: APIClient, + create_change_request_segment_override: CreateChangeRequestSegmentOverrideFixture, + environment_api_key: str, + project: int, + versioned_environment: Environment, +) -> None: + # Given + payments = Feature.objects.create(name="payments", project_id=project) + checkout = Feature.objects.create(name="checkout", project_id=project) + storefront = Feature.objects.create(name="storefront", project_id=project) + dependency_via_system_segment = admin_client.post( + f"/api/v1/environments/{environment_api_key}/features/{checkout.id}/dependencies/{payments.id}/", + ).json() + user_segment_id = admin_client.post( + f"/api/v1/projects/{project}/segments/", + data={ + "name": "paying", + "rules": [ + { + "type": "ALL", + "conditions": [ + { + "property": "$.flags.payments.enabled", + "operator": "EQUAL", + "value": "true", + "description": None, + } + ], + "rules": [], + } + ], + }, + format="json", + ).json()["id"] + create_change_request_segment_override( + versioned_environment, storefront.id, user_segment_id + ) + + # When + response = admin_client.get( + f"/api/v1/environments/{environment_api_key}/features/{payments.id}/dependents/", + ) + + # Then + assert response.status_code == 200 + assert response.json() == {"results": [dependency_via_system_segment]} + + +def test_list_feature_dependents__committed_but_scheduled_dependent__responds_200_without_it( + admin_client: APIClient, + create_change_request_segment_override: CreateChangeRequestSegmentOverrideFixture, + environment_api_key: str, + project: int, + versioned_environment: Environment, +) -> None: + # Given + payments = Feature.objects.create(name="payments", project_id=project) + checkout = Feature.objects.create(name="checkout", project_id=project) + storefront = Feature.objects.create(name="storefront", project_id=project) + dependency_via_system_segment = admin_client.post( + f"/api/v1/environments/{environment_api_key}/features/{checkout.id}/dependencies/{payments.id}/", + ).json() + user_segment_id = admin_client.post( + f"/api/v1/projects/{project}/segments/", + data={ + "name": "paying", + "rules": [ + { + "type": "ALL", + "conditions": [ + { + "property": "$.flags.payments.enabled", + "operator": "EQUAL", + "value": "true", + "description": None, + } + ], + "rules": [], + } + ], + }, + format="json", + ).json()["id"] + create_change_request_segment_override( + versioned_environment, + storefront.id, + user_segment_id, + committed=True, + live_from=timezone.now() + timedelta(days=1), + ) + + # When + response = admin_client.get( + f"/api/v1/environments/{environment_api_key}/features/{payments.id}/dependents/", + ) + + # Then + assert response.status_code == 200 + assert response.json() == {"results": [dependency_via_system_segment]} + + +def test_list_feature_dependents__another_environment__responds_200_without_it( + admin_client: APIClient, + environment_api_key: str, + other_environment: Environment, + project: int, +) -> None: + # Given + payments = Feature.objects.create(name="payments", project_id=project) + checkout = Feature.objects.create(name="checkout", project_id=project) + storefront = Feature.objects.create(name="storefront", project_id=project) + dependency_via_system_segment = admin_client.post( + f"/api/v1/environments/{environment_api_key}/features/{checkout.id}/dependencies/{payments.id}/", + ).json() + admin_client.post( + f"/api/v1/environments/{other_environment.api_key}/features/{storefront.id}/dependencies/{payments.id}/", + ) + + # When + response = admin_client.get( + f"/api/v1/environments/{environment_api_key}/features/{payments.id}/dependents/", + ) + + # Then + assert response.status_code == 200 + assert response.json() == {"results": [dependency_via_system_segment]} + + +@pytest.mark.parametrize("relationship", ["dependencies", "dependents"]) +def test_either_list__feature_does_not_exist__responds_404_with_error( + admin_client: APIClient, + environment_api_key: str, + relationship: str, +) -> None: + # Given / When + response = admin_client.get( + f"/api/v1/environments/{environment_api_key}/features/777333777/{relationship}/", + ) + + # Then + assert response.status_code == 404 + assert response.json() == { + "code": "feature_not_found", + "message": "Feature ID '777333777' does not exist in the project.", + } + + +@pytest.mark.parametrize("relationship", ["dependencies", "dependents"]) +def test_either_list__environment_does_not_exist__responds_404_with_error( + admin_client: APIClient, + feature: int, + relationship: str, +) -> None: + # Given / When + response = admin_client.get( + f"/api/v1/environments/no-such-thing/features/{feature}/{relationship}/", + ) + + # Then + assert response.status_code == 404 + assert response.json() == { + "code": "environment_not_found", + "message": "Environment key 'no-such-thing' does not exist.", + } + + +@pytest.mark.parametrize("relationship", ["dependencies", "dependents"]) +def test_either_list__missing_environment_permission__responds_404_with_error( + environment: int, + environment_api_key: str, + organisation: int, + project: int, + relationship: str, + staff_client: APIClient, + staff_user: FFAdminUser, + with_environment_permissions: WithEnvironmentPermissionsCallable, +) -> None: + # Given + feature = Feature.objects.create(name="checkout", project_id=project) + staff_user.add_organisation(Organisation.objects.get(id=organisation)) + with_environment_permissions([], environment, False) + + # When + response = staff_client.get( + f"/api/v1/environments/{environment_api_key}/features/{feature.id}/{relationship}/", + ) + + # Then + assert response.status_code == 404 + assert response.json() == {"detail": "Not found."} diff --git a/api/tests/types.py b/api/tests/types.py index ffbfd955bda5..a036add05073 100644 --- a/api/tests/types.py +++ b/api/tests/types.py @@ -1,7 +1,9 @@ +from datetime import datetime from typing import Callable, Literal, Optional, Protocol from django_test_migrations.migrator import Migrator +from environments.models import Environment from environments.permissions.models import UserEnvironmentPermission from organisations.permissions.models import UserOrganisationPermission from projects.models import UserProjectPermission @@ -55,3 +57,14 @@ def __call__( enabled: bool = True, priority: int | None = None, ) -> None: ... + + +class CreateChangeRequestSegmentOverrideFixture(Protocol): + def __call__( + self, + environment: Environment, + feature_id: int, + segment_id: int, + committed: bool = False, + live_from: datetime | None = None, + ) -> None: ... diff --git a/api/tests/unit/segments/test_unit_segments_services.py b/api/tests/unit/segments/test_unit_segments_services.py index 93c0be9e3389..7f5948aff934 100644 --- a/api/tests/unit/segments/test_unit_segments_services.py +++ b/api/tests/unit/segments/test_unit_segments_services.py @@ -19,7 +19,7 @@ from organisations.models import Organisation from projects.models import Project from segments.models import Condition, Segment, SegmentRule -from segments.services import delete_segment, get_all_live_or_scheduled_overrides +from segments.services import delete_segment, get_live_overrides from users.models import FFAdminUser @@ -370,7 +370,7 @@ def test_copy_rules_and_conditions_from__varying_segment_sizes__query_count_is_c 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( +def test_get_live_overrides__feature_versioning_v1_uncommitted_change_request__returns_no_overrides( feature_segment: FeatureSegment, feature: Feature, environment: Environment, @@ -386,14 +386,14 @@ def test_get_all_live_or_scheduled_overrides__feature_versioning_v1_uncommitted_ ) # When - overrides = list(get_all_live_or_scheduled_overrides()) + overrides = list(get_live_overrides(include_scheduled=True)) # 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( +def test_get_live_overrides__feature_versioning_v1_committed_change_request__returns_distinct_overrides( feature_segment: FeatureSegment, feature: Feature, environment: Environment, @@ -411,14 +411,14 @@ def test_get_all_live_or_scheduled_overrides__feature_versioning_v1_committed_ch change_request.commit(admin_user) # When - overrides = list(get_all_live_or_scheduled_overrides()) + overrides = list(get_live_overrides(include_scheduled=True)) # 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( +def test_get_live_overrides__feature_versioning_v1_scheduled_feature_change_including_scheduled__returns_scheduled_overrides( feature_segment: FeatureSegment, feature: Feature, environment: Environment, @@ -437,13 +437,59 @@ def test_get_all_live_or_scheduled_overrides__feature_versioning_v1_scheduled_fe change_request.commit(admin_user) # When - overrides = list(get_all_live_or_scheduled_overrides()) + overrides = list(get_live_overrides(include_scheduled=True)) # Then assert overrides == [feature_segment] -def test_get_all_live_or_scheduled_overrides__feature_versioning_v2_uncommitted_change_request__returns_no_overrides( +def test_get_live_overrides__feature_versioning_v1_scheduled_feature_change__returns_no_overrides( + feature_segment: FeatureSegment, + feature: Feature, + environment: Environment, +) -> None: + # Given + FeatureState.objects.create( + feature_segment=feature_segment, + feature=feature, + environment=environment, + live_from=timezone.now() + timedelta(days=1), + ) + + # When + overrides = list(get_live_overrides()) + + # Then + assert overrides == [] + + +@pytest.mark.usefixtures("segment_featurestate") +def test_get_live_overrides__feature_versioning_v1_live_override_with_scheduled_feature_change__returns_live_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_live_overrides()) + + # Then + assert overrides == [feature_segment] + + +def test_get_live_overrides__feature_versioning_v2_uncommitted_change_request__returns_no_overrides( environment_v2_versioning: Environment, feature: Feature, segment: Segment, @@ -468,13 +514,13 @@ def test_get_all_live_or_scheduled_overrides__feature_versioning_v2_uncommitted_ ) # When - overrides = list(get_all_live_or_scheduled_overrides()) + overrides = list(get_live_overrides(include_scheduled=True)) # Then assert overrides == [] -def test_get_all_live_or_scheduled_overrides__feature_versioning_v2_committed_change_request__returns_distinct_overrides( +def test_get_live_overrides__feature_versioning_v2_committed_change_request__returns_distinct_overrides( environment_v2_versioning: Environment, feature: Feature, segment: Segment, @@ -507,13 +553,13 @@ def test_get_all_live_or_scheduled_overrides__feature_versioning_v2_committed_ch change_request.commit(admin_user) # When - overrides = list(get_all_live_or_scheduled_overrides()) + overrides = list(get_live_overrides(include_scheduled=True)) # Then assert overrides == [committed_override] -def test_get_all_live_or_scheduled_overrides__feature_versioning_v2_scheduled_feature_change__returns_distinct_overrides( +def test_get_live_overrides__feature_versioning_v2_scheduled_feature_change_including_scheduled__returns_live_and_scheduled_overrides( environment_v2_versioning: Environment, feature: Feature, segment: Segment, @@ -548,7 +594,45 @@ def test_get_all_live_or_scheduled_overrides__feature_versioning_v2_scheduled_fe change_request.commit(admin_user) # When - overrides = list(get_all_live_or_scheduled_overrides().order_by("id")) + overrides = list(get_live_overrides(include_scheduled=True).order_by("id")) # Then assert overrides == [live_override, scheduled_override] + + +def test_get_live_overrides__feature_versioning_v2_scheduled_feature_change__returns_live_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, + ) + EnvironmentFeatureVersion.objects.create( + environment=environment_v2_versioning, + feature=feature, + change_request=change_request, + live_from=timezone.now() + timedelta(days=1), + ) + change_request.commit(admin_user) + + # When + overrides = list(get_live_overrides()) + + # Then + assert overrides == [live_override] diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index 74f08d0935c0..b6a7781db8f3 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -289,11 +289,11 @@ Attributes: ### `features.dependencies.create_failed` Logged at `info` from: - - `api/features/dependencies/services.py:155` - - `api/features/dependencies/services.py:243` - - `api/features/dependencies/services.py:263` - - `api/features/dependencies/services.py:268` - - `api/features/dependencies/services.py:278` + - `api/features/dependencies/services.py:156` + - `api/features/dependencies/services.py:267` + - `api/features/dependencies/services.py:289` + - `api/features/dependencies/services.py:294` + - `api/features/dependencies/services.py:304` Attributes: - `environment.key` @@ -305,7 +305,7 @@ Attributes: ### `features.dependencies.created` Logged at `info` from: - - `api/features/dependencies/services.py:125` + - `api/features/dependencies/services.py:124` Attributes: - `environment.key` @@ -317,8 +317,8 @@ Attributes: ### `features.dependencies.deleted` Logged at `info` from: - - `api/features/dependencies/services.py:97` - - `api/features/dependencies/services.py:123` + - `api/features/dependencies/services.py:96` + - `api/features/dependencies/services.py:122` Attributes: - `environment.key` @@ -707,7 +707,7 @@ Attributes: ### `segments.delete_rejected` Logged at `warning` from: - - `api/segments/views.py:279` + - `api/segments/views.py:283` Attributes: - `organisation.id` @@ -718,7 +718,7 @@ Attributes: ### `segments.serializers.segment_revision_created` Logged at `info` from: - - `api/segments/serializers.py:237` + - `api/segments/serializers.py:239` Attributes: - `revision_id` diff --git a/mcp/src/flagsmith_mcp/openapi.json b/mcp/src/flagsmith_mcp/openapi.json index e9ad4aeb3f19..9a390efc37a9 100644 --- a/mcp/src/flagsmith_mcp/openapi.json +++ b/mcp/src/flagsmith_mcp/openapi.json @@ -4150,6 +4150,91 @@ ], "title": "SegmentRule" }, + "DependencyListSegmentCondition": { + "type": "object", + "properties": { + "property": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "title": "Property" + }, + "operator": { + "allOf": [ + { + "$ref": "#/components/schemas/Operator2a7Enum" + } + ], + "title": "Operator" + }, + "value": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "title": "Value" + }, + "description": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "title": "Description" + } + }, + "required": [ + "property", + "operator", + "value", + "description" + ], + "title": "SegmentCondition" + }, + "DependencyListSegmentRule": { + "type": "object", + "properties": { + "type": { + "allOf": [ + { + "$ref": "#/components/schemas/TypeC8eEnum" + } + ], + "title": "Type" + }, + "conditions": { + "type": "array", + "items": { + "$ref": "#/components/schemas/DependencyListSegmentCondition" + }, + "title": "Conditions" + }, + "rules": { + "type": "array", + "items": { + "$ref": "#/components/schemas/DependencyListSegmentRule" + }, + "title": "Rules" + } + }, + "required": [ + "type", + "conditions" + ], + "title": "SegmentRule" + }, "DirectionEnum": { "description": "* `up` - Higher is better\n* `down` - Lower is better\n* `informational` - Informational only", "type": "string", diff --git a/openapi.yaml b/openapi.yaml index d068f8a58cd0..e01b0ba31c8b 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -4183,6 +4183,39 @@ paths: - Master API Key: [] tags: - Environments + '/api/v1/environments/{environment_api_key}/features/{feature_id}/dependencies/': + get: + operationId: api_v1_environments_features_dependencies_retrieve + description: List the features the feature depends on in the environment. + parameters: + - name: environment_api_key + in: path + required: true + schema: + type: string + - name: feature_id + in: path + required: true + schema: + type: integer + responses: + '200': + description: '' + content: + application/json: + schema: + $ref: '#/components/schemas/DependencyList' + '404': + description: '' + content: + application/json: + schema: + $ref: '#/components/schemas/DependencyErrorDetail' + security: + - tokenAuth: [] + - Master API Key: [] + tags: + - Features '/api/v1/environments/{environment_api_key}/features/{feature_id}/dependencies/{prerequisite_feature_id}/': post: operationId: api_v1_environments_features_dependencies_create @@ -4227,6 +4260,39 @@ paths: - Master API Key: [] tags: - Features + '/api/v1/environments/{environment_api_key}/features/{feature_id}/dependents/': + get: + operationId: api_v1_environments_features_dependents_retrieve + description: List the features depending on the feature in the environment. + parameters: + - name: environment_api_key + in: path + required: true + schema: + type: string + - name: feature_id + in: path + required: true + schema: + type: integer + responses: + '200': + description: '' + content: + application/json: + schema: + $ref: '#/components/schemas/DependencyList' + '404': + description: '' + content: + application/json: + schema: + $ref: '#/components/schemas/DependencyErrorDetail' + security: + - tokenAuth: [] + - Master API Key: [] + tags: + - Features '/api/v1/environments/{environment_api_key}/features/{feature_pk}/create-segment-override/': post: operationId: create_segment_override @@ -20904,6 +20970,124 @@ components: - code - message title: DependencyErrorDetail + DependencyList: + description: The live dependencies a feature takes part in within an environment. + type: object + properties: + results: + type: array + items: + $ref: '#/components/schemas/DependencyListDependencyEdge' + title: Results + required: + - results + title: DependencyList + DependencyListDependencyEdge: + description: One feature's dependency on another. + type: object + properties: + feature: + $ref: '#/components/schemas/DependencyList_Feature' + prerequisite: + $ref: '#/components/schemas/DependencyList_Feature' + segment: + $ref: '#/components/schemas/DependencyList_ReferencingSegment' + required: + - feature + - prerequisite + - segment + title: DependencyEdge + DependencyListSegmentCondition: + type: object + properties: + property: + anyOf: + - type: string + - type: 'null' + title: Property + operator: + allOf: + - $ref: '#/components/schemas/Operator2a7Enum' + title: Operator + value: + anyOf: + - type: string + - type: 'null' + title: Value + description: + anyOf: + - type: string + - type: 'null' + title: Description + required: + - property + - operator + - value + - description + title: SegmentCondition + DependencyListSegmentRule: + type: object + properties: + type: + allOf: + - $ref: '#/components/schemas/TypeC8eEnum' + title: Type + conditions: + type: array + items: + $ref: '#/components/schemas/DependencyListSegmentCondition' + title: Conditions + rules: + type: array + items: + $ref: '#/components/schemas/DependencyListSegmentRule' + title: Rules + required: + - type + - conditions + title: SegmentRule + DependencyList_Feature: + description: Either a feature or a prerequisite feature in a dependency graph. + type: object + properties: + id: + type: integer + title: Id + name: + type: string + title: Name + required: + - id + - name + title: _Feature + DependencyList_ReferencingSegment: + description: The segment whose rules hold the `$.flags` condition making up a dependency. + type: object + properties: + id: + type: integer + title: Id + name: + type: string + title: Name + rules: + type: array + items: + $ref: '#/components/schemas/DependencyListSegmentRule' + title: Rules + condition_json_path: + type: string + title: Condition Json Path + is_system: + type: boolean + title: Is System + required: + - id + - name + - rules + - condition_json_path + - is_system + title: _ReferencingSegment DependencyRefusedDetail: oneOf: - $ref: '#/components/schemas/DependencyErrorDetail' diff --git a/sdk/openapi.yaml b/sdk/openapi.yaml index 6e8c5959335a..b9068d57dcf7 100644 --- a/sdk/openapi.yaml +++ b/sdk/openapi.yaml @@ -271,6 +271,55 @@ components: - type - conditions title: SegmentRule + DependencyListSegmentCondition: + type: object + properties: + property: + anyOf: + - type: string + - type: 'null' + title: Property + operator: + allOf: + - $ref: '#/components/schemas/Operator2a7Enum' + title: Operator + value: + anyOf: + - type: string + - type: 'null' + title: Value + description: + anyOf: + - type: string + - type: 'null' + title: Description + required: + - property + - operator + - value + - description + title: SegmentCondition + DependencyListSegmentRule: + type: object + properties: + type: + allOf: + - $ref: '#/components/schemas/TypeC8eEnum' + title: Type + conditions: + type: array + items: + $ref: '#/components/schemas/DependencyListSegmentCondition' + title: Conditions + rules: + type: array + items: + $ref: '#/components/schemas/DependencyListSegmentRule' + title: Rules + required: + - type + - conditions + title: SegmentRule Operator2a7Enum: type: string enum: