diff --git a/api/conftest.py b/api/conftest.py index dec3538ec493..6ccd956544e3 100644 --- a/api/conftest.py +++ b/api/conftest.py @@ -364,6 +364,7 @@ def _with_project_permissions( @pytest.fixture() def environment_v2_versioning(environment): enable_v2_versioning(environment.id) + environment.refresh_from_db() return environment diff --git a/api/features/managers.py b/api/features/managers.py index b7fd39155210..a285e89c2a61 100644 --- a/api/features/managers.py +++ b/api/features/managers.py @@ -8,6 +8,8 @@ from ordered_model.models import OrderedModelManager from softdelete.models import SoftDeleteManager +from features.versioning.models import EnvironmentFeatureVersion + if typing.TYPE_CHECKING: from environments.models import Environment from features.models import FeatureState @@ -31,11 +33,11 @@ def get_live_feature_states( qs_filter = Q(environment=environment, deleted_at__isnull=True) if environment.use_v2_feature_versioning: - qs_filter &= Q( - environment_feature_version__isnull=False, - environment_feature_version__published_at__isnull=False, - environment_feature_version__live_from__lte=now, + latest_versions = EnvironmentFeatureVersion.objects.get_latest_versions( + environment ) + latest_version_uuids = [efv.uuid for efv in latest_versions] + qs_filter &= Q(environment_feature_version__uuid__in=latest_version_uuids) else: qs_filter &= Q( live_from__isnull=False, diff --git a/api/features/versioning/managers.py b/api/features/versioning/managers.py new file mode 100644 index 000000000000..32c41ad0a831 --- /dev/null +++ b/api/features/versioning/managers.py @@ -0,0 +1,23 @@ +import typing +from pathlib import Path + +from django.db.models.query import RawQuerySet +from softdelete.models import SoftDeleteManager + +if typing.TYPE_CHECKING: + from environments.models import Environment + + +with open(Path(__file__).parent.resolve() / "sql/get_latest_versions.sql") as f: + get_latest_versions_sql = f.read() + + +class EnvironmentFeatureVersionManager(SoftDeleteManager): + def get_latest_versions(self, environment: "Environment") -> RawQuerySet: + """ + Get the latest EnvironmentFeatureVersion objects + for a given environment. + """ + return self.raw( + get_latest_versions_sql, params={"environment_id": environment.id} + ) diff --git a/api/features/versioning/models.py b/api/features/versioning/models.py index ff70facdcc29..27600284c397 100644 --- a/api/features/versioning/models.py +++ b/api/features/versioning/models.py @@ -12,6 +12,7 @@ from django.utils import timezone from features.versioning.exceptions import FeatureVersioningError +from features.versioning.managers import EnvironmentFeatureVersionManager from features.versioning.signals import environment_feature_version_published if typing.TYPE_CHECKING: @@ -61,6 +62,8 @@ class EnvironmentFeatureVersion( blank=True, ) + objects = EnvironmentFeatureVersionManager() + class Meta: indexes = [Index(fields=("environment", "feature"))] ordering = ("-live_from",) diff --git a/api/features/versioning/sql/get_latest_versions.sql b/api/features/versioning/sql/get_latest_versions.sql new file mode 100644 index 000000000000..3c1257e8605b --- /dev/null +++ b/api/features/versioning/sql/get_latest_versions.sql @@ -0,0 +1,25 @@ +select + efv1."uuid", + efv1."published_at", + efv1."live_from" +from + feature_versioning_environmentfeatureversion efv1 +join ( + select + efv2."feature_id", + efv2."environment_id", + MAX(efv2."live_from") as "latest_release" + from + feature_versioning_environmentfeatureversion efv2 + where + efv2."deleted_at" is null + and efv2."published_at" is not null + group by + efv2."feature_id", + efv2."environment_id" +) latest_release_dates on + efv1."feature_id" = latest_release_dates."feature_id" + and efv1."environment_id" = latest_release_dates."environment_id" + and efv1."live_from" = latest_release_dates."latest_release" +where + efv1.environment_id = %(environment_id)s; \ No newline at end of file diff --git a/api/features/versioning/versioning_service.py b/api/features/versioning/versioning_service.py index abeb3a50c778..66732650c108 100644 --- a/api/features/versioning/versioning_service.py +++ b/api/features/versioning/versioning_service.py @@ -14,6 +14,9 @@ def get_environment_flags_queryset( """ Get a queryset of the latest live versions of an environments' feature states """ + if environment.use_v2_feature_versioning: + return _get_feature_states_queryset(environment, feature_name) + feature_states_list = get_environment_flags_list(environment, feature_name) return FeatureState.objects.filter(id__in=[fs.id for fs in feature_states_list]) @@ -36,26 +39,16 @@ def get_environment_flags_list( feature states. The logic to grab the latest version is then handled in python by building a dictionary. Returns a list of FeatureState objects. """ - additional_select_related_args = additional_select_related_args or tuple() - additional_prefetch_related_args = additional_prefetch_related_args or tuple() - - feature_states = ( - FeatureState.objects.get_live_feature_states( - environment=environment, additional_filters=additional_filters - ) - .select_related( - "environment", - "feature", - "feature_state_value", - "environment_feature_version", - "feature_segment", - *additional_select_related_args, - ) - .prefetch_related(*additional_prefetch_related_args) + feature_states = _get_feature_states_queryset( + environment, + feature_name, + additional_filters, + additional_select_related_args, + additional_prefetch_related_args, ) - if feature_name: - feature_states = feature_states.filter(feature__name__iexact=feature_name) + if environment.use_v2_feature_versioning: + return list(feature_states) # Build up a dictionary in the form # {(feature_id, feature_segment_id, identity_id): feature_state} @@ -87,3 +80,36 @@ def get_current_live_environment_feature_version( .order_by("-live_from") .first() ) + + +def _get_feature_states_queryset( + environment: "Environment", + feature_name: str = None, + additional_filters: Q = None, + additional_select_related_args: typing.Iterable[str] = None, + additional_prefetch_related_args: typing.Iterable[ + typing.Union[str, Prefetch] + ] = None, +) -> QuerySet[FeatureState]: + additional_select_related_args = additional_select_related_args or tuple() + additional_prefetch_related_args = additional_prefetch_related_args or tuple() + + queryset = ( + FeatureState.objects.get_live_feature_states( + environment=environment, additional_filters=additional_filters + ) + .select_related( + "environment", + "feature", + "feature_state_value", + "environment_feature_version", + "feature_segment", + *additional_select_related_args, + ) + .prefetch_related(*additional_prefetch_related_args) + ) + + if feature_name: + queryset = queryset.filter(feature__name__iexact=feature_name) + + return queryset diff --git a/api/tests/unit/features/conftest.py b/api/tests/unit/features/conftest.py index d4d2ff2722f2..093b450b4930 100644 --- a/api/tests/unit/features/conftest.py +++ b/api/tests/unit/features/conftest.py @@ -1,13 +1,7 @@ -import typing - import pytest -from environments.models import Environment from features.models import FeatureState -if typing.TYPE_CHECKING: - from projects.models import Project - @pytest.fixture() def feature_state_version_generator(environment, feature, request): @@ -32,10 +26,3 @@ def feature_state_version_generator(environment, feature, request): ), expected_result, ) - - -@pytest.fixture() -def environment_v2_versioning(project: "Project") -> Environment: - return Environment.objects.create( - name="v2_versioning", project=project, use_v2_feature_versioning=True - ) diff --git a/api/tests/unit/features/test_unit_features_views.py b/api/tests/unit/features/test_unit_features_views.py index a4d8df6ed025..e63ac8cb6e9d 100644 --- a/api/tests/unit/features/test_unit_features_views.py +++ b/api/tests/unit/features/test_unit_features_views.py @@ -1901,6 +1901,59 @@ def test_list_features_provides_information_on_number_of_overrides( assert response_json["results"][0]["num_identity_overrides"] == 1 +def test_list_features_provides_correct_information_on_number_of_overrides_based_on_version( + feature: Feature, + segment: Segment, + project: Project, + environment_v2_versioning: Environment, + admin_client_new: APIClient, + admin_user: FFAdminUser, +): + # Given + url = "%s?environment=%d" % ( + reverse("api-v1:projects:project-features-list", args=[project.id]), + environment_v2_versioning.id, + ) + + # let's create a new version with a segment override + version_2 = EnvironmentFeatureVersion.objects.create( + environment=environment_v2_versioning, feature=feature + ) + FeatureState.objects.create( + feature=feature, + environment=environment_v2_versioning, + feature_segment=FeatureSegment.objects.create( + feature=feature, + environment=environment_v2_versioning, + segment=segment, + ), + environment_feature_version=version_2, + ) + version_2.publish(admin_user) + + # and now let's create a new version which removes the segment override + version_3 = EnvironmentFeatureVersion.objects.create( + environment=environment_v2_versioning, feature=feature + ) + FeatureState.objects.filter( + environment_feature_version=version_3, + feature_segment__segment=segment, + ).delete() + version_3.publish(admin_user) + + # When + response = admin_client_new.get(url) + + # Then + assert response.status_code == status.HTTP_200_OK + + response_json = response.json() + assert response_json["count"] == 1 + + # The number of segment overrides in the latest version should be 0 + assert response_json["results"][0]["num_segment_overrides"] == 0 + + def test_list_features_provides_segment_overrides_for_dynamo_enabled_project( dynamo_enabled_project: Project, dynamo_enabled_project_environment_one: Environment, @@ -2915,7 +2968,7 @@ def test_feature_list_last_modified_values( Feature.objects.create(name=f"feature_{i}", project=project) # When - with django_assert_num_queries(16): # TODO: reduce this number of queries! + with django_assert_num_queries(18): # TODO: reduce this number of queries! response = staff_client.get(url) # Then diff --git a/api/tests/unit/features/versioning/test_unit_versioning_versioning_service.py b/api/tests/unit/features/versioning/test_unit_versioning_versioning_service.py index d026e291d8de..422fde91f79b 100644 --- a/api/tests/unit/features/versioning/test_unit_versioning_versioning_service.py +++ b/api/tests/unit/features/versioning/test_unit_versioning_versioning_service.py @@ -13,6 +13,7 @@ get_environment_flags_queryset, ) from projects.models import Project +from segments.models import Segment from users.models import FFAdminUser @@ -114,7 +115,7 @@ def test_get_environment_flags_v2_versioning_returns_latest_live_versions_of_fea environment_v2_versioning: Environment, feature: Feature, admin_user: FFAdminUser, -): +) -> None: # Given # a second feature with its corresponding environment feature version feature_2 = Feature.objects.create(name="feature_2", project=project) @@ -150,6 +151,47 @@ def test_get_environment_flags_v2_versioning_returns_latest_live_versions_of_fea } +def test_get_environment_flags_v2_versioning_does_not_return_removed_segment_override( + project: Project, + feature: Feature, + admin_user: FFAdminUser, + segment: Segment, + segment_featurestate: FeatureState, + environment_v2_versioning: Environment, +) -> None: + # Given + # The initial version has a segment override + initial_version = EnvironmentFeatureVersion.objects.get( + environment=environment_v2_versioning, feature=feature + ) + assert FeatureState.objects.filter( + feature=feature, + environment=environment_v2_versioning, + feature_segment__segment=segment, + environment_feature_version=initial_version, + ).exists() + + # Now let's create a new version, remove the segment override and publish the version + new_version = EnvironmentFeatureVersion.objects.create( + environment=environment_v2_versioning, feature=feature + ) + FeatureState.objects.filter( + feature=feature, + environment=environment_v2_versioning, + feature_segment__segment=segment, + environment_feature_version=new_version, + ).delete() + new_version.publish(published_by=admin_user) + + # When + environment_feature_states = get_environment_flags_list( + environment=environment_v2_versioning, + ) + + # Then + assert len(environment_feature_states) == 1 + + def test_get_current_live_environment_feature_version( environment_v2_versioning: Environment, staff_user: FFAdminUser, feature: Feature ) -> None: