From b970b873433208e1061a7962463058bc42a9d189 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 18:24:23 -0400 Subject: [PATCH] =?UTF-8?q?fix(server):=20drift=20#578=20=E2=80=94=20/api/?= =?UTF-8?q?me=20returns=20profile=20shape=20with=20display=5Fname=20+=20em?= =?UTF-8?q?ail?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Server-side fix for the drift audit finding (Scribe #578, parent #552). Mirrored on Android (and Flutter) but the root cause and the smallest blast-radius fix both live here. The bug: - Android MeApi.getProfile() calls GET /api/me and deserializes into MyProfileWire which has nullable display_name + email. - Server's handleGetMe was emitting the narrower UserView shape (id, username, is_admin only). - Android always saw displayName=null, email=null. The Settings → Profile screen rendered BLANK form fields for users with stored values. - Saving from the blank state submitted empty strings to PUT /api/me/profile, which interprets empty as "clear to NULL" (me_profile.go:53-65) — DESTROYING the user's saved profile. - Flutter (flutter_client/lib/api/endpoints/settings.dart:9-12) has the identical bug pattern. The fix: - handleGetMe now emits profileViewFromUser(user) — the same shape PUT /api/me/profile already returns (meProfileResp: id, username, display_name, email, is_admin). - auth.UserFromContext already returns a full dbq.User row, so no extra DB lookup needed. - Web's User TypeScript type is narrower than this response but doesn't care about the extra fields (TS structural typing). - LoginResp.User still uses UserView; login response unchanged. New test asserts the regression directly: a user with stored display_name + email sees them in /api/me. Old test updated to decode into meProfileResp and assert the nullable fields are correctly null for an unset profile. Android side needs no change — the existing wire shape already expected display_name + email; this just delivers them. --- internal/api/me.go | 24 +++++++++++++------ internal/api/me_test.go | 51 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/internal/api/me.go b/internal/api/me.go index 5372dfd1..bc33cd8f 100644 --- a/internal/api/me.go +++ b/internal/api/me.go @@ -1,7 +1,6 @@ package api import ( - "encoding/json" "errors" "net/http" @@ -9,6 +8,22 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/auth" ) +// handleGetMe returns the authenticated user's profile shape — id, +// username, display_name, email, is_admin. Mirrors the response shape +// of PUT /api/me/profile so the Android Settings → Profile screen +// (and Flutter's equivalent) can read its starting state from a GET +// before the user has done a first save. +// +// Drift #578 fix: this previously emitted the narrower UserView +// (id, username, is_admin only). Android's MeApi.getProfile() deser- +// ialised into MyProfileWire and saw display_name=null, email=null +// on every read — wiping the form fields on every Settings open. +// Saving from that blank state then submitted empty strings, which +// handleUpdateMyProfile treats as "clear to NULL" — DESTROYING the +// user's stored display_name + email. Returning the full profile +// shape here closes the loop. Web (User type narrower than this +// response) keeps working via TypeScript structural typing — the +// extra fields are ignored. func (h *handlers) handleGetMe(w http.ResponseWriter, r *http.Request) { user, ok := auth.UserFromContext(r.Context()) if !ok { @@ -18,10 +33,5 @@ func (h *handlers) handleGetMe(w http.ResponseWriter, r *http.Request) { writeErr(w, apierror.InternalMsg("missing auth context", errors.New("missing auth context"))) return } - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(UserView{ - ID: user.ID, - Username: user.Username, - IsAdmin: user.IsAdmin, - }) + writeJSON(w, http.StatusOK, profileViewFromUser(user)) } diff --git a/internal/api/me_test.go b/internal/api/me_test.go index 5f37b92e..a9515e6c 100644 --- a/internal/api/me_test.go +++ b/internal/api/me_test.go @@ -1,6 +1,7 @@ package api import ( + "context" "encoding/json" "net/http" "net/http/httptest" @@ -21,13 +22,61 @@ func TestHandleGetMe_ReturnsAuthenticatedUser(t *testing.T) { if w.Code != http.StatusOK { t.Fatalf("status = %d body = %s", w.Code, w.Body.String()) } - var got UserView + var got meProfileResp if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil { t.Fatalf("decode: %v", err) } if got.Username != "test-alice" || !got.IsAdmin { t.Errorf("user = %+v, want test-alice/admin", got) } + if got.DisplayName != nil { + t.Errorf("DisplayName = %v, want nil for a user with no profile set", *got.DisplayName) + } + if got.Email != nil { + t.Errorf("Email = %v, want nil for a user with no profile set", *got.Email) + } +} + +// Drift #578 regression guard: a user whose profile fields ARE set +// must see them in the /api/me response. Previously this handler +// emitted the narrower UserView so Android Settings → Profile saw +// display_name=null and email=null on every read, wiped the form +// fields, and submitting from that blank state destroyed the user's +// stored values via the "empty string clears to NULL" semantics of +// PUT /api/me/profile. +func TestHandleGetMe_IncludesDisplayNameAndEmail(t *testing.T) { + h, pool := testHandlers(t) + user := seedUser(t, pool, "alice", "hunter2", false) + + displayName := "Alice Liddell" + email := "alice@example.com" + updated, err := dbq.New(pool).UpdateUserProfile(context.Background(), dbq.UpdateUserProfileParams{ + ID: user.ID, + DisplayName: &displayName, + Email: &email, + }) + if err != nil { + t.Fatalf("UpdateUserProfile: %v", err) + } + + req := httptest.NewRequest(http.MethodGet, "/api/me", nil) + req = withUser(req, updated) + w := httptest.NewRecorder() + h.handleGetMe(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d body = %s", w.Code, w.Body.String()) + } + var got meProfileResp + if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil { + t.Fatalf("decode: %v", err) + } + if got.DisplayName == nil || *got.DisplayName != displayName { + t.Errorf("DisplayName = %v, want %q", got.DisplayName, displayName) + } + if got.Email == nil || *got.Email != email { + t.Errorf("Email = %v, want %q", got.Email, email) + } } func TestHandleGetMe_MissingContextReturns500(t *testing.T) {