Skip to content

feat(settings): the import preview edits the check-in facts, and the form takes the format's bounds and anchors #283

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aleskiontherun merged 1 commit into from
Sep 23, 2026
Merged

feat(settings): the import preview edits the check-in facts, and the form takes the format's bounds and anchors #283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/settings/data-export-card.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
162 changes: 162 additions & 0 deletions src/components/settings/data-import-card.render.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) => ({
Expand Down Expand Up @@ -55,6 +62,7 @@ function preview(overrides: Partial<ImportPreview> = {}): ImportPreview {
generator: { name: "OpenDiving", version: "0.4.0" },
archive: false,
token: "tok-1",
check_in_details: [],
...overrides,
};
}
Expand Down Expand Up @@ -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(<DataImportCard />);
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();
});
});
129 changes: 86 additions & 43 deletions src/components/settings/data-import-card.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,23 @@ 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,
logbookImportAPI,
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,
Expand Down Expand Up @@ -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 (
<div className="rounded-lg border p-4 space-y-4">
<div>
<h3 className="font-medium">Ready to import {file.name}</h3>
{/* `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`. */}
<p className="text-sm text-muted-foreground mt-1">
{importSourceSentence(preview)} Nothing has been written yet.
</p>
</div>

<ImportReportView report={preview} archive={preview.archive} />

<ImportCheckInDetails checkIn={checkIn} />

<div className="flex flex-col sm:flex-row gap-2">
<Button type="button" onClick={handleApply} disabled={isApplying}>
{isApplying ? (
<div className="flex items-center space-x-2">
<ButtonSpinner />
<span>Importing...</span>
</div>
) : (
<span>Import this logbook</span>
)}
</Button>
<Button
type="button"
variant="outline"
onClick={onCancel}
disabled={isApplying}
>
Cancel
</Button>
</div>
</div>
);
}

// "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
Expand All @@ -283,6 +353,7 @@ function ImportReportView({
// discover afterwards.
export function DataImportCard() {
const { toast } = useToast();
const { refreshUser } = useAuth();
const fileInputRef = useRef<HTMLInputElement>(null);

// The file is held alongside the preview because apply needs *both* it and the
Expand Down Expand Up @@ -350,15 +421,21 @@ export function DataImportCard() {
}
};

const handleApply = async () => {
const handleApply = async (checkIn: ImportCheckInSubmission | undefined) => {
if (!pending) return;

try {
setIsApplying(true);
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({
Expand Down Expand Up @@ -447,48 +524,14 @@ export function DataImportCard() {
</div>

{pending && (
<div className="rounded-lg border p-4 space-y-4">
<div>
<h3 className="font-medium">
Ready to import {pending.file.name}
</h3>
{/* `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`. */}
<p className="text-sm text-muted-foreground mt-1">
{importSourceSentence(pending.preview)} Nothing has been written
yet.
</p>
</div>

<ImportReportView
report={pending.preview}
archive={pending.preview.archive}
/>

<div className="flex flex-col sm:flex-row gap-2">
<Button type="button" onClick={handleApply} disabled={isApplying}>
{isApplying ? (
<div className="flex items-center space-x-2">
<ButtonSpinner />
<span>Importing...</span>
</div>
) : (
<span>Import this logbook</span>
)}
</Button>
<Button
type="button"
variant="outline"
onClick={() => setPending(null)}
disabled={isApplying}
>
Cancel
</Button>
</div>
</div>
<PendingImport
key={pending.preview.token}
file={pending.file}
preview={pending.preview}
isApplying={isApplying}
onApply={handleApply}
onCancel={() => setPending(null)}
/>
)}

{result && (
Expand Down
Loading
Loading