diff --git a/api/segment_membership/services.py b/api/segment_membership/services.py index 16e97da5ece2..9565ca2ab5e2 100644 --- a/api/segment_membership/services.py +++ b/api/segment_membership/services.py @@ -160,11 +160,13 @@ def get_segment_members_page( *, cursor: str | None, limit: int, + q: str | None = None, ) -> list[SegmentMember]: """Return one page of identities matching `segment` in `environment`, ordered by `identifier`. Provide identifier as `cursor` to get a page after that identifier. + Provide `q` to filter to identifiers containing it (case-insensitive). """ translate_ctx = TranslateContext( evaluation_context=EvaluationContext( @@ -189,6 +191,9 @@ def get_segment_members_page( if cursor: conditions.append("i.identifier > %(cursor)s") params["cursor"] = cursor + if q: + conditions.append("positionCaseInsensitiveUTF8(i.identifier, %(q)s) > 0") + params["q"] = q conditions.append(f"({predicate})") sql = ( diff --git a/api/segments/serializers.py b/api/segments/serializers.py index e8d116db9473..7bb06eaa0f85 100644 --- a/api/segments/serializers.py +++ b/api/segments/serializers.py @@ -257,6 +257,10 @@ class SegmentMembersQuerySerializer(serializers.Serializer): # type: ignore[typ required=False, help_text="The identifier of the previous page's last row; omit for the first page.", ) + q = serializers.CharField( + required=False, + help_text="Case-insensitive substring to filter members by identifier.", + ) limit = serializers.IntegerField( required=False, default=100, diff --git a/api/segments/views.py b/api/segments/views.py index b93efa083012..b1dbafa4ed33 100644 --- a/api/segments/views.py +++ b/api/segments/views.py @@ -178,8 +178,15 @@ def associated_features(self, request, *args, **kwargs): # type: ignore[no-unty @action(detail=True, methods=["GET"], url_path="members") def members(self, request: Request, *args: Any, **kwargs: Any) -> Response: user: "FFAdminUser" = request.user # type: ignore[assignment] - segment = self.get_object() - project = segment.project + project = self.get_project() + # Fetch by pk directly rather than via get_object()/get_queryset(): the + # latter applies the list endpoint's `q` (segment-name search), which + # would filter this segment out when `q` is used here to search members. + segment = get_object_or_404( + Segment.live_objects.filter(project=project, is_system_segment=False), + pk=self.kwargs["pk"], + ) + self.check_object_permissions(request, segment) if not is_membership_enabled(project.organisation): raise NotFound() @@ -194,11 +201,12 @@ def members(self, request: Request, *args: Any, **kwargs: Any) -> Response: limit = query_serializer.validated_data["limit"] cursor = query_serializer.validated_data.get("cursor") + q = query_serializer.validated_data.get("q") with flagsmith_segment_membership_read_duration_seconds.time(): # Fetch one extra row to detect whether a further page exists, so the # last page doesn't advertise a phantom (empty) next page. members = get_segment_members_page( - segment, environment, cursor=cursor, limit=limit + 1 + segment, environment, cursor=cursor, limit=limit + 1, q=q ) has_more = len(members) > limit diff --git a/api/tests/integration/segment_membership/test_segment_members_endpoint.py b/api/tests/integration/segment_membership/test_segment_members_endpoint.py index 32ff601f09e8..61d205710a10 100644 --- a/api/tests/integration/segment_membership/test_segment_members_endpoint.py +++ b/api/tests/integration/segment_membership/test_segment_members_endpoint.py @@ -138,6 +138,37 @@ def test_get_segment_members__matching_identities__returns_members( } +@pytest.mark.clickhouse +def test_get_segment_members__q__filters_by_identifier( + segment_membership_identities: None, + admin_client: APIClient, + project: int, + environment: int, + segment: int, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features("segment_membership_inspection") + + # When + response = admin_client.get( + f"/api/v1/projects/{project}/segments/{segment}/members/?environment={environment}&q=ali" + ) + + # Then + assert response.status_code == status.HTTP_200_OK + assert response.json() == { + "results": [ + { + "identifier": "alice", + "identity_key": "alice_key", + "traits": {"foo": "bar"}, + }, + ], + "next_cursor": None, + } + + @pytest.mark.clickhouse def test_get_segment_members__more_results_than_limit__returns_next_cursor( segment_membership_identities: None, diff --git a/api/tests/unit/segment_membership/test_unit_segment_membership_services.py b/api/tests/unit/segment_membership/test_unit_segment_membership_services.py index 2f612bac038f..042a3d117aa7 100644 --- a/api/tests/unit/segment_membership/test_unit_segment_membership_services.py +++ b/api/tests/unit/segment_membership/test_unit_segment_membership_services.py @@ -254,6 +254,80 @@ def test_get_segment_members_page__limit__caps_results( ] +@pytest.mark.parametrize( + "q,expected_result", + [ + pytest.param( + "ali", + [ + SegmentMember( + identifier="alice", + identity_key="alice_key", + traits={"foo": "bar"}, + ), + ], + id="substring", + ), + pytest.param( + "ALICE", + [ + SegmentMember( + identifier="alice", + identity_key="alice_key", + traits={"foo": "bar"}, + ), + ], + id="case-insensitive", + ), + pytest.param("zzz", [], id="no-match"), + ], +) +@pytest.mark.clickhouse +def test_get_segment_members_page__q__returns_expected( + segment_membership_identities: None, + matching_segment: Segment, + environment: Environment, + q: str, + expected_result: list[SegmentMember], +) -> None: + # Given / When + members = get_segment_members_page( + matching_segment, + environment, + cursor=None, + limit=100, + q=q, + ) + + # Then + assert members == expected_result + + +@pytest.mark.clickhouse +def test_get_segment_members_page__q_matches_beyond_first_page__still_found( + segment_membership_identities: None, + matching_segment: Segment, + environment: Environment, +) -> None: + # Given / When + members = get_segment_members_page( + matching_segment, + environment, + cursor=None, + limit=1, + q="bob", + ) + + # Then + assert members == [ + SegmentMember( + identifier="bob", + identity_key="bob_key", + traits={"foo": "bar"}, + ), + ] + + def test_enqueue_membership_refresh__flag_on__enqueues_refresh( run_tasks: RunTasksFixture, mocker: MockerFixture, diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index f9d84605dc07..5c049694e96a 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -397,7 +397,7 @@ Attributes: ### `segment_membership.members.segment.skipped` Logged at `error` from: - - `api/segment_membership/services.py:180` + - `api/segment_membership/services.py:182` Attributes: - `reason`