From 9c823ed4d907a0f048d9d2546d8ad3001f647822 Mon Sep 17 00:00:00 2001 From: Ibrahim Roshdy <22573766+ibrahimroshdy@users.noreply.github.com> Date: Fri, 16 Oct 2026 17:56:36 +0300 Subject: [PATCH] fix(reports): lock email-only options when mail isn't set up (#141) Print inventory's Email it to me and Send it again are disabled, with a lock and the reason under them, when the server has no mail or the account has no address. Owners and admins get Set up email (the docs' Mail page); others are told to ask the owner. The server refuses email: true and report schedules with 409 mail_not_configured or no_email_address instead of accepting them and dropping the mail (D254). Closes #141 Signed-off-by: Ibrahim Roshdy <22573766+ibrahimroshdy@users.noreply.github.com> --- .../src/content/docs/admin/notifications.md | 7 +- .../src/content/docs/users/everyday-use.md | 3 +- apps/server/src/reports/reports.test.ts | 45 ++++++- apps/server/src/reports/routes.ts | 27 +++-- apps/server/src/reports/schedules.ts | 27 ++++- apps/web/src/api/inventory/mock/reports.ts | 12 ++ .../src/components/reports/print-sheet.tsx | 114 ++++++++++++++++-- apps/web/src/components/setup-code-help.tsx | 4 + apps/web/src/locales/ar/messages.po | 12 ++ apps/web/src/locales/de/messages.po | 12 ++ apps/web/src/locales/en/messages.po | 12 ++ apps/web/src/locales/fr/messages.po | 12 ++ apps/web/src/locales/it/messages.po | 12 ++ apps/web/src/test/screens/reports.test.tsx | 96 ++++++++++++++- docs/specs/2026-09-25-kept-product-design.md | 1 + packages/shared/src/errors.test.ts | 4 +- packages/shared/src/errors.ts | 12 +- 17 files changed, 385 insertions(+), 27 deletions(-) diff --git a/apps/docs/src/content/docs/admin/notifications.md b/apps/docs/src/content/docs/admin/notifications.md index 33423e82..e6c0a7c6 100644 --- a/apps/docs/src/content/docs/admin/notifications.md +++ b/apps/docs/src/content/docs/admin/notifications.md @@ -26,8 +26,11 @@ pass through the URL. Mail is only logged as due: one warning per mail, `mail not sent: no mail transport is configured`, naming the kind of mail and never the address, link or token. **Admin → Status** shows **Mail: Not configured** and the notice **Mail isn't configured**. Without mail there are -no sign-in links, password resets, email invites or emailed alerts. Link invites and passwords -work without it, and reminders still reach people by push and in the app. +no sign-in links, password resets, email invites, emailed alerts or emailed reports. Link +invites and passwords work without it, and reminders still reach people by push and in the app. +**Email it to me** and **Send it again** in Print inventory are locked, and owners and admins +get a **Set up email** link to this page; the API refuses `email: true` and report schedules with +409 `mail_not_configured`. When mail is configured, an account whose address isn't verified can't add a passkey or an authenticator app until it is. diff --git a/apps/docs/src/content/docs/users/everyday-use.md b/apps/docs/src/content/docs/users/everyday-use.md index bd4995ad..ff5f8321 100644 --- a/apps/docs/src/content/docs/users/everyday-use.md +++ b/apps/docs/src/content/docs/users/everyday-use.md @@ -299,7 +299,8 @@ To hand someone a list on paper instead, print it (below). see money. Anyone who can see the location can make one. **Email it to me** also mails the PDF when it's ready, and **Send it again** mails the whole location every Monday or on the 1st of the month; Settings → Me → Notifications lists these and stops them. A report over 10 MB isn't - attached: print it from Kept instead. + attached: print it from Kept instead. Both are locked, with the reason under them, when the + server has no mail set up or your account has no email address. - **Insurance report** (where you can see money): every thing with its photo, brand, model, serial, purchase date, price, current value and receipts, with totals per place and per currency, as a PDF and a CSV. **Also total in** adds one converted total when your exchange rates cover diff --git a/apps/server/src/reports/reports.test.ts b/apps/server/src/reports/reports.test.ts index 5ce524df..c416aa11 100644 --- a/apps/server/src/reports/reports.test.ts +++ b/apps/server/src/reports/reports.test.ts @@ -145,7 +145,7 @@ beforeAll(async () => { db = await testDb(); await db.reset(); files = await testFiles(); - t = await peopleApp(db, { sent, files }); + t = await peopleApp(db, { sent, files, mailConfigured: true }); ann = await person(t, db, 'ann'); bob = await person(t, db, 'bob'); vic = await person(t, db, 'vic'); @@ -671,3 +671,46 @@ describe('emailed and scheduled reports (D228)', () => { expect(listed?.lastError).toBe('rate_limited'); }); }); + +describe("email Kept can't send is refused (#141)", () => { + const code = (res: { statusCode: number; json: () => { code?: string } }) => [ + res.statusCode, + res.json().code, + ]; + + it('refuses email: true and a schedule on a server without mail; a plain report still runs', async () => { + const bare = await peopleApp(db, { sent: [], files }); + const zed = await person(bare, db, 'zed'); + const at = { locationId: zed.personalLocationId }; + const ask = (body: object) => call(bare, '/api/v1/reports/inventory', { as: zed, body }); + expect(code(await ask({ scope: at, email: true }))).toEqual([409, 'mail_not_configured']); + expect((await ask({ scope: at })).statusCode).toBe(202); + const put = await call(bare, '/api/v1/me/report-schedules', { + as: zed, + method: 'PUT', + body: { ...at, kind: 'inventory', cadence: 'weekly' }, + }); + expect(code(put)).toEqual([409, 'mail_not_configured']); + expect( + await own(db, 'SELECT id FROM public.report_schedules WHERE user_id = $1', [zed.userId]), + ).toEqual([]); + await bare.app.close(); + }); + + it('refuses them for an account with no address to send to', async () => { + const max = await person(t, db, 'max'); + await db.pools.auth.query('UPDATE auth."user" SET email = $2 WHERE id = $1', [ + max.userId, + `${max.userId}@managed.invalid`, + ]); + const at = { locationId: max.personalLocationId }; + const ask = await request(max, { scope: at, email: true }); + expect(code(ask)).toEqual([409, 'no_email_address']); + const put = await call(t, '/api/v1/me/report-schedules', { + as: max, + method: 'PUT', + body: { ...at, kind: 'inventory', cadence: 'monthly' }, + }); + expect(code(put)).toEqual([409, 'no_email_address']); + }); +}); diff --git a/apps/server/src/reports/routes.ts b/apps/server/src/reports/routes.ts index 2f886a52..39298c32 100644 --- a/apps/server/src/reports/routes.ts +++ b/apps/server/src/reports/routes.ts @@ -8,6 +8,7 @@ import type { InventoryDeps } from '../http/routes.js'; import { scopedRead, scopedWrite } from '../http/write.js'; import { gatherInsurance, insuranceCsv } from '../incidents/report.js'; import { locationOfMove } from '../moving/routes.js'; +import { requireMail } from './schedules.js'; import { ConditionReportBody, createConditionRun, @@ -35,7 +36,9 @@ import { // include?: {photos? = true, qr? = false, money? = true}, // locale?: 'en' | 'ar', digits?: 'western' | 'eastern'} // → 202 {id, status: 'queued', expiresAt}; 404 for a scope the caller can't see; 429 past -// five an hour (`retryAfter` seconds in the body and the Retry-After header). +// five an hour (`retryAfter` seconds in the body and the Retry-After header). With `email: true`, +// 409 `mail_not_configured` on a server without mail, or `no_email_address` for an account with +// no address (1.10.5, #141; the insurance report the same). // GET /api/v1/reports/:id // → {id, status: 'queued'|'running'|'done'|'failed'|'expired', scope, progress: {done, total}, // fileUrl? (five minutes, attachment), viewUrl? (the same, inline), bytes?, error?, @@ -105,10 +108,13 @@ export async function reportRoutes(app: KeptApp, deps: InventoryDeps): Promise { const { jobs } = needs(); try { - return await scopedWrite(pools, req, reply, async (tx, client, scope) => ({ - status: 202, - body: await createInventoryRun(tx, client, scope, jobs, req.body, req.id), - })); + return await scopedWrite(pools, req, reply, async (tx, client, scope) => { + if (req.body.email === true) await requireMail(deps, scope.userId); + return { + status: 202, + body: await createInventoryRun(tx, client, scope, jobs, req.body, req.id), + }; + }); } catch (err) { const retryAfter = err instanceof AppError ? err.extra?.retryAfter : undefined; if (typeof retryAfter === 'number') reply.header('retry-after', String(retryAfter)); @@ -149,10 +155,13 @@ export async function reportRoutes(app: KeptApp, deps: InventoryDeps): Promise { const { jobs } = needs(); try { - return await scopedWrite(pools, req, reply, async (tx, client, scope) => ({ - status: 202, - body: await createInsuranceRun(tx, client, scope, jobs, req.body, req.id), - })); + return await scopedWrite(pools, req, reply, async (tx, client, scope) => { + if (req.body.email === true) await requireMail(deps, scope.userId); + return { + status: 202, + body: await createInsuranceRun(tx, client, scope, jobs, req.body, req.id), + }; + }); } catch (err) { const retryAfter = err instanceof AppError ? err.extra?.retryAfter : undefined; if (typeof retryAfter === 'number') reply.header('retry-after', String(retryAfter)); diff --git a/apps/server/src/reports/schedules.ts b/apps/server/src/reports/schedules.ts index 0322ad6f..848bdabc 100644 --- a/apps/server/src/reports/schedules.ts +++ b/apps/server/src/reports/schedules.ts @@ -1,4 +1,4 @@ -import { REPORT_CADENCES, SCHEDULED_REPORT_KINDS } from '@kept/shared'; +import { ErrorCode, REPORT_CADENCES, SCHEDULED_REPORT_KINDS } from '@kept/shared'; import { z } from 'zod'; import { isUndeliverableEmail } from '../auth/emails.js'; import { withScope, withSystem } from '../db/scope.js'; @@ -30,6 +30,9 @@ import { // inventory for any member, the insurance report for whoever may read its money (D158). Audited // on the person's account (`report.schedule`, `report.unschedule`). // +// A schedule is refused (409 `mail_not_configured` / `no_email_address`, requireMail) when the +// report could never reach the person (1.10.5, #141). +// // The hourly `report-schedules` job (kept_system) finds the schedules due (Mondays or the 1st, // from 08:00 in the location's time zone, once that local day) and requests each run in its // owner's scope with `email: true`, exactly as the person would: their rights, the location's @@ -103,6 +106,8 @@ export async function reportScheduleRoutes(app: KeptApp, deps: InventoryDeps): P const { locationId, kind, cadence } = req.body; // Only what the person could ask for by hand (404 for a location they can't see). await requireMembership(client, locationId); + // A schedule only ever mails (#141). + await requireMail(deps, scope.userId); if (kind === 'insurance') { await requireInsuranceReader( tx, @@ -154,6 +159,26 @@ export async function reportScheduleRoutes(app: KeptApp, deps: InventoryDeps): P ); } +/** + * 1.10.5 (#141): refuse a request that asks for an email Kept can't send, instead of accepting it + * and dropping the mail: 409 `mail_not_configured` on a server without KEPT_SMTP_URL, 409 + * `no_email_address` for an account with no address to send to (a managed one, D47). An address + * that isn't verified yet is still accepted: verifying it is the person's next step, and the mail + * waits for nothing (mailReport skips it). + */ +export async function requireMail( + deps: Pick, + userId: string, +): Promise { + if (!deps.notify?.mailConfigured) throw new AppError(ErrorCode.mail_not_configured, 409); + const { rows } = await deps.pools.auth.query<{ email: string | null }>( + 'SELECT email FROM auth."user" WHERE id = $1', + [userId], + ); + const email = rows[0]?.email; + if (!email || isUndeliverableEmail(email)) throw new AppError(ErrorCode.no_email_address, 409); +} + /** A person's deliverable address and language, as the report mail needs it; null for a managed, * unverified, disabled or leaving account (none of them is mailed a report). */ export async function mailAddressOf( diff --git a/apps/web/src/api/inventory/mock/reports.ts b/apps/web/src/api/inventory/mock/reports.ts index 57f13d94..65f8557f 100644 --- a/apps/web/src/api/inventory/mock/reports.ts +++ b/apps/web/src/api/inventory/mock/reports.ts @@ -103,6 +103,14 @@ export function reportsRoutes(state: MockState): MockRoute[] { ); }; + /** 1.10.5 (#141): an email Kept can't send is refused, as the server does. */ + const mailRefusal = () => + !state.household.settings.smtpConfigured + ? err(409, 'mail_not_configured', "Email isn't set up on this server.") + : !state.me.user.email + ? err(409, 'no_email_address', 'Your account has no email address.') + : null; + return [ route('POST', p.reportsInventory, ({ body }) => { const gate = sessionGate(state); @@ -110,6 +118,8 @@ export function reportsRoutes(state: MockState): MockRoute[] { const b = body as InventoryReportBody; const locationIds = covered(b?.scope); if (!locationIds) return notFound(); + const refused = b.email ? mailRefusal() : null; + if (refused) return refused; const me = state.me.user.id; const recent = [...runs().values()].filter( (r) => r.userId === me && Date.now() - Date.parse(r.createdAt) < HOUR_MS, @@ -167,6 +177,8 @@ export function reportsRoutes(state: MockState): MockRoute[] { const b = body as Pick; const loc = state.locations.find((l) => l.id === b.locationId); if (!loc) return notFound(); + const refused = mailRefusal(); + if (refused) return refused; const all = reportSchedulesOf(state); const had = all.find((s) => s.locationId === b.locationId && s.kind === b.kind); if (had) { diff --git a/apps/web/src/components/reports/print-sheet.tsx b/apps/web/src/components/reports/print-sheet.tsx index b199d35f..032f9503 100644 --- a/apps/web/src/components/reports/print-sheet.tsx +++ b/apps/web/src/components/reports/print-sheet.tsx @@ -11,21 +11,29 @@ * 1.3 (D228): "Email it to me" also mails the PDF when it's done, and for a location "Send it * again" keeps a weekly or monthly schedule of the whole location (Settings → Me → Notifications * lists and stops them). + * + * 1.10.5 (#141): both only work by email, so where it can't go out they're disabled, dimmed, and + * a line with a lock says why (MailLock): mail isn't set up on this server (owners and admins get + * the docs' setup page, others are told to ask the owner), or the account has no address. The + * server refuses them the same way (`mail_not_configured`, `no_email_address`). */ import { Trans, useLingui } from '@lingui/react/macro'; import { useMutation, useQueryClient } from '@tanstack/react-query'; -import { useState, useSyncExternalStore } from 'react'; +import { useId, useState, useSyncExternalStore } from 'react'; import { Button as AriaButton } from 'react-aria-components'; import { isApiError } from '@/api/client'; +import { useNotificationSettings } from '@/api/household/queries'; import { reportKeys, reportsApi } from '@/api/inventory/reports'; import type { InventoryReportBody, ReportScope } from '@/api/inventory/types'; +import { useLocations, useMe } from '@/api/queries'; import { FieldEditor } from '@/components/filters/panel'; import { useFilterRegistry } from '@/components/filters/registry'; import type { FilterDef } from '@/components/filters/types'; import { useValueLabels } from '@/components/filters/values'; -import { ChevronEndIcon, ChevronStartIcon, DocumentIcon } from '@/components/icons'; +import { ChevronEndIcon, ChevronStartIcon, DocumentIcon, LockIcon } from '@/components/icons'; import { Notice, useErrorText } from '@/components/page'; import { Sheet } from '@/components/places/sheet'; +import { MAIL_SETUP_DOCS } from '@/components/setup-code-help'; import { Button } from '@/components/ui/button'; import { DialogFooter } from '@/components/ui/dialog'; import { Segmented } from '@/components/ui/segmented'; @@ -51,6 +59,72 @@ function useOnline(): boolean { ); } +/** Why email can't go out from here, or null while it can (or isn't known yet: the server + * refuses it then anyway). */ +type MailBlock = 'server' | 'address' | null; + +function useMailBlock(locationIds: string[]): { block: MailBlock; canSetUp: boolean } { + const settings = useNotificationSettings(); + const me = useMe(); + const locations = useLocations(); + const user = me.data?.user; + const block: MailBlock = + settings.data?.smtpConfigured === false + ? 'server' + : user && (user.managed || !user.email) + ? 'address' + : null; + // The instance admin sets mail up; an owner or admin of these locations is who people ask. + const canSetUp = + !!user?.instanceAdmin || + (locations.data ?? []).some( + (l) => locationIds.includes(l.id) && (l.role === 'owner' || l.role === 'admin'), + ); + return { block, canSetUp }; +} + +/** The line under the email-only options: a lock, why, and what to do about it. */ +function MailLock({ + id, + block, + canSetUp, +}: { + id: string; + block: 'server' | 'address'; + canSetUp: boolean; +}) { + return ( +

+

+ ); +} + export function PrintInventory({ scope, locationIds, @@ -112,8 +186,13 @@ function PrintForm({ const [includeEnded, setIncludeEnded] = useState(false); const [includeTrashed, setIncludeTrashed] = useState(false); const [qr, setQr] = useState(false); - const [email, setEmail] = useState(false); - const [repeat, setRepeat] = useState<'never' | 'weekly' | 'monthly'>('never'); + const [emailChosen, setEmail] = useState(false); + const [repeatChosen, setRepeat] = useState<'never' | 'weekly' | 'monthly'>('never'); + const { block, canSetUp } = useMailBlock(locationIds); + const lockId = useId(); + // Locked, neither goes in the request whatever was chosen before the settings arrived. + const email = emailChosen && !block; + const repeat = block ? 'never' : repeatChosen; const [runId, setRunId] = useState(null); const qc = useQueryClient(); const locationId = 'locationId' in scope ? scope.locationId : null; @@ -147,10 +226,19 @@ function PrintForm({ }, onSuccess: ({ id }) => setRunId(id), }); - const startError = (e: unknown) => - isApiError(e) && e.code === 'rate_limited' - ? t`Kept makes at most five reports an hour. Try again later.` - : errorText(e); + const startError = (e: unknown) => { + if (!isApiError(e)) return errorText(e); + switch (e.code) { + case 'rate_limited': + return t`Kept makes at most five reports an hour. Try again later.`; + case 'mail_not_configured': + return t`Email isn't set up on this server.`; + case 'no_email_address': + return t`Add an email address to your account.`; + default: + return errorText(e); + } + }; if (runId) return ( @@ -247,7 +335,12 @@ function PrintForm({ QR codes - + Email it to me @@ -257,6 +350,8 @@ function PrintForm({ label={t`Send it again`} value={repeat} onChange={setRepeat} + isDisabled={!!block} + aria-describedby={block ? lockId : undefined} options={[ { id: 'never', label: t`Never` }, { id: 'weekly', label: t`Every week` }, @@ -273,6 +368,7 @@ function PrintForm({ ) : null} ) : null} + {block ? : null} {start.error ? {startError(start.error)} : null}