Skip to content

fix(Admin API): Sort features by overrides #6722

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.

Sign up for GitHub

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

Draft
emyller wants to merge 4 commits into
base: main
Choose a base branch
from
+108 −7
Draft

fix(Admin API): Sort features by overrides #6722

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
194e214
Sort features by segment override
emyller Feb 16, 2026
2f3c4a6
Sort features by identity override
emyller Feb 16, 2026
36917da
Fix sorting based on segment overrides
emyller Feb 17, 2026
520db30
Fix identity URI param
emyller Feb 17, 2026
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
4 changes: 4 additions & 0 deletions api/features/serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ class FeatureQuerySerializer(serializers.Serializer): # type: ignore[type-arg]
required=False,
help_text="Integer ID of the segment to retrieve segment overrides for.",
)
identity = serializers.IntegerField(
Copy link
Contributor

@Zaimwa9 Zaimwa9 Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes ok, that's working when identities are stored in postgres, but when using edge identities, it passes an UUID.

I'll post a full comment with my findings as we'll need to have 2 implementations

Copy link
Contributor

@Zaimwa9 Zaimwa9 Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you confirm you were having integer identity.id in the screenshot you shared ?

Copy link
Contributor Author

@emyller emyller Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed. It was in my local environment.

required=False,
help_text="Integer ID of the identity to sort features with identity overrides first.",
)
is_enabled = serializers.BooleanField(
allow_null=True,
required=False,
Expand Down
28 changes: 25 additions & 3 deletions api/features/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from common.projects.permissions import VIEW_PROJECT
from django.conf import settings
from django.core.cache import caches
from django.db.models import Max, Q, QuerySet
from django.db.models import Exists, Max, OuterRef, Q, QuerySet
from django.utils import timezone
from django.utils.decorators import method_decorator
from django.views.decorators.cache import cache_page
Expand Down Expand Up @@ -56,7 +56,7 @@

from .constants import INTERSECTION, UNION
from .features_service import get_overrides_data
from .models import Feature, FeatureState
from .models import Feature, FeatureSegment, FeatureState
from .multivariate.serializers import (
FeatureMVOptionsValuesResponseSerializer,
)
Expand Down Expand Up @@ -224,7 +224,29 @@ def get_queryset(self): # type: ignore[no-untyped-def]
"-" if query_data["sort_direction"] == "DESC" else "",
query_data["sort_field"],
)
queryset = queryset.order_by(sort)
override_ordering: list[str] = []
if environment_id and (segment_id := query_data.get("segment")):
queryset = queryset.annotate(
has_segment_override=Exists(
FeatureSegment.objects.filter(
feature=OuterRef("pk"),
segment_id=segment_id,
environment_id=environment_id,
)
),
)
override_ordering.append("-has_segment_override")
if identity_id := query_data.get("identity"):
queryset = queryset.annotate(
has_identity_override=Exists(
Copy link
Contributor

@Zaimwa9 Zaimwa9 Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So here, this would make a subquery for each feature row right? I'm comparing to the segment query just above that have a unique_constraint on (feature, segment_id, environment_id), there might be a slight performance difference?
I personally think it's negligeable given that feature limit is 400 per project (~max overrides we can expect) but do you have an idea of the perf loss, how we could measaure it and at what point it could become problematic?

Copy link
Contributor Author

@emyller emyller Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TL;DR: we should be safe.

So here, this would make a subquery for each feature row right?

Yes, this renders to a subquery in SQL, and translates to a hashed subplan — which roughly means that it executes the subquery only once rather than against each row.

I'm comparing to the segment query just above that have a unique_constraint on (feature, segment_id, environment_id), there might be a slight performance difference?

In theory, as we're crossing tuples using foreign keys as anchors, the RDBMS should make the best use of each B-tree index associated to them.

I personally think it's negligeable given that feature limit is 400 per project (~max overrides we can expect) but do you have an idea of the perf loss, how we could measaure it and at what point it could become problematic?

The added effort of these conditional filters should cause minimal / negligible performance impact overall. I expect neither data frame size nor query limit to influence performance as we scale.

Here's an example output of explain analyze that illustrates an hypothetical query plan containing both subqueries.

Sort  (cost=98.11..98.14 rows=12 width=30) (actual time=0.753..0.764 rows=212 loops=1)
    Sort Key: ((hashed SubPlan 2)) DESC, ((hashed SubPlan 4)) DESC, f.name
    Sort Method: quicksort  Memory: 38kB
    Buffers: shared hit=177
    ->  Index Scan using features_feature_project_id_72859830 on features_feature f  (cost=0.42..97.89 rows=12 width=30) (actual time=0.119..0.364 rows=212 loops=1)
          Index Cond: (project_id = <project_id>)
          Buffers: shared hit=169
          SubPlan 2
            ->  Index Scan using features_featuresegment_segment_id_64602469 on features_featuresegment fs  (cost=0.29..3.63 rows=1 width=4) (actual time=0.022..0.026 rows=2 loops=1)
                  Index Cond: (segment_id = <segment_id>)
                  Filter: (environment_id = <environment_id>)
                  Buffers: shared hit=4
          SubPlan 4
            ->  Index Scan using features_featurestate_identity_id_8229f26c on features_featurestate fst  (cost=0.42..8.41 rows=8 width=4) (actual time=0.035..0.035 rows=0 loops=1)
                  Index Cond: (identity_id = <identity_id>)
                  Buffers: shared hit=3
  Planning Time: 1.122 ms
  Execution Time: 0.845 ms

FeatureState.objects.filter(
feature=OuterRef("pk"),
identity_id=identity_id,
)
),
)
override_ordering.append("-has_identity_override")
queryset = queryset.order_by(*override_ordering, sort)

