peter/siftlode
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(notifications): a notice keeps what to say and says it in the reader's language when shown

Browse Source
This commit is contained in:
peter
2026-10-29 22:37:14 +01:00
parent 6986eb1937
commit 4ef772d37b
125 changed files with 973 additions and 674 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/modules/users/AccountPanel.tsx
+5 -5
View File
@@ -7,7 +7,7 @@ import { LoadingState, StateMessage } from "../../components/ui/QueryState";
import { api } from "../../lib/api";
import { displayName } from "../../lib/api/account";
import { qk } from "../../lib/queryKeys";
import { notify } from "../../lib/notifications";
import { msg, notify } from "../../lib/notifications";
import { notifyActionError } from "../../lib/actionErrors";
import { formatDateTime } from "../../lib/format";
import { invalidateUserAccess } from "../../lib/adminUsers";
@@ -73,19 +73,19 @@ export default function AccountPanel({
const issue = useMutation({
mutationFn: () => api.createSetPasswordLink(user.id),
onSuccess: (r) => setLink({ url: r.url, expires_at: r.expires_at }),
onError: (err) => notifyActionError(err, t("users.account.linkFailed")),
onError: (err) => notifyActionError(err, msg("users.account.linkFailed")),
});
const resend = useMutation({
mutationFn: () => api.resendUserVerification(user.id),
onSuccess: () => notify({ level: "success", message: t("users.account.resendSent") }),
onError: (err) => notifyActionError(err, t("users.pendingAccess.resendFailed")),
onSuccess: () => notify({ level: "success", message: msg("users.account.resendSent") }),
onError: (err) => notifyActionError(err, msg("users.pendingAccess.resendFailed")),
});
const activate = useMutation({
mutationFn: () => api.activateUser(user.id),
onSuccess: () => invalidateUserAccess(qc, user.id),
onError: (err) => notifyActionError(err, t("users.pendingAccess.activateFailed")),
onError: (err) => notifyActionError(err, msg("users.pendingAccess.activateFailed")),
});
// Via the shared helper, whose synchronous `execCommand` path is what makes copying work on a
frontend/src/modules/users/AdminUsers.dom.test.tsx
+6 -3
View File
@@ -26,7 +26,7 @@ vi.mock("../../lib/useMe", () => ({
}));
vi.mock("../../components/ui/confirmContext", () => ({ useConfirm: () => async () => true }));
const { notify } = vi.hoisted(() => ({
notify: vi.fn((_n: { level: string; message: string }) => 0),
notify: vi.fn((_n: { level: string; title?: NotifText; message: NotifText }) => 0),
}));
vi.mock("../../lib/notifications", async (importOriginal) => ({
...(await importOriginal<typeof import("../../lib/notifications")>()),
@@ -39,6 +39,7 @@ import { PageToolbarSlot } from "../../components/ui/pageToolbarSlot";
import { api, type AdminUserRow } from "../../lib/api";
import { relativeTime } from "../../lib/format";
import i18n from "../../i18n";
import { msg, sayNotif, type NotifText } from "../../lib/notifications";
const row = {
id: 2,
@@ -175,10 +176,12 @@ describe("a user action that fails", () => {
await waitFor(() =>
expect(notify).toHaveBeenCalledWith({
level: "error",
title: "users.roles.failed", // what failed…
message: i18n.t("errors.notFound.user"), // …and why: the request layer localizes a structured detail
title: msg("users.roles.failed"), // what failed…
message: { detail: { code: "not_found", reason: "user" } }, // …and why: the server's refusal
}),
);
// …said in the reader's language when it is shown.
expect(sayNotif(notify.mock.lastCall![0].message)).toBe(i18n.t("errors.notFound.user"));
await waitFor(() => expect(users).toHaveBeenCalledTimes(2));
});
frontend/src/modules/users/AdminUsers.tsx
+33 -24
View File
@@ -24,7 +24,7 @@ import {
type PendingAccessRow,
} from "../../lib/api";
import { displayName } from "../../lib/api/account";
import { notify } from "../../lib/notifications";
import { msg, type NotifText, notify } from "../../lib/notifications";
import { formatDateTime, relativeTime } from "../../lib/format";
import { invalidateUserAccess } from "../../lib/adminUsers";
import { ADMIN_USERS_TAB_KEY, USERS_TAB_URL_KEY, usersTabParams } from "../../lib/adminUsersTab";
@@ -134,7 +134,7 @@ function UsersRoles({ me }: { me: Me }) {
// A failed user action: said once (notifyActionError skips what `req` already put in its dialog),
// and the list refetched — a user deleted in another tab would otherwise stay listed, every click
// on the row doing nothing.
const onGoneOrFailed = (e: unknown, fallback: string) => {
const onGoneOrFailed = (e: unknown, fallback: NotifText) => {
notifyActionError(e, fallback);
qc.invalidateQueries({ queryKey: qk.adminUsers() });
};
@@ -144,12 +144,12 @@ function UsersRoles({ me }: { me: Me }) {
onMutate: (vars) => startPending(vars.id),
onSuccess: (_d, vars) => {
qc.invalidateQueries({ queryKey: qk.adminUsers() });
notify({ level: "success", message: t("users.roles.updated", { email: vars.email }) });
notify({ level: "success", message: msg("users.roles.updated", { email: vars.email }) });
},
onSettled: (_d, _e, vars) => endPending(vars.id),
// Guards (last admin / self) surface via the global error dialog with the server's detail; what
// it does not show (a user deleted meanwhile, a 404) is said here — see onGoneOrFailed.
onError: (e) => onGoneOrFailed(e, t("users.roles.failed")),
onError: (e) => onGoneOrFailed(e, msg("users.roles.failed")),
});
const suspend = useMutation({
@@ -160,24 +160,24 @@ function UsersRoles({ me }: { me: Me }) {
qc.invalidateQueries({ queryKey: qk.adminUsers() });
notify({
level: "success",
message: t(vars.suspended ? "users.suspend.done" : "users.suspend.undone", {
message: msg(vars.suspended ? "users.suspend.done" : "users.suspend.undone", {
email: vars.email,
}),
});
},
onSettled: (_d, _e, vars) => endPending(vars.id),
// Guards (self / last admin) surface via the global error dialog; the rest, as above.
onError: (e) => onGoneOrFailed(e, t("users.suspend.failed")),
onError: (e) => onGoneOrFailed(e, msg("users.suspend.failed")),
});
const del = useMutation({
mutationFn: ({ id }: { id: number; email: string }) => api.adminDeleteUser(id),
onMutate: (vars) => startPending(vars.id),
onSuccess: (_d, vars) => {
qc.invalidateQueries({ queryKey: qk.adminUsers() });
notify({ level: "success", message: t("users.delete.done", { email: vars.email }) });
notify({ level: "success", message: msg("users.delete.done", { email: vars.email }) });
},
onSettled: (_d, _e, vars) => endPending(vars.id),
onError: (e) => onGoneOrFailed(e, t("users.delete.failed")),
onError: (e) => onGoneOrFailed(e, msg("users.delete.failed")),
});
const onToggle = async (u: AdminUserRow) => {
@@ -367,20 +367,23 @@ function PendingAccess() {
mutationFn: (u: PendingAccessRow) => api.activateUser(u.id),
onSuccess: (_r, u) => {
refresh();
notify({ level: "success", message: t("users.pendingAccess.activated", { email: u.email }) });
notify({
level: "success",
message: msg("users.pendingAccess.activated", { email: u.email }),
});
},
onError: (err) => notifyActionError(err, t("users.pendingAccess.activateFailed")),
onError: (err) => notifyActionError(err, msg("users.pendingAccess.activateFailed")),
});
const resend = useMutation({
mutationFn: (u: PendingAccessRow) => api.resendUserVerification(u.id),
onSuccess: (_r, u) => {
refresh();
notify({ level: "success", message: t("users.pendingAccess.resent", { email: u.email }) });
notify({ level: "success", message: msg("users.pendingAccess.resent", { email: u.email }) });
},
// The public resend is deliberately silent; this one is not, and the server's reason says WHY
// (no working SMTP, a refused send) — a success toast for a mail never sent would be the lie.
onError: (err) => notifyActionError(err, t("users.pendingAccess.resendFailed")),
onError: (err) => notifyActionError(err, msg("users.pendingAccess.resendFailed")),
});
const list = rows.data ?? [];
@@ -543,17 +546,20 @@ function AdminInvites() {
mutationFn: ({ id }: { id: number; email: string }) => api.approveInvite(id),
onSuccess: (_d, vars) => {
refresh();
notify({ level: "success", message: t("settings.invites.approved", { email: vars.email }) });
notify({
level: "success",
message: msg("settings.invites.approved", { email: vars.email }),
});
},
onError: (e) => notifyActionError(e, t("settings.invites.approveFailed")),
onError: (e) => notifyActionError(e, msg("settings.invites.approveFailed")),
});
const deny = useMutation({
mutationFn: (id: number) => api.denyInvite(id),
onSuccess: (_d, _id) => {
refresh();
notify({ level: "success", message: t("settings.invites.denied") });
notify({ level: "success", message: msg("settings.invites.denied") });
},
onError: (e) => notifyActionError(e, t("settings.invites.denyFailed")),
onError: (e) => notifyActionError(e, msg("settings.invites.denyFailed")),
});
const onDeny = async (i: { id: number; email: string }) => {
const ok = await confirm({
@@ -571,15 +577,15 @@ function AdminInvites() {
// Removing the row is not always a revoke: is_allowed falls back to the env allow-lists, so
// for an address on those the person keeps access. Saying "removed" alone would mislead.
if (d.still_allowed_by_env)
notify({ level: "warning", message: t("settings.invites.deletedStillAllowed") });
else notify({ level: "success", message: t("settings.invites.deleted") });
notify({ level: "warning", message: msg("settings.invites.deletedStillAllowed") });
else notify({ level: "success", message: msg("settings.invites.deleted") });
},
onError: (e) => {
// A 409 means the list was stale — someone registered on that address since it loaded. Say
// what to do instead of a generic failure, and refetch so the row re-renders as read-only.
const conflict = e instanceof HttpError && e.status === 409;
if (conflict) notifyKnownFailure(e, t("settings.invites.deleteHasAccount"));
else notifyActionError(e, t("settings.invites.deleteFailed"));
if (conflict) notifyKnownFailure(e, msg("settings.invites.deleteHasAccount"));
else notifyActionError(e, msg("settings.invites.deleteFailed"));
if (conflict) refresh();
},
});
@@ -607,9 +613,12 @@ function AdminInvites() {
mutationFn: (v: { inv: Invite; lang: LangCode }) => api.sendInvitation(v.inv.id, v.lang),
onSuccess: (_r, v) => {
refresh();
notify({ level: "success", message: t("settings.invites.invited", { email: v.inv.email }) });
notify({
level: "success",
message: msg("settings.invites.invited", { email: v.inv.email }),
});
},
onError: (err) => notifyActionError(err, t("settings.invites.inviteFailed")),
onError: (err) => notifyActionError(err, msg("settings.invites.inviteFailed")),
});
// The one recipient the app has no language signal about: the admin typed this address in, so
// nobody ever told us how to write to its owner. The dialog is where that gets decided — and it
@@ -648,9 +657,9 @@ function AdminInvites() {
onSuccess: (_d, email) => {
setNewEmail("");
refresh();
notify({ level: "success", message: t("settings.invites.addedToWhitelist", { email }) });
notify({ level: "success", message: msg("settings.invites.addedToWhitelist", { email }) });
},
onError: (e) => notifyActionError(e, t("settings.invites.addFailed")),
onError: (e) => notifyActionError(e, msg("settings.invites.addFailed")),
});
const list = invites.data ?? [];
frontend/src/modules/users/accountPanel.dom.test.tsx
+20 -10
View File
@@ -13,6 +13,7 @@
import { describe, it, expect, afterEach, beforeEach, vi } from "vitest";
import { render, screen, cleanup, waitFor, fireEvent } from "@testing-library/react";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import type { NotifText } from "../../lib/notifications";
vi.mock("react-i18next", async (importOriginal) => {
const actual = await importOriginal<typeof import("react-i18next")>();
@@ -51,6 +52,8 @@ vi.mock("../../lib/api", async (importOriginal) => {
});
const { HttpError } = await import("../../lib/api");
const { msg, sayNotif } = await import("../../lib/notifications");
const i18n = (await import("../../i18n")).default;
const AccountPanel = (await import("./AccountPanel")).default;
const user = {
@@ -90,7 +93,7 @@ afterEach(cleanup);
// it), else the specific sentence (the no-mail case).
const errorMessages = () =>
notifyMock.mock.calls
.map(([a]) => a as { level?: string; title?: string; message?: string })
.map(([a]) => a as { level?: string; title?: NotifText; message?: NotifText })
.filter((a) => a.level === "error")
.map((a) => a.title ?? a.message);
@@ -100,13 +103,13 @@ describe("the account panel reports a refused recovery action", () => {
await clickAction("users.account.setPassword");
await waitFor(() => expect(errorMessages()).toContain("users.account.linkFailed"));
await waitFor(() => expect(errorMessages()).toContainEqual(msg("users.account.linkFailed")));
// …and no link block was rendered, so the toast is the only thing that happened.
expect(screen.queryByText("users.account.copy")).toBeNull();
});
it("names the no-mail case, so a no-SMTP instance does not read as a bug", async () => {
// `req` has already localized the refusal into `detail` (lib/api/core); the toast carries it.
// The toast carries the refusal itself, said in the reader's language when it is shown.
apiMock.resendUserVerification.mockRejectedValue(
new HttpError(409, "errors.admin.email_off", { code: "resend", reason: "email_off" }),
);
@@ -116,11 +119,14 @@ describe("the account panel reports a refused recovery action", () => {
await waitFor(() =>
expect(notifyMock).toHaveBeenCalledWith(
expect.objectContaining({
title: "users.pendingAccess.resendFailed",
message: "errors.admin.email_off",
title: msg("users.pendingAccess.resendFailed"),
message: { detail: { code: "resend", reason: "email_off" } },
}),
),
);
// …which says the no-mail sentence, not a generic failure.
const [shown] = notifyMock.mock.lastCall as [{ message: NotifText }];
expect(sayNotif(shown.message)).toBe(i18n.t("errors.admin.email_off"));
});
it("reports a genuine send failure as a failure, not as a sent mail", async () => {
@@ -130,11 +136,13 @@ describe("the account panel reports a refused recovery action", () => {
await clickAction("users.account.resend");
await waitFor(() => expect(errorMessages()).toContain("users.pendingAccess.resendFailed"));
// The success toast must NOT also fire — that is the exact lie the backend guards against.
expect(notifyMock.mock.calls.map(([a]) => (a as { message?: string }).message)).not.toContain(
"users.account.resendSent",
await waitFor(() =>
expect(errorMessages()).toContainEqual(msg("users.pendingAccess.resendFailed")),
);
// The success toast must NOT also fire — that is the exact lie the backend guards against.
expect(
notifyMock.mock.calls.map(([a]) => (a as { message?: NotifText }).message),
).not.toContainEqual(msg("users.account.resendSent"));
});
it("reports a refused activation", async () => {
@@ -144,6 +152,8 @@ describe("the account panel reports a refused recovery action", () => {
await clickAction("users.account.activate");
await waitFor(() => expect(errorMessages()).toContain("users.pendingAccess.activateFailed"));
await waitFor(() =>
expect(errorMessages()).toContainEqual(msg("users.pendingAccess.activateFailed")),
);
});
});
frontend/src/modules/users/adminInvites.dom.test.tsx
+12 -6
View File
@@ -81,6 +81,8 @@ vi.mock("../../lib/api", async (importOriginal) => {
import { HttpError } from "../../lib/api";
import AdminUsers from "./AdminUsers";
import { msg, sayNotif, type NotifText } from "../../lib/notifications";
import i18n from "../../i18n";
const invite = (over: Partial<Record<string, unknown>> = {}) => ({
id: 1,
@@ -207,7 +209,7 @@ describe("Access requests — the delete affordance", () => {
await waitFor(() =>
expect(notifyMock).toHaveBeenCalledWith({
level: "warning",
message: "settings.invites.deletedStillAllowed",
message: msg("settings.invites.deletedStillAllowed"),
}),
);
});
@@ -223,7 +225,7 @@ describe("Access requests — the delete affordance", () => {
await waitFor(() =>
expect(notifyMock).toHaveBeenCalledWith({
level: "error",
message: "settings.invites.deleteHasAccount",
message: msg("settings.invites.deleteHasAccount"),
}),
);
});
@@ -261,7 +263,9 @@ describe("inviting someone the admin whitelisted", () => {
// Falls back to the admin's own UI language: nobody ever told us anything about this mailbox.
await waitFor(() => expect(inviteMock).toHaveBeenCalledWith(1, "en"));
expect(notifyMock).toHaveBeenCalledWith(
expect.objectContaining({ message: "settings.invites.invited:asker@example.com" }),
expect.objectContaining({
message: msg("settings.invites.invited", { email: "asker@example.com" }),
}),
);
});
@@ -356,14 +360,16 @@ describe("inviting someone the admin whitelisted", () => {
fireEvent.click(screen.getByLabelText("settings.invites.inviteLabel:asker@example.com"));
// A button the admin pressed must not answer with a success toast for a mail nobody received.
// `req` has already localized the refusal into `detail` (lib/api/core); the toast carries it.
// The toast carries the refusal itself, said in the reader's language when it is shown.
await waitFor(() =>
expect(notifyMock).toHaveBeenCalledWith(
expect.objectContaining({
title: "settings.invites.inviteFailed",
message: "errors.admin.email_off",
title: msg("settings.invites.inviteFailed"),
message: { detail: { code: "invite_send", reason: "email_off" } },
}),
),
);
const [shown] = notifyMock.mock.lastCall as [{ message: NotifText }];
expect(sayNotif(shown.message)).toBe(i18n.t("errors.admin.email_off"));
});
});
frontend/src/modules/users/adminPendingAccess.dom.test.tsx
+11 -5
View File
@@ -69,6 +69,8 @@ vi.mock("../../lib/api", async (importOriginal) => {
});
import AdminUsers from "./AdminUsers";
import { msg, sayNotif, type NotifText } from "../../lib/notifications";
import i18n from "../../i18n";
/** A row as /api/admin/pending-access serves it. */
const row = (over: Record<string, unknown> = {}) => ({
@@ -113,7 +115,9 @@ describe("the stuck sign-ups section", () => {
await waitFor(() => expect(activateMock).toHaveBeenCalledWith(7));
expect(notifyMock).toHaveBeenCalledWith(
expect.objectContaining({ message: "users.pendingAccess.activated:later@example.com" }),
expect.objectContaining({
message: msg("users.pendingAccess.activated", { email: "later@example.com" }),
}),
);
});
@@ -165,16 +169,18 @@ describe("the stuck sign-ups section", () => {
fireEvent.click(await screen.findByLabelText("users.pendingAccess.resend"));
// Mail is fail-soft everywhere else, which is what left the 0.78.5 sign-ups with nothing to fall
// back on. A button the admin pressed must not answer with a success toast. `req` has already
// localized the refusal into `detail` (lib/api/core); the toast carries it.
// back on. A button the admin pressed must not answer with a success toast. The toast carries the
// refusal itself, said in the reader's language when it is shown.
await waitFor(() =>
expect(notifyMock).toHaveBeenCalledWith(
expect.objectContaining({
title: "users.pendingAccess.resendFailed",
message: "errors.admin.email_off",
title: msg("users.pendingAccess.resendFailed"),
message: { detail: { code: "resend", reason: "email_off" } },
}),
),
);
const [shown] = notifyMock.mock.lastCall as [{ message: NotifText }];
expect(sayNotif(shown.message)).toBe(i18n.t("errors.admin.email_off"));
});
it("renders nothing at all when nobody is stuck", async () => {