Skip to content

Commit

Permalink
fix(versioning): endpoints should return latest versions (#3209)
Browse files Browse the repository at this point in the history
  • Loading branch information
matthewelwell committed Jan 10, 2024
1 parent 6500451 commit 5e16e56
Show file tree
Hide file tree
Showing 22 changed files with 259 additions and 50 deletions.
8 changes: 6 additions & 2 deletions api/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@
from projects.tags.models import Tag
from segments.models import Condition, Segment, SegmentRule
from task_processor.task_run_method import TaskRunMethod
from tests.types import (
WithEnvironmentPermissionsCallable,
WithProjectPermissionsCallable,
)
from users.models import FFAdminUser, UserPermissionGroup

trait_key = "key1"
Expand Down Expand Up @@ -188,7 +192,7 @@ def environment(project):
@pytest.fixture()
def with_environment_permissions(
environment: Environment, staff_user: FFAdminUser
) -> typing.Callable[[list[str], int | None], UserEnvironmentPermission]:
) -> WithEnvironmentPermissionsCallable:
"""
Add environment permissions to the staff_user fixture.
Defaults to associating to the environment fixture.
Expand All @@ -211,7 +215,7 @@ def _with_environment_permissions(
@pytest.fixture()
def with_project_permissions(
project: Project, staff_user: FFAdminUser
) -> typing.Callable:
) -> WithProjectPermissionsCallable:
"""
Add project permissions to the staff_user fixture.
Defaults to associating to the project fixture.
Expand Down
15 changes: 15 additions & 0 deletions api/features/feature_segments/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,17 @@
from rest_framework.generics import get_object_or_404
from rest_framework.response import Response

from environments.models import Environment
from features.feature_segments.serializers import (
FeatureSegmentChangePrioritiesSerializer,
FeatureSegmentCreateSerializer,
FeatureSegmentListSerializer,
FeatureSegmentQuerySerializer,
)
from features.models import FeatureSegment
from features.versioning.versioning_service import (
get_current_live_environment_feature_version,
)
from projects.permissions import VIEW_PROJECT

from .permissions import FeatureSegmentPermissions
Expand Down Expand Up @@ -53,6 +57,17 @@ def get_queryset(self):
data=self.request.query_params
)
filter_serializer.is_valid(raise_exception=True)

environment_id = filter_serializer.validated_data["environment"]
environment = Environment.objects.get(id=environment_id)
if environment.use_v2_feature_versioning:
queryset = queryset.filter(
environment_feature_version=get_current_live_environment_feature_version(
environment_id=environment_id,
feature_id=filter_serializer.validated_data["feature"],
)
)

return queryset.select_related("segment").filter(**filter_serializer.data)

return queryset
Expand Down
19 changes: 18 additions & 1 deletion api/features/versioning/versioning_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from django.utils import timezone

from features.models import FeatureState
from features.versioning.models import EnvironmentFeatureVersion

