From 67cbe085558dc413876b066afcc101e9c6863cbb Mon Sep 17 00:00:00 2001 From: Amr Gaber Date: Wed, 27 May 2026 00:09:04 -0500 Subject: [PATCH] feat(users): include email in GET /users/search response (#42) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consumer pickers (e.g. the admin owner-grant flow in hera-streamer-invitational-2026-web) currently render a raw UUID as the fallback label when a matched user has no display_name set — common for Steam-OAuth users who haven't customized their profile. This makes the picker functionally unusable for that population. Adding email to UserSearchResult lets consumer-side code render ``display_name ?? email`` and never bottom out at the UUID. The endpoint is authenticated and the search query already matches against email as an input key, so returning email doesn't expand the caller's existing knowledge surface — they already had to know (or guess) it to find the user via email-match. email is null for Steam-OAuth users pre-tos-gate, matching the existing nullability on /auth/me's UserRead. --- app/routers/users.py | 9 ++++++--- app/schemas/user.py | 18 ++++++++++++++++-- tests/test_users_search.py | 31 ++++++++++++++++++++----------- 3 files changed, 42 insertions(+), 16 deletions(-) diff --git a/app/routers/users.py b/app/routers/users.py index 872e297..60ee4e3 100644 --- a/app/routers/users.py +++ b/app/routers/users.py @@ -1,7 +1,8 @@ """User lookup endpoints. Two surfaces with intentionally different shapes: -- ``/users/search`` — substring picker for type-ahead UI; omits email. +- ``/users/search`` — substring picker for type-ahead UI; includes email as + a readable fallback when ``display_name`` is null (issue #42). - ``/users/lookup`` — bulk by-id resolver for privileged consumers; includes email. Trusts the caller to enforce authority over the looked-up users. """ @@ -68,8 +69,9 @@ async def search_users( """Type-ahead user picker for consumer apps. Matches `q` (case-insensitive substring) against display_name AND email. - Email is accepted as an input match-key for admin convenience but is - never returned — that's why the response schema has no email field. + Both are included in the response so consumer pickers can render + ``display_name ?? email`` and never fall back to raw UUID for users + who haven't customized their profile. """ q = q.strip() if not q: @@ -107,6 +109,7 @@ async def search_users( id=user.id, display_name=user.display_name, avatar_url=user.avatar_url, + email=user.email, ) for user in users ] diff --git a/app/schemas/user.py b/app/schemas/user.py index f5abb0c..ee00c82 100644 --- a/app/schemas/user.py +++ b/app/schemas/user.py @@ -42,13 +42,27 @@ class UserUpdate(schemas.BaseUserUpdate): class UserSearchResult(BaseModel): """Public projection returned by /users/search. - Deliberately omits email — the endpoint accepts email as a match-key but - never surfaces it. PII stays server-side. + Includes ``email`` so consumer-side type-ahead pickers have a readable + label fallback for users with no ``display_name`` set (especially + Steam-OAuth users who haven't customized their profile). Without + this, those rows render as the raw UUID, which makes admin pickers + in downstream apps functionally unusable for that population. + + ``email`` is ``None`` for Steam-OAuth users who haven't yet passed + through the accept-tos email-collection gate — same nullability as + ``UserRead``. + + The endpoint is authenticated, and the search query is matched + against both ``display_name`` and ``email``, so returning ``email`` + doesn't expand the caller's existing knowledge surface — they + already had to know (or guess) the email to find the user via + email-match. """ id: UUID display_name: str | None = None avatar_url: str | None = None + email: str | None = None class UserLookupResult(BaseModel): diff --git a/tests/test_users_search.py b/tests/test_users_search.py index 12c033f..8e2746e 100644 --- a/tests/test_users_search.py +++ b/tests/test_users_search.py @@ -133,31 +133,40 @@ async def test_match_is_case_insensitive(auth_client: AsyncClient, seeded: list[ assert any(row["display_name"] == "Alice" for row in body) -# --- response shape: never includes email ---------------------------------- +# --- response shape: includes email (issue #42) ---------------------------- -async def test_email_is_never_in_response(auth_client: AsyncClient, seeded: list[User]) -> None: +async def test_email_is_included_for_users_who_have_one( + auth_client: AsyncClient, seeded: list[User] +) -> None: + """Issue #42: email is now returned so consumer pickers can fall back + to a readable label (``display_name ?? email``) instead of rendering + raw UUIDs for users without a custom display_name.""" resp = await auth_client.get("/users/search?q=alice") body = resp.json() assert resp.status_code == 200 assert body, "expected matches" for row in body: - assert "email" not in row, f"email leaked into response: {row}" - assert set(row.keys()) <= {"id", "display_name", "avatar_url"} + assert set(row.keys()) == {"id", "display_name", "avatar_url", "email"} + # Alice has both fields populated; confirm the email actually rides + # through. + alice = next(r for r in body if r["display_name"] == "Alice") + assert alice["email"] == "alice@example.com" -async def test_null_email_user_is_matchable_via_display_name( +async def test_null_email_user_is_matchable_via_display_name_and_email_is_null( auth_client: AsyncClient, seeded: list[User] ) -> None: - # Post-#36, Steam users have email=NULL. Search's email-substring - # branch can't match them — but display_name still can. + """Steam-OAuth users (no email until accept-tos gate) still match via + display_name. Their row in the response carries ``email: null`` so + consumers can detect the no-email state explicitly rather than + treating a missing key as a failure.""" resp = await auth_client.get("/users/search?q=GabeStreams") body = resp.json() assert resp.status_code == 200 - assert any(row["display_name"] == "GabeStreams" for row in body) - # Email is never in the response payload regardless. - for row in body: - assert "email" not in row + gabe = next((row for row in body if row["display_name"] == "GabeStreams"), None) + assert gabe is not None + assert gabe["email"] is None # --- limit -----------------------------------------------------------------