Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(versioning): endpoints should return latest versions #3209

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions api/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,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 @@ -192,7 +196,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 @@ -215,7 +219,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 @@ -6,10 +6,10 @@
from django.urls import reverse
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
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
Loading