Skip to content
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
9 changes: 6 additions & 3 deletions app/routers/users.py
Original file line number Diff line number Diff line change
@@ -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.
"""
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
]
Expand Down
18 changes: 16 additions & 2 deletions app/schemas/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
31 changes: 20 additions & 11 deletions tests/test_users_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 -----------------------------------------------------------------
Expand Down