mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 13:01:00 +00:00
fix(profile): stop echoing the raw save error verbatim on Settings
The Settings > Account save handler read json.error straight off the wire and rendered it as-is - the exact path a raw validator message would have reached a person's screen through, and the only place the report's symptom actually reproduces (baseline-form already goes through the errorCode-aware localizedApiError helper). It now resolves meta.errorCode into a localized sentence, the same convention used everywhere else in the app, and never touches the top-level string. A partial save (some fields landed, one didn't) gets its own message naming the rejected field via the same label already shown next to that input on this form, and reseeds the form from what the server actually persisted so a field that was silently skipped can't read as saved. describeRejectedProfileField lives in account-section-utils so both the warning and the failure path share one field-name-to-label mapping instead of duplicating it.
This commit is contained in:
1 parent
356888134d
commit
3ec3a8c6aa
4 files changed
+207
-18
No files matched your search
@@ -0,0 +1,84 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import {
|
||||
describeRejectedProfileField,
|
||||
resolveInitialTimezone,
|
||||
} from "../account-section-utils";
|
||||
|
||||
describe("describeRejectedProfileField", () => {
|
||||
const t = (key: string) => `translated:${key}`;
|
||||
|
||||
it("resolves a known field path to its existing form label", () => {
|
||||
const label = describeRejectedProfileField(
|
||||
[{ path: "gender", code: "invalid_value" }],
|
||||
t,
|
||||
);
|
||||
expect(label).toBe("translated:settings.gender");
|
||||
});
|
||||
|
||||
it("uses the first rejected field when several are present", () => {
|
||||
const label = describeRejectedProfileField(
|
||||
[
|
||||
{ path: "heightCm", code: "too_small" },
|
||||
{ path: "gender", code: "invalid_value" },
|
||||
],
|
||||
t,
|
||||
);
|
||||
expect(label).toBe("translated:settings.height");
|
||||
});
|
||||
|
||||
it("falls back to the raw path for a field this screen has no label for", () => {
|
||||
const label = describeRejectedProfileField(
|
||||
[{ path: "moodReminderEnabled", code: "invalid_type" }],
|
||||
t,
|
||||
);
|
||||
expect(label).toBe("moodReminderEnabled");
|
||||
});
|
||||
|
||||
it("returns null for an empty or missing list", () => {
|
||||
expect(describeRejectedProfileField([], t)).toBeNull();
|
||||
expect(describeRejectedProfileField(undefined, t)).toBeNull();
|
||||
});
|
||||
|
||||
// Regression guard for the underlying bug: this helper is the only
|
||||
// thing account-section renders after a rejection, so it must never
|
||||
// pass a raw Zod `message` (which could carry enum literal syntax)
|
||||
// straight through — only `path`, resolved to a pre-existing label.
|
||||
it("never echoes the issue's own `message`, even when one is present", () => {
|
||||
const label = describeRejectedProfileField(
|
||||
[
|
||||
{
|
||||
path: "gender",
|
||||
code: "invalid_value",
|
||||
message: 'Invalid option: expected one of "MALE"|"FEMALE"|"OTHER"',
|
||||
},
|
||||
],
|
||||
t,
|
||||
);
|
||||
expect(label).not.toMatch(/expected one of/i);
|
||||
expect(label).toBe("translated:settings.gender");
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveInitialTimezone", () => {
|
||||
it("keeps an explicitly stored non-default zone", () => {
|
||||
expect(resolveInitialTimezone("America/New_York", "Europe/Berlin")).toBe(
|
||||
"America/New_York",
|
||||
);
|
||||
});
|
||||
|
||||
it("auto-seeds the browser zone when the stored value is still the default", () => {
|
||||
expect(resolveInitialTimezone("Europe/Berlin", "Asia/Tokyo")).toBe(
|
||||
"Asia/Tokyo",
|
||||
);
|
||||
});
|
||||
|
||||
it("stays on the default when the browser zone is also the default", () => {
|
||||
expect(resolveInitialTimezone("Europe/Berlin", "Europe/Berlin")).toBe(
|
||||
"Europe/Berlin",
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to the default when nothing is stored", () => {
|
||||
expect(resolveInitialTimezone(null, "Europe/Berlin")).toBe("Europe/Berlin");
|
||||
});
|
||||
});
|
||||
@@ -43,8 +43,13 @@ export function resolveInitialTimezone(
|
||||
/**
|
||||
* v1.16.4 — settings status hints store the i18n KEY (+ params), not
|
||||
* the translated string: a locale switch re-renders the hint in the
|
||||
* new language instead of freezing the old-language snapshot. Server-
|
||||
* provided error text (which has no key) rides `text` verbatim.
|
||||
* new language instead of freezing the old-language snapshot. The
|
||||
* `text` variant rides a server string verbatim — reserved for
|
||||
* endpoints whose message is already a specific, hand-curated,
|
||||
* person-safe sentence (e.g. the timezone endpoint's own IANA
|
||||
* validation text). It must never carry a generic validator message;
|
||||
* the profile save path resolves `meta.errorCode` into a `key` instead
|
||||
* so nothing server-generated reaches this screen untranslated.
|
||||
*/
|
||||
export type StatusMessage =
|
||||
{ key: string; params?: Record<string, string | number> } | { text: string };
|
||||
@@ -55,3 +60,45 @@ export function statusText(
|
||||
): string {
|
||||
return "key" in msg ? t(msg.key, msg.params) : msg.text;
|
||||
}
|
||||
|
||||
/** A single rejected-field entry from `applyProfileUpdate`'s wire shape
|
||||
* (`details.issues` on failure, `data.rejectedFields` on a partial
|
||||
* success) — `path` is the schema field name, never shown verbatim. */
|
||||
export interface RejectedProfileField {
|
||||
path: string;
|
||||
code: string;
|
||||
message?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Maps a rejected field's schema `path` to the same i18n label already
|
||||
* shown next to that input on this screen. The server only ever knows
|
||||
* the field by its schema key (`heightCm`, `gender`, ...) — naming it
|
||||
* in a person-facing sentence is the client's job, using labels that
|
||||
* already exist and are already localized for this form.
|
||||
*/
|
||||
const PROFILE_FIELD_LABEL_KEYS: Record<string, string> = {
|
||||
email: "auth.email",
|
||||
heightCm: "settings.height",
|
||||
dateOfBirth: "settings.dateOfBirth",
|
||||
gender: "settings.gender",
|
||||
fullName: "settings.identity.fullName",
|
||||
insurerName: "settings.identity.insurer",
|
||||
insuranceNumber: "settings.identity.insuranceNumber",
|
||||
};
|
||||
|
||||
/**
|
||||
* Renders the first rejected field's label, falling back to its raw
|
||||
* schema key for a field this screen has no input for (defensive —
|
||||
* every field `applyProfileUpdate` accepts from this form is mapped
|
||||
* above).
|
||||
*/
|
||||
export function describeRejectedProfileField(
|
||||
fields: RejectedProfileField[] | undefined,
|
||||
t: (key: string) => string,
|
||||
): string | null {
|
||||
const first = fields?.[0];
|
||||
if (!first) return null;
|
||||
const labelKey = PROFILE_FIELD_LABEL_KEYS[first.path];
|
||||
return labelKey ? t(labelKey) : first.path;
|
||||
}
|
||||
@@ -38,8 +38,10 @@ import { CycleTrackingCard } from "@/components/settings/cycle-tracking-card";
|
||||
import { detectBrowserTimezone } from "@/lib/tz/format";
|
||||
import { apiFetchRaw } from "@/lib/api/api-fetch";
|
||||
import {
|
||||
describeRejectedProfileField,
|
||||
resolveInitialTimezone,
|
||||
statusText,
|
||||
type RejectedProfileField,
|
||||
type StatusMessage,
|
||||
} from "./account-section-utils";
|
||||
import { AvatarSection } from "./avatar-section";
|
||||
@@ -85,9 +87,9 @@ export function AccountSection() {
|
||||
});
|
||||
const [saving, setSaving] = useState(false);
|
||||
const [saveMsg, setSaveMsg] = useState<StatusMessage | null>(null);
|
||||
const [saveMsgType, setSaveMsgType] = useState<"success" | "error" | null>(
|
||||
null,
|
||||
);
|
||||
const [saveMsgType, setSaveMsgType] = useState<
|
||||
"success" | "warning" | "error" | null
|
||||
>(null);
|
||||
|
||||
// Password change dialog state.
|
||||
const [currentPassword, setCurrentPassword] = useState("");
|
||||
@@ -227,14 +229,34 @@ export function AccountSection() {
|
||||
if (user && timezone && timezone !== user.timezone) {
|
||||
storeTimezone(timezone);
|
||||
}
|
||||
setSaveMsg({ key: "settings.profileSaved" });
|
||||
setSaveMsgType("success");
|
||||
setEmail(savedProfile.email);
|
||||
setFullName(savedProfile.fullName);
|
||||
setInsurerName(savedProfile.insurerName);
|
||||
setInsuranceNumber(savedProfile.insuranceNumber);
|
||||
setProfileSeed(savedProfile);
|
||||
await refetch();
|
||||
const profileJson = (await profileRes.json().catch(() => null)) as {
|
||||
data?: { rejectedFields?: RejectedProfileField[] };
|
||||
} | null;
|
||||
const rejected = profileJson?.data?.rejectedFields;
|
||||
|
||||
if (rejected && rejected.length > 0) {
|
||||
// The write is field-independent: everything but the rejected
|
||||
// field(s) landed. Name the field instead of implying full
|
||||
// success, and reseed the form from what the server actually
|
||||
// persisted — the client's optimistic value for the rejected
|
||||
// field was never written, so it must not read as saved.
|
||||
setSaveMsg({
|
||||
key: "settings.profilePartiallySaved",
|
||||
params: { field: describeRejectedProfileField(rejected, t) ?? "" },
|
||||
});
|
||||
setSaveMsgType("warning");
|
||||
await refetch();
|
||||
setSeededUserId(null);
|
||||
} else {
|
||||
setSaveMsg({ key: "settings.profileSaved" });
|
||||
setSaveMsgType("success");
|
||||
setEmail(savedProfile.email);
|
||||
setFullName(savedProfile.fullName);
|
||||
setInsurerName(savedProfile.insurerName);
|
||||
setInsuranceNumber(savedProfile.insuranceNumber);
|
||||
setProfileSeed(savedProfile);
|
||||
await refetch();
|
||||
}
|
||||
} else if (!tzRes.ok) {
|
||||
// The dedicated tz endpoint owns the IANA validation error
|
||||
// text. Surface its message verbatim so the user sees
|
||||
@@ -252,10 +274,22 @@ export function AccountSection() {
|
||||
}
|
||||
setSaveMsgType("error");
|
||||
} else {
|
||||
const json = await profileRes.json();
|
||||
setSaveMsg(
|
||||
json.error ? { text: json.error } : { key: "settings.savingError" },
|
||||
);
|
||||
// The server's top-level `error` string is intentionally generic
|
||||
// (never a raw validator message); resolve the specific, localized
|
||||
// reason through `meta.errorCode` instead of echoing it verbatim.
|
||||
const json = (await profileRes.json().catch(() => null)) as {
|
||||
meta?: { errorCode?: string };
|
||||
details?: { issues?: RejectedProfileField[] };
|
||||
} | null;
|
||||
const errorCode = json?.meta?.errorCode;
|
||||
const field = describeRejectedProfileField(json?.details?.issues, t);
|
||||
if (errorCode === "profile.update.nothingSaved" && field) {
|
||||
setSaveMsg({ key: "settings.profileNothingSaved", params: { field } });
|
||||
} else if (errorCode) {
|
||||
setSaveMsg({ key: `apiErrors.${errorCode}` });
|
||||
} else {
|
||||
setSaveMsg({ key: "settings.savingError" });
|
||||
}
|
||||
setSaveMsgType("error");
|
||||
}
|
||||
setSaving(false);
|
||||
@@ -512,7 +546,11 @@ export function AccountSection() {
|
||||
<p
|
||||
role="alert"
|
||||
className={`text-sm ${
|
||||
saveMsgType === "success" ? "text-success" : "text-destructive"
|
||||
saveMsgType === "success"
|
||||
? "text-success"
|
||||
: saveMsgType === "warning"
|
||||
? "text-warning"
|
||||
: "text-destructive"
|
||||
}`}
|
||||
>
|
||||
{statusText(saveMsg, t)}
|
||||
|
||||
@@ -262,3 +262,23 @@ describe("applyProfileUpdate top-level error message", () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("applyProfileUpdate email conflict", () => {
|
||||
it("carries an errorCode alongside the specific 409 message", async () => {
|
||||
vi.mocked(prisma.user.findUnique).mockResolvedValue({
|
||||
id: "someone-else",
|
||||
} as never);
|
||||
|
||||
const result = await applyProfileUpdate(USER_ID, {
|
||||
email: "taken@example.com",
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) {
|
||||
expect(result.status).toBe(409);
|
||||
expect(result.errorCode).toBe("profile.update.emailInUse");
|
||||
expect(result.issues).toBeUndefined();
|
||||
}
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user