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 -----------------------------------------------------------------