From 9ddf2d2fab8150272031eabff3115da2511f8a60 Mon Sep 17 00:00:00 2001 From: Aleksei Vesnin Date: Wed, 23 Sep 2026 21:36:52 +0300 Subject: [PATCH] feat(settings): the import preview edits the check-in facts, and the form takes the format's bounds and anchors Co-Authored-By: Claude Opus 5.5 --- src/components/settings/data-export-card.tsx | 2 +- .../settings/data-import-card.render.test.tsx | 162 ++++++++++++++++++ src/components/settings/data-import-card.tsx | 129 +++++++++----- .../settings/import-check-in-details.tsx | 155 +++++++++++++++++ src/components/user/user-fields-form.tsx | 12 +- src/lib/api/auth.ts | 5 +- src/lib/api/logbook-import.ts | 68 +++++++- src/lib/import-check-in.test.ts | 122 +++++++++++++ src/lib/import-check-in.ts | 148 ++++++++++++++++ src/lib/logbook-import.test.ts | 3 + src/lib/logbook-import.ts | 15 +- src/lib/validations/user-fields.test.ts | 74 ++++++++ src/lib/validations/user-fields.ts | 70 ++++++-- 13 files changed, 891 insertions(+), 74 deletions(-) create mode 100644 src/components/settings/import-check-in-details.tsx create mode 100644 src/lib/import-check-in.test.ts create mode 100644 src/lib/import-check-in.ts diff --git a/src/components/settings/data-export-card.tsx b/src/components/settings/data-export-card.tsx index bb03819a..095aea68 100644 --- a/src/components/settings/data-export-card.tsx +++ b/src/components/settings/data-export-card.tsx @@ -66,7 +66,7 @@ const EXPORT_ROWS: ExportRow[] = [ icon: FileCode, title: "UDDF", description: - "Every dive with its sites, trips, gases, cylinders, gear and sample profile, in the open format Subsurface, MacDive and divelogs.de import. A dive recorded by two computers writes one profile here — the recording shown by default — because a UDDF dive carries one set of samples; the DiveJSON and the archive carry them all. This is the file to hand another program, and the import card below reads it back too — gear sets, service history, your courses and your c-cards have no slot in it, and ride in the DiveJSON and the archive instead.", + "Every dive with its sites, trips, gases, cylinders, gear and sample profile, plus your date of birth, phone and dive insurance, in the open format Subsurface, MacDive and divelogs.de import. A dive recorded by two computers writes one profile here — the recording shown by default — because a UDDF dive carries one set of samples; the DiveJSON and the archive carry them all. This is the file to hand another program, and the import card below reads it back too — gear sets, service history, your courses, your c-cards, your emergency contact and your insurance policy number have no slot in it, and ride in the DiveJSON and the archive instead.", }, { format: "csv", diff --git a/src/components/settings/data-import-card.render.test.tsx b/src/components/settings/data-import-card.render.test.tsx index 65f836fd..fcb8debc 100644 --- a/src/components/settings/data-import-card.render.test.tsx +++ b/src/components/settings/data-import-card.render.test.tsx @@ -4,9 +4,11 @@ import userEvent from "@testing-library/user-event"; import { DataImportCard } from "./data-import-card"; import type { ConversionReport, + ImportCheckInDetail, ImportPreview, ImportReport, } from "@/lib/api/logbook-import"; +import { isoDaysFromNow } from "@/test/local-day"; // What only a render can reach: that a preview is shown and nothing is written // until the diver says so, that `restored` survives to the screen as its own @@ -21,6 +23,11 @@ const mocks = vi.hoisted(() => ({ preview: vi.fn(), apply: vi.fn(), toast: vi.fn(), + refreshUser: vi.fn(), +})); + +vi.mock("@/contexts/AuthContext", () => ({ + useAuth: () => ({ refreshUser: mocks.refreshUser }), })); vi.mock("@/lib/api/logbook-import", async (importOriginal) => ({ @@ -55,6 +62,7 @@ function preview(overrides: Partial = {}): ImportPreview { generator: { name: "OpenDiving", version: "0.4.0" }, archive: false, token: "tok-1", + check_in_details: [], ...overrides, }; } @@ -386,3 +394,157 @@ describe("the logbook import card", () => { ); }); }); + +describe("the check-in details in an import preview", () => { + const details: ImportCheckInDetail[] = [ + { detail: "born_on", account: null, proposed: "1988-04-02" }, + { detail: "phone", account: "+44 1", proposed: "+44 2" }, + { + detail: "emergency_contact", + account: { name: "Sam", phone: "0111", relationship: "Partner" }, + proposed: { name: "Alex", phone: "0456", relationship: null }, + }, + { + detail: "insurance", + account: null, + proposed: { provider: "DAN Europe", number: "P-42", expires_on: null }, + }, + ]; + + const applyButton = () => + screen.getByRole("button", { name: /import this logbook/i }); + + async function previewWith(checkIn: ImportCheckInDetail[]) { + mocks.preview.mockResolvedValue(preview({ check_in_details: checkIn })); + render(); + await choose(documentFile()); + await screen.findByText(/nothing has been written yet/i); + } + + it("shows no section, and sends no facts, for a document carrying none", async () => { + mocks.apply.mockResolvedValue(report()); + await previewWith([]); + + expect(screen.queryByText("Check-in details")).not.toBeInTheDocument(); + await userEvent.click(applyButton()); + await waitFor(() => expect(mocks.apply).toHaveBeenCalledTimes(1)); + expect(mocks.apply.mock.calls[0][2]).toBeUndefined(); + }); + + it("shows each fact's account value beside the proposal, pre-filled", async () => { + await previewWith(details); + + const contact = screen.getByRole("group", { name: "Emergency contact" }); + expect( + within(contact).getByText(/yours now: sam · 0111 · partner/i), + ).toBeVisible(); + expect(within(contact).getByLabelText("Name")).toHaveValue("Alex"); + expect(screen.getByLabelText("Phone number")).toHaveValue("+44 2"); + const insurance = screen.getByRole("group", { name: "Dive insurance" }); + expect(within(insurance).getByText(/yours now: not set/i)).toBeVisible(); + expect(within(insurance).getByLabelText("Policy number")).toHaveValue( + "P-42", + ); + }); + + it("sends the facts as edited, and leaves a kept one out", async () => { + mocks.apply.mockResolvedValue(report()); + await previewWith(details); + + const contact = screen.getByRole("group", { name: "Emergency contact" }); + await userEvent.clear(within(contact).getByLabelText("Their phone number")); + await userEvent.type( + within(contact).getByLabelText("Their phone number"), + "0999", + ); + const insurance = screen.getByRole("group", { name: "Dive insurance" }); + await userEvent.clear(within(insurance).getByLabelText("Provider")); + await userEvent.clear(within(insurance).getByLabelText("Policy number")); + const phone = screen.getByRole("group", { name: "Phone number" }); + await userEvent.click( + within(phone).getByRole("button", { name: "Keep mine" }), + ); + expect(within(phone).queryByLabelText("Phone number")).toBeNull(); + + await userEvent.click(applyButton()); + + await waitFor(() => expect(mocks.apply).toHaveBeenCalledTimes(1)); + // The phone is absent rather than null: absent is what leaves the account's + // alone, and null would clear it. + expect(mocks.apply.mock.calls[0][2]).toEqual({ + born_on: "1988-04-02", + emergency_contact: { name: "Alex", phone: "0999", relationship: null }, + insurance: null, + }); + }); + + it("refuses a contact phone without a name before any request", async () => { + await previewWith(details); + + const contact = screen.getByRole("group", { name: "Emergency contact" }); + await userEvent.clear(within(contact).getByLabelText("Name")); + await userEvent.click(applyButton()); + + expect( + await screen.findByText( + "Required while the emergency contact has a phone or a relationship", + ), + ).toBeInTheDocument(); + expect(mocks.apply).not.toHaveBeenCalled(); + }); + + it("refuses a date of birth in the future, but not once it is kept", async () => { + mocks.apply.mockResolvedValue(report()); + await previewWith([ + { detail: "born_on", account: null, proposed: isoDaysFromNow(1) }, + ]); + + await userEvent.click(applyButton()); + expect( + await screen.findByText("Date of birth cannot be in the future"), + ).toBeInTheDocument(); + expect(mocks.apply).not.toHaveBeenCalled(); + + await userEvent.click(screen.getByRole("button", { name: "Leave unset" })); + expect( + screen.queryByText("Date of birth cannot be in the future"), + ).not.toBeInTheDocument(); + await userEvent.click(applyButton()); + await waitFor(() => expect(mocks.apply).toHaveBeenCalledTimes(1)); + expect(mocks.apply.mock.calls[0][2]).toBeUndefined(); + }); + + it("re-reads the signed-in user only when a fact was written", async () => { + // A re-read resets every form on the page seeded from the user, so it is + // spent only when the check-in card would otherwise be showing stale facts. + mocks.apply.mockResolvedValueOnce( + report({ + notes: [ + { + code: "check_in_detail_written", + collection: null, + uuid: null, + message: "The phone number confirmed in the preview was saved.", + }, + ], + }), + ); + await previewWith(details); + await userEvent.click(applyButton()); + await screen.findByText("Imported"); + expect(mocks.refreshUser).toHaveBeenCalledTimes(1); + + mocks.refreshUser.mockClear(); + mocks.apply.mockResolvedValueOnce(report()); + await userEvent.click(screen.getByRole("button", { name: "Done" })); + await choose(documentFile()); + await userEvent.click( + await screen.findByRole("button", { + name: /import this logbook/i, + }), + ); + await waitFor(() => expect(mocks.apply).toHaveBeenCalledTimes(2)); + await screen.findByText("Imported"); + expect(mocks.refreshUser).not.toHaveBeenCalled(); + }); +}); diff --git a/src/components/settings/data-import-card.tsx b/src/components/settings/data-import-card.tsx index f8fd9110..e0149644 100644 --- a/src/components/settings/data-import-card.tsx +++ b/src/components/settings/data-import-card.tsx @@ -13,6 +13,11 @@ import { CardTitle, } from "@/components/ui/card"; import { useToast } from "@/components/ui/use-toast"; +import { + ImportCheckInDetails, + useImportCheckIn, +} from "@/components/settings/import-check-in-details"; +import { useAuth } from "@/contexts/AuthContext"; import { getApiErrorMessage } from "@/lib/api/error"; import { importSourceLabel, @@ -20,9 +25,11 @@ import { LOGBOOK_IMPORT_ACCEPT, MAX_IMPORT_ARCHIVE_SIZE, MAX_IMPORT_DOCUMENT_SIZE, + type ImportCheckInSubmission, type ImportPreview, type ImportReport, } from "@/lib/api/logbook-import"; +import { checkInWasWritten } from "@/lib/import-check-in"; import { collectionLabel, collectionRowIsEmpty, @@ -271,6 +278,69 @@ function ImportReportView({ ); } +// The plan waiting for the diver's word. Mounted per preview, keyed on its token, so +// the check-in form inside it is seeded from this preview's proposal and no other. +function PendingImport({ + file, + preview, + isApplying, + onApply, + onCancel, +}: { + file: File; + preview: ImportPreview; + isApplying: boolean; + onApply: (checkIn: ImportCheckInSubmission | undefined) => void; + onCancel: () => void; +}) { + const checkIn = useImportCheckIn(preview.check_in_details); + + const handleApply = async () => { + const submission = await checkIn.collect(); + if (submission !== null) onApply(submission); + }; + + return ( +
+
+