if typing.TYPE_CHECKING:
from environments.models import Environment
Expand Down Expand Up @@ -50,6 +51,7 @@ def get_environment_flags_list(
"feature",
"feature_state_value",
"environment_feature_version",
"feature_segment",
*additional_select_related_args,
)
.prefetch_related(*additional_prefetch_related_args)
Expand All @@ -63,7 +65,7 @@ def get_environment_flags_list(
for feature_state in feature_states:
key = (
feature_state.feature_id,
feature_state.feature_segment_id,
getattr(feature_state.feature_segment, "segment_id", None),
feature_state.identity_id,
)
current_feature_state = feature_states_dict.get(key)
Expand All @@ -73,6 +75,21 @@ def get_environment_flags_list(
return list(feature_states_dict.values())


def get_current_live_environment_feature_version(
environment_id: int, feature_id: int
) -> EnvironmentFeatureVersion | None:
return (
EnvironmentFeatureVersion.objects.filter(
environment_id=environment_id,
feature_id=feature_id,
published_at__isnull=False,
live_from__lte=timezone.now(),
)
.order_by("-live_from")
.first()
)


def _build_environment_flags_qs_filter(
environment: "Environment", feature_name: str = None, additional_filters: Q = None
) -> Q:
Expand Down
2 changes: 1 addition & 1 deletion api/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use_parentheses = true
multi_line_output = 3
include_trailing_comma = true
line_length = 79
known_first_party=['analytics','app','custom_auth','environments','integrations','organisations','projects','segments','users','webhooks','api','audit','e2etests','features','permissions','util']
known_first_party=['analytics','app','custom_auth','environments','integrations','organisations','projects','segments','tests','users','webhooks','api','audit','e2etests','features','permissions','util']
known_third_party=['_pytest','apiclient','app_analytics','axes','chargebee','core','coreapi','corsheaders','dj_database_url','django','django_lifecycle','djoser','drf_writable_nested','drf_yasg','environs','google','influxdb_client','ordered_model','pyotp','pytest','pytz','requests','responses','rest_framework','rest_framework_nested','rest_framework_recursive','sentry_sdk','shortuuid','simple_history','six','telemetry','tests','trench','whitenoise']
skip = ['migrations','.venv','.direnv']

Expand Down
2 changes: 1 addition & 1 deletion api/tests/integration/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
from pytest_django.fixtures import SettingsWrapper
from rest_framework import status
from rest_framework.test import APIClient
from tests.integration.helpers import create_mv_option_with_api

from app.utils import create_hash
from organisations.models import Organisation
from tests.integration.helpers import create_mv_option_with_api


@pytest.fixture()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from rest_framework import status
from rest_framework.exceptions import NotFound
from rest_framework.test import APIClient

from tests.integration.helpers import create_mv_option_with_api


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@
import pytest
from django.urls import reverse
from rest_framework import status

from features.feature_types import MULTIVARIATE
from tests.integration.helpers import (
create_feature_with_api,
create_mv_option_with_api,
)

from features.feature_types import MULTIVARIATE

variant_1_value = "variant-1-value"
variant_2_value = "variant-2-value"
control_value = "control"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from pytest_lazyfixture import lazy_fixture
from rest_framework import status
from rest_framework.test import APIClient

from tests.integration.helpers import (
get_env_feature_states_list_with_api,
get_feature_segement_list_with_api,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import pytest
from django.urls import reverse
from rest_framework import status

from tests.test_helpers import generate_segment_data

from .types import (
Expand Down
11 changes: 11 additions & 0 deletions api/tests/types.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
from typing import Callable

from environments.permissions.models import UserEnvironmentPermission
from projects.models import Project, UserProjectPermission

WithProjectPermissionsCallable = Callable[
[list[str], Project | None], UserProjectPermission
]
WithEnvironmentPermissionsCallable = Callable[
[list[str], Project | None], UserEnvironmentPermission
]
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
from typing import Callable

from django.urls import reverse
from pytest_mock import MockerFixture
from rest_framework import status
Expand All @@ -16,6 +14,7 @@
)
from environments.permissions.permissions import NestedEnvironmentPermissions
from features.models import Feature
from tests.types import WithEnvironmentPermissionsCallable


def test_edge_identity_view_set_get_permissions():
Expand Down Expand Up @@ -97,7 +96,7 @@ def test_edge_identity_viewset_returns_404_for_invalid_environment_key(admin_cli

def test_get_edge_identity_overrides_for_a_feature(
staff_client: APIClient,
with_environment_permissions: Callable,
with_environment_permissions: WithEnvironmentPermissionsCallable,
mocker: MockerFixture,
feature: Feature,
environment: Environment,
Expand Down Expand Up @@ -166,7 +165,7 @@ def test_get_edge_identity_overrides_for_a_feature(

def test_user_without_manage_identities_permission_cannot_get_edge_identity_overrides_for_a_feature(
staff_client: APIClient,
with_environment_permissions: Callable,
with_environment_permissions: WithEnvironmentPermissionsCallable,
feature: Feature,
environment: Environment,
) -> None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
import pytest
from django.urls import reverse
from rest_framework import status
from tests.unit.environments.helpers import get_environment_user_client

from environments.permissions.constants import (
UPDATE_FEATURE_STATE,
VIEW_ENVIRONMENT,
)
from tests.unit.environments.helpers import get_environment_user_client


def test_user_without_update_feature_state_permission_cannot_create_identity_feature_state(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
import pytest
from django.urls import reverse
from rest_framework import status
from tests.unit.environments.helpers import get_environment_user_client

from environments.permissions.constants import (
UPDATE_FEATURE_STATE,
VIEW_ENVIRONMENT,
)
from tests.unit.environments.helpers import get_environment_user_client


def test_user_without_update_feature_state_permission_cannot_update_feature_state(
Expand Down
3 changes: 2 additions & 1 deletion api/tests/unit/environments/test_unit_environments_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
)
from projects.permissions import CREATE_ENVIRONMENT, VIEW_PROJECT
from segments.models import Condition, SegmentRule
from tests.types import WithEnvironmentPermissionsCallable
from users.models import FFAdminUser
from util.tests import Helper

Expand Down Expand Up @@ -605,7 +606,7 @@ def test_create_environment_without_required_metadata_returns_400(
def test_view_environment_with_staff__query_count_is_expected(
staff_client: APIClient,
environment: Environment,
with_environment_permissions: Callable[[list[str], int], None],
with_environment_permissions: WithEnvironmentPermissionsCallable,
project: Project,
django_assert_num_queries: Callable[[int], None],
environment_metadata_a: Metadata,
Expand Down

0 comments on commit 5e16e56

Please sign in to comment.