diff --git a/src/app/api/auth/profile/route.ts b/src/app/api/auth/profile/route.ts index 8b9d17347..c3dc7fc9c 100644 --- a/src/app/api/auth/profile/route.ts +++ b/src/app/api/auth/profile/route.ts @@ -1,6 +1,7 @@ import { apiSuccess, apiError, + apiValidationError, getClientIp, safeJson, } from "@/lib/api-response"; @@ -19,7 +20,14 @@ export const PUT = apiHandler(async (request: NextRequest) => { const result = await applyProfileUpdate(user.id, body, getClientIp(request)); if (!result.ok) { - return apiError(result.message, result.status); + return result.issues + ? apiValidationError( + result.message, + result.issues, + result.status, + result.errorCode ? { errorCode: result.errorCode } : undefined, + ) + : apiError(result.message, result.status); } annotate({ action: { name: "auth.profile.update" } }); @@ -37,5 +45,6 @@ export const PUT = apiHandler(async (request: NextRequest) => { insurerName: result.user.insurerName, insurerIkNumber: result.user.insurerIkNumber, hasInsuranceNumber: result.user.hasInsuranceNumber, + ...(result.rejectedFields ? { rejectedFields: result.rejectedFields } : {}), }); }); diff --git a/src/app/api/user/profile/route.ts b/src/app/api/user/profile/route.ts index d3c211840..e84dd736b 100644 --- a/src/app/api/user/profile/route.ts +++ b/src/app/api/user/profile/route.ts @@ -11,6 +11,7 @@ import { apiHandler, requireAuth } from "@/lib/api-handler"; import { apiError, apiSuccess, + apiValidationError, getClientIp, safeJson, } from "@/lib/api-response"; @@ -101,7 +102,14 @@ export const PATCH = apiHandler(async (request: NextRequest) => { const result = await applyProfileUpdate(user.id, body, getClientIp(request)); if (!result.ok) { - return apiError(result.message, result.status); + return result.issues + ? apiValidationError( + result.message, + result.issues, + result.status, + result.errorCode ? { errorCode: result.errorCode } : undefined, + ) + : apiError(result.message, result.status); } annotate({ action: { name: "user.profile.update" } }); @@ -122,5 +130,6 @@ export const PATCH = apiHandler(async (request: NextRequest) => { insurerName: result.user.insurerName, insurerIkNumber: result.user.insurerIkNumber, hasInsuranceNumber: result.user.hasInsuranceNumber, + ...(result.rejectedFields ? { rejectedFields: result.rejectedFields } : {}), }); }); diff --git a/src/lib/api-response.ts b/src/lib/api-response.ts index 95168156b..c65868f26 100644 --- a/src/lib/api-response.ts +++ b/src/lib/api-response.ts @@ -175,6 +175,32 @@ export function returnAllZodIssues( ); } +/** + * Like {@link returnAllZodIssues}, but for callers that have already + * reduced a `ZodError` to a caller-composed, person-safe top-level + * `error` sentence and a pre-sanitised issue list (e.g. a shared + * write-path helper that validates outside the route handler, such as + * `applyProfileUpdate`). The multi-issue envelope (`details.issues`) is + * unchanged from `returnAllZodIssues` — this exists so the top-level + * string never has to repeat a raw Zod default message (an enum's + * `Invalid option: expected one of "A"|"B"|"C"` reaching a person's + * screen) while the machine-readable detail is still relocated, not + * dropped. `meta.errorCode` lets the web client resolve a localized + * sentence via `apiErrors.` (see `localizedApiError`). + */ +export function apiValidationError( + message: string, + issues: SanitisedZodIssue[], + status: number = 422, + meta?: ErrorMeta, +): NextResponse { + return buildJsonErrorResponse( + { data: null, error: message, details: { issues } }, + status, + meta, + ); +} + /** * `meta` is additive — clients that ignore it see the unchanged * `{ data: null, error: }` envelope. New callers can pass diff --git a/src/lib/auth/__tests__/profile-update.test.ts b/src/lib/auth/__tests__/profile-update.test.ts index 628f7b70b..cabcb2178 100644 --- a/src/lib/auth/__tests__/profile-update.test.ts +++ b/src/lib/auth/__tests__/profile-update.test.ts @@ -45,14 +45,25 @@ describe("applyProfileUpdate timezone validation", () => { expect(prisma.user.update).toHaveBeenCalledTimes(1); }); - it("rejects an unknown zone with 422", async () => { + it("rejects an unknown zone with 422, naming the field without echoing raw validator prose", async () => { const result = await applyProfileUpdate(USER_ID, { timezone: "Mars/Tharsis", }); expect(result.ok).toBe(false); if (!result.ok) { expect(result.status).toBe(422); - expect(result.message).toMatch(/Invalid IANA timezone/i); + // The specific validator reason is relocated to `issues`, not deleted. + expect(result.issues).toEqual([ + expect.objectContaining({ + path: "timezone", + message: expect.stringMatching(/Invalid IANA timezone/i), + }), + ]); + // The top-level message stays a sentence for a person: it names the + // field but never repeats the validator's own prose. + expect(result.message).toMatch(/nothing was saved/i); + expect(result.message).toMatch(/timezone/i); + expect(result.message).not.toMatch(/Invalid IANA timezone/i); } expect(prisma.user.update).not.toHaveBeenCalled(); }); @@ -126,14 +137,20 @@ describe("applyProfileUpdate insurer IK number", () => { expect("insurerIkNumber" in arg.data).toBe(false); }); - it("rejects a non-numeric IKNR with 422", async () => { + it("rejects a non-numeric IKNR with 422, relocating the reason to issues", async () => { const result = await applyProfileUpdate(USER_ID, { insurerIkNumber: "12345678X", }); expect(result.ok).toBe(false); if (!result.ok) { expect(result.status).toBe(422); - expect(result.message).toMatch(/IK number/i); + expect(result.issues).toEqual([ + expect.objectContaining({ + path: "insurerIkNumber", + message: expect.stringMatching(/IK number/i), + }), + ]); + expect(result.message).toMatch(/nothing was saved/i); } expect(prisma.user.update).not.toHaveBeenCalled(); }); @@ -150,3 +167,98 @@ describe("applyProfileUpdate insurer IK number", () => { } }); }); + +describe("applyProfileUpdate partial-field save", () => { + // Reported regression: the native client sent a lowercase `gender` + // alongside a valid `heightCm` in one PUT. Because the whole body was + // one `safeParse` and one transaction, the gender rejection took the + // height edit down with it — "editing height and gender together lost + // both". The profile schema has no cross-field dependency, so the + // fix is to salvage every field that DOES validate. + it("writes the valid sibling field when another field in the same body is invalid", async () => { + const result = await applyProfileUpdate(USER_ID, { + heightCm: 180, + gender: "male", // wrong case — the client-side half of the bug + }); + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.rejectedFields).toEqual([ + expect.objectContaining({ path: "gender" }), + ]); + } + expect(prisma.user.update).toHaveBeenCalledTimes(1); + const arg = vi.mocked(prisma.user.update).mock.calls[0][0] as { + data: Record; + }; + expect(arg.data.heightCm).toBe(180); + expect("gender" in arg.data).toBe(false); + }); + + it("omits `rejectedFields` entirely when every touched field is valid", async () => { + const result = await applyProfileUpdate(USER_ID, { + heightCm: 175, + gender: "FEMALE", + }); + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.rejectedFields).toBeUndefined(); + } + }); + + it("writes nothing and reports every rejected field when the whole body is invalid", async () => { + const result = await applyProfileUpdate(USER_ID, { + gender: "male", + locale: "fr", + }); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.status).toBe(422); + const paths = result.issues?.map((i) => i.path).sort(); + expect(paths).toEqual(["gender", "locale"]); + expect(result.message).toMatch(/nothing was saved/i); + } + expect(prisma.user.update).not.toHaveBeenCalled(); + }); +}); + +describe("applyProfileUpdate top-level error message", () => { + // The `locale`/`timeFormat`/`dateFormat` enums carry no custom Zod + // `error:` message, unlike `gender` — so an invalid value here is the + // sharpest regression check available for "does the raw Zod default + // ever reach the top-level string a person reads". Zod v4's default + // enum message is shaped like: + // Invalid option: expected one of "de"|"en" + const RAW_ZOD_ENUM_PATTERN = /expected one of/i; + + it("never lets the raw Zod enum message reach the top-level `message`", async () => { + const result = await applyProfileUpdate(USER_ID, { locale: "fr" }); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.message).not.toMatch(RAW_ZOD_ENUM_PATTERN); + expect(result.message).not.toContain('"de"|"en"'); + // Relocated, not deleted: the same detail is still available for + // whoever needs it (the iOS `details.issues` contract), just not + // in the string a person reads first. + expect(result.issues?.[0]?.path).toBe("locale"); + } + }); + + it("never lets the raw Zod enum message reach `message` for timeFormat either", async () => { + const result = await applyProfileUpdate(USER_ID, { + timeFormat: "12h", + }); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.message).not.toMatch(RAW_ZOD_ENUM_PATTERN); + expect(result.issues?.[0]?.path).toBe("timeFormat"); + } + }); + + it("carries a stable errorCode so the web client can localize the sentence", async () => { + const result = await applyProfileUpdate(USER_ID, { locale: "fr" }); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.errorCode).toBe("profile.update.nothingSaved"); + } + }); +}); diff --git a/src/lib/auth/profile-update.ts b/src/lib/auth/profile-update.ts index d62c3cb4e..03f3e226d 100644 --- a/src/lib/auth/profile-update.ts +++ b/src/lib/auth/profile-update.ts @@ -12,11 +12,17 @@ import { invalidateUserMedications, invalidateUserProfile, } from "@/lib/cache/invalidate"; +import { sanitiseZodIssues, type SanitisedZodIssue } from "@/lib/api-response"; import { z } from "zod/v4"; const extendedProfileSchema = profileSchema.extend({ displayName: z.string().min(1).max(80).nullable().optional(), - locale: z.enum(["de", "en"]).nullable().optional(), + locale: z + .enum(["de", "en"], { + error: "Locale must be de or en.", + }) + .nullable() + .optional(), timezone: z .string() .min(1) @@ -26,12 +32,58 @@ const extendedProfileSchema = profileSchema.extend({ moodReminderEnabled: z.boolean().optional(), // Hour-cycle display preference. AUTO follows the locale convention, // H12 forces AM/PM, H24 forces 24-hour. - timeFormat: z.enum(["AUTO", "H12", "H24"]).optional(), + timeFormat: z + .enum(["AUTO", "H12", "H24"], { + error: "Time format must be AUTO, H12 or H24.", + }) + .optional(), // Date-order display preference. AUTO follows the locale convention, // DMY pins day-month-year, MDY month-day-year, YMD ISO yyyy-MM-dd. - dateFormat: z.enum(["AUTO", "DMY", "MDY", "YMD"]).optional(), + dateFormat: z + .enum(["AUTO", "DMY", "MDY", "YMD"], { + error: "Date format must be AUTO, DMY, MDY or YMD.", + }) + .optional(), }); +type ExtendedProfileInput = z.infer; + +/** + * The profile schema has no cross-field `.refine()` — every field is an + * independent scalar column. That means a rejected `gender` has no + * bearing on whether `heightCm` is safe to write, so a single bad field + * no longer has to take every sibling change down with it (the report: + * "editing height and gender together lost both"). + * + * Re-validates each key present in `body` against its own field schema + * (`extendedProfileSchema.shape[key]`), independent of the others. + * Fields that pass land in `valid`; fields that fail land in `issues` + * with `path` set to the field name (a field-level `safeParse` reports + * an empty path, since it never saw the parent object). Unknown keys + * are silently dropped, matching `z.object`'s own default behaviour. + */ +function splitValidProfileFields(body: Record): { + valid: Partial; + issues: SanitisedZodIssue[]; +} { + const shape = extendedProfileSchema.shape as Record; + const valid: Record = {}; + const issues: SanitisedZodIssue[] = []; + for (const key of Object.keys(body)) { + const fieldSchema = shape[key]; + if (!fieldSchema) continue; + const result = fieldSchema.safeParse(body[key]); + if (result.success) { + valid[key] = result.data; + } else { + for (const issue of result.error.issues) { + issues.push({ path: key, code: issue.code, message: issue.message }); + } + } + } + return { valid: valid as Partial, issues }; +} + export interface ApplyProfileResult { ok: true; user: { @@ -58,18 +110,53 @@ export interface ApplyProfileResult { insurerIkNumber: string | null; hasInsuranceNumber: boolean; }; + /** + * Present only when the caller sent at least one invalid field + * alongside at least one valid one. The fields named here were NOT + * written — everything absent from this list landed. Empty/undefined + * means every touched field saved cleanly. The web client resolves + * `path` against its own field labels to name the rejected field(s); + * the message stays machine-readable, never shown verbatim. + */ + rejectedFields?: SanitisedZodIssue[]; } export interface ApplyProfileError { ok: false; status: number; + /** + * A sentence safe for a person to read — never a raw Zod message (no + * enum literals, no validator prose). Native/non-i18n callers that + * ignore `errorCode` still get something a person can act on. + */ message: string; + /** + * Stable code the web client resolves through `apiErrors.` + * for a localized sentence (see `localizedApiError`). Absent for + * non-validation failures (e.g. the 409 email-conflict below) that + * already carry a safe, specific message. + */ + errorCode?: string; + /** + * Full per-field detail, relocated (not deleted) from the top-level + * `message`. Matches the `details.issues` multi-issue envelope the + * iOS contract already expects from other write routes. + */ + issues?: SanitisedZodIssue[]; } /** * Validate and persist profile updates for `userId`. Returns either an * `ApplyProfileResult` on success or `ApplyProfileError` with a status * code so callers can shape the wire response themselves. + * + * The profile schema has no cross-field dependency, so an invalid field + * never has to block its valid siblings: a full-object parse is tried + * first (the common case), and only on failure does the update fall + * back to `splitValidProfileFields` to salvage whatever validated. If + * NOTHING in the payload validates, nothing is written — an honest + * all-or-nothing rejection naming the field that blocked it, rather + * than a raw validator string. */ export async function applyProfileUpdate( userId: string, @@ -77,15 +164,44 @@ export async function applyProfileUpdate( ipAddress?: string | null, ): Promise { const parsed = extendedProfileSchema.safeParse(body); - if (!parsed.success) { - return { - ok: false, - status: 422, - message: parsed.error.issues[0]?.message ?? "Validation error", - }; + + let data: Partial; + let rejectedFields: SanitisedZodIssue[] = []; + + if (parsed.success) { + data = parsed.data; + } else { + if (typeof body !== "object" || body === null || Array.isArray(body)) { + return { + ok: false, + status: 422, + message: "Nothing was saved. The profile update was not an object.", + errorCode: "profile.update.invalidBody", + issues: sanitiseZodIssues(parsed.error.issues), + }; + } + + const split = splitValidProfileFields(body as Record); + data = split.valid; + rejectedFields = split.issues; + + if (Object.keys(data).length === 0) { + // Every touched field failed — there is nothing to salvage. + // Name the field that blocked it so the person fixes one thing + // instead of re-entering everything. + const first = rejectedFields[0]; + return { + ok: false, + status: 422, + message: first + ? `Nothing was saved. Fix the "${first.path}" field and try again.` + : "Nothing was saved. The submitted profile data was invalid.", + errorCode: "profile.update.nothingSaved", + issues: rejectedFields, + }; + } } - const data = parsed.data; const normalizedEmail = data.email ? data.email.trim().toLowerCase() : null; if (data.email !== undefined && normalizedEmail) { @@ -218,5 +334,6 @@ export async function applyProfileUpdate( insurerIkNumber: updatedUser.insurerIkNumber, hasInsuranceNumber: updatedUser.insuranceNumberEncrypted != null, }, + ...(rejectedFields.length > 0 ? { rejectedFields } : {}), }; }