if environment_id:
page = self.paginate_queryset(queryset)
Expand Down
75 changes: 75 additions & 0 deletions api/tests/unit/features/test_unit_features_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4150,6 +4150,81 @@ def test_list_features_segment_query_param_with_invalid_segment(
assert feature_data["segment_feature_state"] is None


@pytest.mark.parametrize("sort_field", ["name", "created_date"])
def test_list_features__segment_query__sorts_by_field_with_overrides_first(
admin_client_new: APIClient,
project: Project,
environment: Environment,
segment: Segment,
sort_field: str,
) -> None:
# Given
Feature.objects.create(project=project, name="feature_a")
feature_b = Feature.objects.create(project=project, name="feature_b")
feature_c = Feature.objects.create(project=project, name="feature_c")
other_environment = Environment.objects.create(
project=project, name="other_environment"
)

# feature_c has a segment override in the requested environment
FeatureSegment.objects.create(
feature=feature_c, segment=segment, environment=environment
)
# feature_b has a segment override in a different environment
FeatureSegment.objects.create(
feature=feature_b, segment=segment, environment=other_environment
)

# When
response = admin_client_new.get(
f"/api/v1/projects/{project.id}/features/"
f"?environment={environment.id}"
f"&segment={segment.id}"
f"&sort_field={sort_field}&sort_direction=ASC"
)

# Then
assert response.status_code == status.HTTP_200_OK
results = response.json()["results"]
result_names = [r["name"] for r in results]
assert result_names[0] == "feature_c"
assert result_names[1:] == ["feature_a", "feature_b"]


@pytest.mark.parametrize("sort_field", ["name", "created_date"])
def test_list_features__identity_query__sorts_by_field_with_overrides_first(
admin_client_new: APIClient,
project: Project,
environment: Environment,
identity: Identity,
sort_field: str,
) -> None:
# Given
Feature.objects.create(project=project, name="feature_a")
Feature.objects.create(project=project, name="feature_b")
feature_c = Feature.objects.create(project=project, name="feature_c")

# feature_c has an identity override
FeatureState.objects.create(
feature=feature_c, environment=environment, identity=identity
)

# When
response = admin_client_new.get(
f"/api/v1/projects/{project.id}/features/"
f"?environment={environment.id}"
f"&identity={identity.id}"
f"&sort_field={sort_field}&sort_direction=ASC"
)

# Then
assert response.status_code == status.HTTP_200_OK
results = response.json()["results"]
result_names = [r["name"] for r in results]
assert result_names[0] == "feature_c"
assert result_names[1:] == ["feature_a", "feature_b"]


def test_create_multiple_features_with_metadata_keeps_metadata_isolated(
admin_client_new: APIClient,
project: Project,
Expand Down
8 changes: 4 additions & 4 deletions frontend/web/components/pages/UserPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,9 @@ const UserPage: FC = () => {
true,
search,
sort,
getServerFilter(filter),
{ ...getServerFilter(filter), identity: id },
Copy link
Contributor

@Zaimwa9 Zaimwa9 Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we will need to use Utils.getIsEdge() to do something like ...(!isEdge ? { identity: id } : {}),, also for the next useEffect

)
}, [filter, environmentId, projectId])
}, [filter, environmentId, projectId, id])

useEffect(() => {
AppActions.getIdentity(environmentId, id)
Expand Down Expand Up @@ -141,10 +141,10 @@ const UserPage: FC = () => {
search,
sort,
pageNumber,
getServerFilter(filter),
{ ...getServerFilter(filter), identity: id },
)
},
[environmentId, projectId, filter],
[environmentId, projectId, filter, id],
)

return (
Expand Down
Loading