Ready to import {file.name}

+ {/* `importSourceSentence` reads `conversion` before `format` and + `generator`, which on a converted upload describe the document the API + ended up reading rather than the file just named above it - + "divejson 1.0, written by divejson convert" about somebody's `.ssrf`. */} +

+ {importSourceSentence(preview)} Nothing has been written yet. +

+
+ + + + + +
+ + +
+
+ ); +} + // "Bring your logbook back" on the settings page, directly under the export card // it is the other half of. The product's promise is that nothing in an account is // locked to this app; export makes that falsifiable and import is what closes the @@ -283,6 +353,7 @@ function ImportReportView({ // discover afterwards. export function DataImportCard() { const { toast } = useToast(); + const { refreshUser } = useAuth(); const fileInputRef = useRef(null); // The file is held alongside the preview because apply needs *both* it and the @@ -350,7 +421,7 @@ export function DataImportCard() { } }; - const handleApply = async () => { + const handleApply = async (checkIn: ImportCheckInSubmission | undefined) => { if (!pending) return; try { @@ -358,7 +429,13 @@ export function DataImportCard() { const applied = await logbookImportAPI.apply( pending.file, pending.preview.token, + checkIn, ); + // Only when a fact changed: the check-in card on this page seeds from the + // signed-in user, and saving it from a stale copy would send the imported + // facts back as nulls. Not otherwise, since a refresh resets every mounted + // form seeded from that user. + if (checkInWasWritten(applied)) await refreshUser(); setResult(applied); setPending(null); toast({ @@ -447,48 +524,14 @@ export function DataImportCard() { {pending && ( -
-
-

- Ready to import {pending.file.name} -

- {/* `importSourceSentence` reads `conversion` before `format` and - `generator`, which on a converted upload describe the document - the API ended up reading rather than the file just named above - it - "divejson 1.0, written by divejson convert" about - somebody's `.ssrf`. */} -

- {importSourceSentence(pending.preview)} Nothing has been written - yet. -

-
- - - -
- - -
-
+ setPending(null)} + /> )} {result && ( diff --git a/src/components/settings/import-check-in-details.tsx b/src/components/settings/import-check-in-details.tsx new file mode 100644 index 00000000..f85a8058 --- /dev/null +++ b/src/components/settings/import-check-in-details.tsx @@ -0,0 +1,155 @@ +"use client"; + +import { useMemo, useState } from "react"; +import { useForm, type Resolver, type UseFormReturn } from "react-hook-form"; +import { zodResolver } from "@hookform/resolvers/zod"; + +import { Button } from "@/components/ui/button"; +import { Form } from "@/components/ui/form"; +import { UserField } from "@/components/user/user-fields-form"; +import type { + ImportCheckInDetail, + ImportCheckInDetailKey, + ImportCheckInSubmission, +} from "@/lib/api/logbook-import"; +import { + CHECK_IN_DETAIL_FIELDS, + CHECK_IN_DETAIL_LABELS, + checkInAccountSummary, + checkInProposalValues, + checkInSubmission, +} from "@/lib/import-check-in"; +import { + userFieldsSchema, + type UserFieldValues, +} from "@/lib/validations/user-fields"; + +export interface ImportCheckIn { + details: readonly ImportCheckInDetail[]; + form: UseFormReturn; + kept: ReadonlySet; + toggleKept: (detail: ImportCheckInDetailKey) => void; + /** + * Validates what is on screen and resolves to the facts to send: `undefined` when + * there are none, `null` when a field is invalid and now says why. + */ + collect: () => Promise; +} + +/** + * The editable half of an import preview: one form over the check-in facts the + * document carries, seeded with the API's proposal. + * + * Seeded once, so the host mounts it per preview (keyed on the token). The resolver + * covers only the facts not kept, so a kept fact's proposal cannot block the apply. + */ +export function useImportCheckIn( + details: readonly ImportCheckInDetail[], +): ImportCheckIn { + const [kept, setKept] = useState>( + () => new Set(), + ); + + const resolver = useMemo(() => { + const fields = details + .filter(({ detail }) => !kept.has(detail)) + .flatMap(({ detail }) => CHECK_IN_DETAIL_FIELDS[detail]); + // Through `unknown` for the reason `UserFieldsForm` gives: a schema built from + // a runtime list infers an index signature. + return zodResolver( + userFieldsSchema(fields), + ) as unknown as Resolver; + }, [details, kept]); + + const form = useForm({ + resolver, + defaultValues: checkInProposalValues(details), + }); + + const toggleKept = (detail: ImportCheckInDetailKey) => { + form.clearErrors([...CHECK_IN_DETAIL_FIELDS[detail]]); + setKept((current) => { + const next = new Set(current); + if (!next.delete(detail)) next.add(detail); + return next; + }); + }; + + const collect = async () => { + if (!(await form.trigger())) return null; + const submission = checkInSubmission(details, kept, form.getValues()); + return Object.keys(submission).length > 0 ? submission : undefined; + }; + + return { details, form, kept, toggleKept, collect }; +} + +// Each fact the document carries, the account's value beside the proposal. What is +// in the boxes when the diver imports is what is written; "Keep mine" takes a fact +// out of the apply altogether, which is different from emptying it - an emptied +// fact is cleared from the account. +export function ImportCheckInDetails({ checkIn }: { checkIn: ImportCheckIn }) { + const { details, form, kept, toggleKept } = checkIn; + if (details.length === 0) return null; + + return ( +
+
+

Check-in details

+

+ This file carries details a dive shop asks for. What is in the boxes + is saved to your account when you import; change or clear any of it, + or keep yours. +

+
+ {details.map((entry) => { + const fields = CHECK_IN_DETAIL_FIELDS[entry.detail]; + const mine = checkInAccountSummary(entry); + const isKept = kept.has(entry.detail); + return ( +
+ + {CHECK_IN_DETAIL_LABELS[entry.detail]} + +

+ Yours now: {mine ?? "not set"} +

+ {isKept ? ( +

+ {mine + ? "Keeping yours; nothing from this file is saved." + : "Left unset; nothing from this file is saved."} +

+ ) : ( + fields.map((field) => ( + + )) + )} + +
+ ); + })} +
+
+
+ ); +} diff --git a/src/components/user/user-fields-form.tsx b/src/components/user/user-fields-form.tsx index 5b0d6f71..3a4aa6d7 100644 --- a/src/components/user/user-fields-form.tsx +++ b/src/components/user/user-fields-form.tsx @@ -196,12 +196,18 @@ export function UserFieldsForm({ ); } -function UserField({ +/** + * One field as every form over the diver's record writes it. `hideLabel` keeps the + * label for a screen reader only, for a host whose legend already says it. + */ +export function UserField({ name, control, + hideLabel = false, }: { name: UserFieldKey; control: Control; + hideLabel?: boolean; }) { const spec = FIELD_SPECS[name]; @@ -211,7 +217,9 @@ function UserField({ name={name} render={({ field }) => ( - {spec.label} + + {spec.label} + {spec.kind === "date" ? ( diff --git a/src/lib/api/auth.ts b/src/lib/api/auth.ts index 5ff42b3c..bbdf4bc9 100644 --- a/src/lib/api/auth.ts +++ b/src/lib/api/auth.ts @@ -129,8 +129,9 @@ export interface UpdateProfileData { // `dive_form_hidden_fields` above, and a form sends every field it showed on // every save - so a group the diver emptied arrives as nulls rather than being // left behind. The string bounds are the columns' own, and `PATCH /user` is - // `extra="forbid"`, so an over-long value is a 422; - // `validations/user-fields.ts` mirrors them. + // `extra="forbid"`, so an over-long value is a 422, as is a contact with no name + // or an insurance with no provider while anything else of it is set; + // `validations/user-fields.ts` mirrors both. date_of_birth?: string | null; phone?: string | null; emergency_contact_name?: string | null; diff --git a/src/lib/api/logbook-import.ts b/src/lib/api/logbook-import.ts index 93edf629..eea0708e 100644 --- a/src/lib/api/logbook-import.ts +++ b/src/lib/api/logbook-import.ts @@ -146,7 +146,13 @@ export type ImportNoteCode = // would teach a diver to distrust a correct result. | "recording_attached" | "recording_filled" - | "diver_not_applied"; + | "diver_not_applied" + // An emergency contact or insurance the document carries but the preview does not + // offer: it names nobody, or it is not the first. A warning. + | "check_in_detail_dropped" + // Only when a column actually changed, which is what tells the card to re-read + // the signed-in user. + | "check_in_detail_written"; /** One thing the import decided, addressed to the diver. */ export interface ImportNote { @@ -281,6 +287,52 @@ export interface ImportGenerator { version: string | null; } +/** An emergency contact as the preview shows it and as the apply takes it back. */ +export interface ImportCheckInEmergencyContact { + name: string | null; + phone: string | null; + relationship: string | null; +} + +/** A dive insurance, on the same terms. `expires_on` is a bare `YYYY-MM-DD`. */ +export interface ImportCheckInInsurance { + provider: string | null; + number: string | null; + expires_on: string | null; +} + +/** + * One check-in fact the document carries: what the account holds beside what the + * API proposes. An object is proposed whole - the account's own when the document's + * agrees with it on every member it carries, otherwise the document's alone. + */ +export type ImportCheckInDetail = + | { detail: "born_on"; account: string | null; proposed: string } + | { detail: "phone"; account: string | null; proposed: string } + | { + detail: "emergency_contact"; + account: ImportCheckInEmergencyContact | null; + proposed: ImportCheckInEmergencyContact; + } + | { + detail: "insurance"; + account: ImportCheckInInsurance | null; + proposed: ImportCheckInInsurance; + }; + +export type ImportCheckInDetailKey = ImportCheckInDetail["detail"]; + +/** + * The facts the diver confirmed, sent beside the token. A key left out is not + * written, `null` clears the fact, and an object replaces all of the account's. + */ +export interface ImportCheckInSubmission { + born_on?: string | null; + phone?: string | null; + emergency_contact?: ImportCheckInEmergencyContact | null; + insurance?: ImportCheckInInsurance | null; +} + /** What `POST /import/logbook/preview` returns. Nothing has been written. */ export interface ImportPreview extends ImportReport { /** @@ -305,6 +357,11 @@ export interface ImportPreview extends ImportReport { * scratch, so the token is not a stored plan to replay. */ token: string; + /** + * One entry per check-in fact the document carries, in the order date of birth, + * phone, emergency contact, insurance. Empty when it carries none. + */ + check_in_details: ImportCheckInDetail[]; } /** What `POST /import/logbook` returns. Everything in it has been committed. */ @@ -363,11 +420,18 @@ export const logbookImportAPI = { * `token` must be the one from this same file's `preview` call: the API hashes * the body it receives and refuses a token minted for different bytes, which is * what stops a diver approving one document and uploading another. + * + * `checkIn` is the facts to write, as a JSON field; omitted, none is written. */ - async apply(file: File, token: string): Promise { + async apply( + file: File, + token: string, + checkIn?: ImportCheckInSubmission, + ): Promise { const formData = new FormData(); formData.append("file", file); formData.append("token", token); + if (checkIn) formData.append("check_in_details", JSON.stringify(checkIn)); const response = await apiClient.post( "/import/logbook", diff --git a/src/lib/import-check-in.test.ts b/src/lib/import-check-in.test.ts new file mode 100644 index 00000000..f54cc833 --- /dev/null +++ b/src/lib/import-check-in.test.ts @@ -0,0 +1,122 @@ +import { describe, expect, it } from "vitest"; + +import { + checkInAccountSummary, + checkInProposalValues, + checkInSubmission, + checkInWasWritten, +} from "./import-check-in"; +import type { + ImportCheckInDetail, + ImportReport, +} from "@/lib/api/logbook-import"; +import { formatDateOnly } from "@/lib/date-time"; +import { EMPTY_USER_FIELDS } from "@/lib/validations/user-fields"; + +const details: ImportCheckInDetail[] = [ + { detail: "born_on", account: null, proposed: "1988-04-02" }, + { detail: "phone", account: "+44 1", proposed: "+44 2" }, + { + detail: "emergency_contact", + account: null, + proposed: { name: "Alex", phone: "0456", relationship: null }, + }, + { + detail: "insurance", + account: { provider: "DAN", number: "P-1", expires_on: "2027-03-01" }, + proposed: { provider: "Aqua", number: null, expires_on: null }, + }, +]; + +describe("checkInProposalValues", () => { + it("seeds every field a carried fact holds with the proposal, and nothing else", () => { + expect(checkInProposalValues(details)).toEqual({ + ...EMPTY_USER_FIELDS, + date_of_birth: "1988-04-02", + phone: "+44 2", + emergency_contact_name: "Alex", + emergency_contact_phone: "0456", + insurance_provider: "Aqua", + }); + }); +}); + +describe("checkInAccountSummary", () => { + it("reads the account's side as one line, dates formatted", () => { + expect(checkInAccountSummary(details[1])).toBe("+44 1"); + expect(checkInAccountSummary(details[3])).toBe( + `DAN · P-1 · expires ${formatDateOnly("2027-03-01")}`, + ); + }); + + it("is null where the account holds nothing of the fact", () => { + expect(checkInAccountSummary(details[0])).toBeNull(); + expect(checkInAccountSummary(details[2])).toBeNull(); + expect( + checkInAccountSummary({ + detail: "insurance", + account: { provider: " ", number: null, expires_on: null }, + proposed: { provider: "Aqua", number: null, expires_on: null }, + }), + ).toBeNull(); + }); +}); + +describe("checkInSubmission", () => { + it("sends every fact not kept, trimmed, with an emptied one as null", () => { + const values = { + ...checkInProposalValues(details), + phone: " +44 3 ", + insurance_provider: "", + }; + expect(checkInSubmission(details, new Set(), values)).toEqual({ + born_on: "1988-04-02", + phone: "+44 3", + emergency_contact: { name: "Alex", phone: "0456", relationship: null }, + insurance: null, + }); + }); + + it("leaves a kept fact out altogether, which is what keeps the account's", () => { + const submission = checkInSubmission( + details, + new Set(["phone", "insurance"] as const), + checkInProposalValues(details), + ); + expect(Object.keys(submission)).toEqual(["born_on", "emergency_contact"]); + }); + + it("sends nothing for a fact the document does not carry", () => { + expect( + checkInSubmission( + [details[1]], + new Set(), + checkInProposalValues(details), + ), + ).toEqual({ phone: "+44 2" }); + }); +}); + +describe("checkInWasWritten", () => { + const report = (codes: string[]): ImportReport => ({ + collections: [], + files: { referenced: 0, restored: 0, not_contained: 0, skipped: 0 }, + notes: codes.map((code) => ({ + code: code as ImportReport["notes"][number]["code"], + collection: null, + uuid: null, + message: "", + })), + notes_truncated: 0, + conversion: null, + }); + + it("is true only when the apply says a fact was written", () => { + expect(checkInWasWritten(report(["check_in_detail_written"]))).toBe(true); + expect( + checkInWasWritten( + report(["check_in_detail_dropped", "diver_not_applied"]), + ), + ).toBe(false); + }); +}); diff --git a/src/lib/import-check-in.ts b/src/lib/import-check-in.ts new file mode 100644 index 00000000..fcb5ae67 --- /dev/null +++ b/src/lib/import-check-in.ts @@ -0,0 +1,148 @@ +import type { + ImportCheckInDetail, + ImportCheckInDetailKey, + ImportCheckInSubmission, + ImportReport, +} from "@/lib/api/logbook-import"; +import { formatDateOnly } from "@/lib/date-time"; +import { + EMERGENCY_CONTACT_FIELDS, + EMPTY_USER_FIELDS, + INSURANCE_FIELDS, + userFieldsUpdate, + type UserFieldKey, + type UserFieldValues, +} from "@/lib/validations/user-fields"; + +/** + * The form module's fields each check-in fact is held in. + * + * The import preview edits a fact in the same fields `/settings` does, so a value + * is bounded, anchored and cleared by one set of rules wherever it is typed. + */ +export const CHECK_IN_DETAIL_FIELDS: Record< + ImportCheckInDetailKey, + readonly UserFieldKey[] +> = { + born_on: ["date_of_birth"], + phone: ["phone"], + emergency_contact: EMERGENCY_CONTACT_FIELDS, + insurance: INSURANCE_FIELDS, +}; + +export const CHECK_IN_DETAIL_LABELS: Record = { + born_on: "Date of birth", + phone: "Phone number", + emergency_contact: "Emergency contact", + insurance: "Dive insurance", +}; + +// One side of a fact as the form's fields hold it, `""` for anything unset. +function asFields( + entry: ImportCheckInDetail, + side: "account" | "proposed", +): Partial { + switch (entry.detail) { + case "born_on": + return { date_of_birth: entry[side] ?? "" }; + case "phone": + return { phone: entry[side] ?? "" }; + case "emergency_contact": { + const contact = entry[side]; + return { + emergency_contact_name: contact?.name ?? "", + emergency_contact_phone: contact?.phone ?? "", + emergency_contact_relationship: contact?.relationship ?? "", + }; + } + case "insurance": { + const insurance = entry[side]; + return { + insurance_provider: insurance?.provider ?? "", + insurance_policy_number: insurance?.number ?? "", + insurance_expires_on: insurance?.expires_on ?? "", + }; + } + } +} + +/** The form's starting values: every fact the document carries, as proposed. */ +export function checkInProposalValues( + details: readonly ImportCheckInDetail[], +): UserFieldValues { + return Object.assign( + { ...EMPTY_USER_FIELDS }, + ...details.map((entry) => asFields(entry, "proposed")), + ); +} + +/** What the account holds of one fact, as one line, or `null` when it holds none. */ +export function checkInAccountSummary( + entry: ImportCheckInDetail, +): string | null { + const values = asFields(entry, "account"); + const parts = CHECK_IN_DETAIL_FIELDS[entry.detail].flatMap((field) => { + const value = values[field]?.trim(); + if (!value) return []; + if (field === "date_of_birth") return [formatDateOnly(value)]; + if (field === "insurance_expires_on") + return [`expires ${formatDateOnly(value)}`]; + return [value]; + }); + return parts.length > 0 ? parts.join(" · ") : null; +} + +/** + * The apply's `check_in_details` body: every fact not in `kept`, as the form holds it. + * + * A kept fact is left out, which is what makes the API leave it alone. An emptied + * one goes as `null`, which clears it - trimmed and nulled by `userFieldsUpdate`, the + * same rule a `/settings` save follows. + */ +export function checkInSubmission( + details: readonly ImportCheckInDetail[], + kept: ReadonlySet, + values: UserFieldValues, +): ImportCheckInSubmission { + const submission: ImportCheckInSubmission = {}; + for (const { detail } of details) { + if (kept.has(detail)) continue; + const update = userFieldsUpdate(CHECK_IN_DETAIL_FIELDS[detail], values); + switch (detail) { + case "born_on": + submission.born_on = update.date_of_birth ?? null; + break; + case "phone": + submission.phone = update.phone ?? null; + break; + case "emergency_contact": { + const contact = { + name: update.emergency_contact_name ?? null, + phone: update.emergency_contact_phone ?? null, + relationship: update.emergency_contact_relationship ?? null, + }; + submission.emergency_contact = Object.values(contact).some(Boolean) + ? contact + : null; + break; + } + case "insurance": { + const insurance = { + provider: update.insurance_provider ?? null, + number: update.insurance_policy_number ?? null, + expires_on: update.insurance_expires_on ?? null, + }; + submission.insurance = Object.values(insurance).some(Boolean) + ? insurance + : null; + break; + } + } + } + return submission; +} + +/** Whether an apply changed any of the account's check-in facts. */ +export function checkInWasWritten(report: ImportReport): boolean { + return report.notes.some((note) => note.code === "check_in_detail_written"); +} diff --git a/src/lib/logbook-import.test.ts b/src/lib/logbook-import.test.ts index af09b043..3a5e7492 100644 --- a/src/lib/logbook-import.test.ts +++ b/src/lib/logbook-import.test.ts @@ -132,6 +132,7 @@ describe("noteIsWarning", () => { "reference_unresolved", "species_unresolved", "file_skipped", + "check_in_detail_dropped", ].every(noteIsWarning), ).toBe(true); }); @@ -156,6 +157,7 @@ describe("noteIsWarning", () => { // is reporting the import working, not falling short. "recording_attached", "recording_filled", + "check_in_detail_written", ].some(noteIsWarning), ).toBe(false); }); @@ -333,6 +335,7 @@ describe("importSourceSentence", () => { generator: { name: "OpenDiving", version: "0.4.0" }, archive: false, token: "tok-1", + check_in_details: [], ...overrides, }; } diff --git a/src/lib/logbook-import.ts b/src/lib/logbook-import.ts index 864fd5f1..b678df7f 100644 --- a/src/lib/logbook-import.ts +++ b/src/lib/logbook-import.ts @@ -84,11 +84,13 @@ export function collectionRowIsEmpty(row: ImportCollectionReport): boolean { * How a note should read: as something lost, or as something merely worth saying. * * The split is "did the diver end up with less than the document described". - * A skipped record, a dropped value, an unresolved reference or species and a - * file that could not be stored all lose something, so they are warnings. A - * linked or restored record, either remap, a `diver` deliberately not applied and - * a file whose bytes simply are not in a bare document are all the import working - * as designed - `file_not_contained` especially, which is the *expected* state of + * A skipped record, a dropped value, an unresolved reference or species, a + * file that could not be stored and a check-in detail the preview does not offer + * all lose something, so they are warnings. A linked or restored record, either + * remap, a `diver` deliberately not applied, a check-in detail written as + * confirmed and a file whose bytes simply are not in a bare document are all the + * import working as designed - `file_not_contained` especially, which is the + * *expected* state of * every referenced file when a document rather than an archive was imported, and * colouring it as a failure would make the ordinary case look broken. * @@ -101,7 +103,8 @@ export function noteIsWarning(code: ImportNoteCode | string): boolean { code === "value_dropped" || code === "reference_unresolved" || code === "species_unresolved" || - code === "file_skipped" + code === "file_skipped" || + code === "check_in_detail_dropped" ); } diff --git a/src/lib/validations/user-fields.test.ts b/src/lib/validations/user-fields.test.ts index f375d3fd..a6253a79 100644 --- a/src/lib/validations/user-fields.test.ts +++ b/src/lib/validations/user-fields.test.ts @@ -75,6 +75,80 @@ describe("userFieldsSchema", () => { ).success, ).toBe(true); }); + + it("takes the columns' bounds", () => { + const contact = userFieldsSchema([...EMERGENCY_CONTACT_FIELDS]); + const insurance = userFieldsSchema([...INSURANCE_FIELDS]); + expect( + contact.safeParse(values({ emergency_contact_name: "a".repeat(255) })) + .success, + ).toBe(true); + expect( + contact.safeParse(values({ emergency_contact_name: "a".repeat(256) })) + .success, + ).toBe(false); + expect( + contact.safeParse( + values({ + emergency_contact_name: "Alex", + emergency_contact_relationship: "a".repeat(64), + }), + ).success, + ).toBe(true); + expect( + contact.safeParse( + values({ + emergency_contact_name: "Alex", + emergency_contact_relationship: "a".repeat(65), + }), + ).success, + ).toBe(false); + expect( + insurance.safeParse(values({ insurance_provider: "a".repeat(255) })) + .success, + ).toBe(true); + expect( + insurance.safeParse(values({ insurance_provider: "a".repeat(256) })) + .success, + ).toBe(false); + }); + + it("requires a contact's name and an insurance's provider once anything else of it is set", () => { + const contact = userFieldsSchema([...EMERGENCY_CONTACT_FIELDS]); + const insurance = userFieldsSchema([...INSURANCE_FIELDS]); + + const noName = contact.safeParse( + values({ emergency_contact_name: " ", emergency_contact_phone: "0456" }), + ); + expect(noName.success).toBe(false); + // The API's own sentence, on the field it names. + expect(noName.error?.issues).toEqual([ + expect.objectContaining({ + path: ["emergency_contact_name"], + message: + "Required while the emergency contact has a phone or a relationship", + }), + ]); + + const noProvider = insurance.safeParse( + values({ insurance_expires_on: "2027-03-01" }), + ); + expect(noProvider.error?.issues).toEqual([ + expect.objectContaining({ + path: ["insurance_provider"], + message: + "Required while the insurance has a policy number or an expiry date", + }), + ]); + + // An empty group is not a contact at all, and one with its anchor is whole. + expect(contact.safeParse(values()).success).toBe(true); + expect( + insurance.safeParse( + values({ insurance_provider: "DAN", insurance_policy_number: "P-1" }), + ).success, + ).toBe(true); + }); }); describe("userFieldsUpdate", () => { diff --git a/src/lib/validations/user-fields.ts b/src/lib/validations/user-fields.ts index df40a699..74ebc265 100644 --- a/src/lib/validations/user-fields.ts +++ b/src/lib/validations/user-fields.ts @@ -65,20 +65,20 @@ const FIELD_SCHEMAS: Record = { phone: z.string().max(32, "Phone number cannot exceed 32 characters"), insurance_provider: z .string() - .max(100, "Provider cannot exceed 100 characters"), + .max(255, "Provider cannot exceed 255 characters"), insurance_policy_number: z .string() .max(64, "Policy number cannot exceed 64 characters"), insurance_expires_on: optionalDate("Use a valid date"), emergency_contact_name: z .string() - .max(100, "Name cannot exceed 100 characters"), + .max(255, "Name cannot exceed 255 characters"), emergency_contact_phone: z .string() .max(32, "Phone number cannot exceed 32 characters"), emergency_contact_relationship: z .string() - .max(50, "Relationship cannot exceed 50 characters"), + .max(64, "Relationship cannot exceed 64 characters"), }; /** @@ -99,6 +99,24 @@ export const EMERGENCY_CONTACT_FIELDS = [ "emergency_contact_relationship", ] as const; +/** + * The member each group is named by, first in its list, and what `PATCH /user` says + * when a group holds anything else without it - the API's own sentences, so the + * field reads the same whether the form or the server caught it. + */ +const ANCHORED_GROUPS = [ + { + fields: EMERGENCY_CONTACT_FIELDS, + message: + "Required while the emergency contact has a phone or a relationship", + }, + { + fields: INSURANCE_FIELDS, + message: + "Required while the insurance has a policy number or an expiry date", + }, +] as const; + /** Which fields are required, and so cannot be cleared. */ const REQUIRED: ReadonlySet = new Set(["name", "username"]); @@ -114,22 +132,38 @@ export function userFieldsSchema(fields: readonly UserFieldKey[]) { const shape = Object.fromEntries( fields.map((field) => [field, FIELD_SCHEMAS[field]]), ); - const schema = z.object(shape); + const text = (data: Record, field: UserFieldKey) => + String(data[field] ?? "").trim(); - // Mirrors the API's own `field_validator`, so a slipped digit is caught in the - // field rather than coming back as a 422. Today itself is accepted, as it is - // there. An object-level refine, unlike a field-level transform, leaves what - // `z.input<>` infers untouched. - return fields.includes("date_of_birth") - ? schema.refine( - (data) => - !data.date_of_birth || String(data.date_of_birth) <= todayIsoDate(), - { - message: "Date of birth cannot be in the future", - path: ["date_of_birth"], - }, - ) - : schema; + // Both rules mirror the API's, so each is caught in the field rather than coming + // back as a 422. An object-level refine, unlike a field-level transform, leaves + // what `z.input<>` infers untouched. + return z.object(shape).superRefine((data, ctx) => { + // Today itself is accepted, as it is there. + if ( + fields.includes("date_of_birth") && + text(data, "date_of_birth") > todayIsoDate() + ) { + ctx.addIssue({ + code: "custom", + message: "Date of birth cannot be in the future", + path: ["date_of_birth"], + }); + } + // Every host shows a group whole, so the fields shown are the row after the save. + for (const { + fields: [anchor, ...members], + message, + } of ANCHORED_GROUPS) { + if ( + fields.includes(anchor) && + !text(data, anchor) && + members.some((member) => text(data, member)) + ) { + ctx.addIssue({ code: "custom", message, path: [anchor] }); + } + } + }); } /** The stored record as a form holds it: `null` and absent both become `""`. */