fix(sharing): let the owner replace a link the upgrade closed, and count only what is shown

Migration 0273 revokes every live record share link minted before the selection
model, because their frozen scope was "everything except mood and cycle" — a
scope nobody chose. The owner has to see why the link stopped and be able to
replace it without retyping what they already decided, or a deliberate closure
reads as a fault. The list now names that state, says why in one sentence, and
offers a replacement that carries the label, the window and the lifetime
across, leaving exactly the decision that link never carried.

Writing the test for it found a defect in the picker. The group count and the
group checkbox read the catalogue's group rather than the leaves the surface
actually renders, so on the share-link form — where the insurance leaf is
hidden because the server refuses it — the identity row read "1/2" and drew
itself half-selected over a leaf the owner could neither see nor reach, and its
checkbox would have tried to add it. The three helpers take the rendered leaves
now, so a control cannot describe more than it offers.
This commit is contained in:
Marc-André Bombeck committed 2026-07-26 00:45:08 +02:00
1 parent 9e68ff525f
commit f12290532f
12 files changed
+220 -37

No files matched your search

+2 -1
View File
@@ -5373,7 +5373,8 @@
"scopeTitle": "Was dieser Link enthält",
"scopeInsuranceExcluded": "Versichertendaten werden über einen Link nie geteilt.",
"statusNeedsReselect": "Inhalt neu wählen",
"reselectReason": "Dieser Link lieferte einen Umfang, den niemand gewählt hatte, und wurde deshalb geschlossen. Legen Sie einen neuen an und wählen Sie den Inhalt."
"reselectReason": "Dieser Link lieferte einen Umfang, den niemand gewählt hatte, und wurde deshalb geschlossen. Legen Sie einen neuen an und wählen Sie den Inhalt.",
"reselectAction": "Ersatz anlegen"
},
"genderOther": "Divers",
"cycleTracking": {
+2 -1
View File
@@ -5373,7 +5373,8 @@
"scopeTitle": "What this link contains",
"scopeInsuranceExcluded": "Insurance details are never shared through a link.",
"statusNeedsReselect": "Pick contents again",
"reselectReason": "This link served a scope that was never chosen, so it was closed. Create a new one and pick what it should contain."
"reselectReason": "This link served a scope that was never chosen, so it was closed. Create a new one and pick what it should contain.",
"reselectAction": "Create a replacement"
},
"genderOther": "Other",
"cycleTracking": {
+2 -1
View File
@@ -5373,7 +5373,8 @@
"scopeTitle": "Qué contiene este enlace",
"scopeInsuranceExcluded": "Los datos del seguro nunca se comparten por enlace.",
"statusNeedsReselect": "Elegir contenido de nuevo",
"reselectReason": "Este enlace mostraba un alcance que nadie eligió, por eso se cerró. Cree uno nuevo y elija su contenido."
"reselectReason": "Este enlace mostraba un alcance que nadie eligió, por eso se cerró. Cree uno nuevo y elija su contenido.",
"reselectAction": "Crear un reemplazo"
},
"genderOther": "Otro",
"cycleTracking": {
+2 -1
View File
@@ -5373,7 +5373,8 @@
"scopeTitle": "Ce que contient ce lien",
"scopeInsuranceExcluded": "Les données d'assurance ne sont jamais partagées via un lien.",
"statusNeedsReselect": "Choisir à nouveau le contenu",
"reselectReason": "Ce lien diffusait un périmètre que personne n'avait choisi ; il a donc été fermé. Créez-en un nouveau et choisissez son contenu."
"reselectReason": "Ce lien diffusait un périmètre que personne n'avait choisi ; il a donc été fermé. Créez-en un nouveau et choisissez son contenu.",
"reselectAction": "Créer un remplacement"
},
"genderOther": "Autre",
"cycleTracking": {
+2 -1
View File
@@ -5373,7 +5373,8 @@
"scopeTitle": "Cosa contiene questo link",
"scopeInsuranceExcluded": "I dati assicurativi non vengono mai condivisi tramite link.",
"statusNeedsReselect": "Scegli di nuovo il contenuto",
"reselectReason": "Questo link mostrava un ambito che nessuno aveva scelto, perciò è stato chiuso. Creane uno nuovo e scegli cosa deve contenere."
"reselectReason": "Questo link mostrava un ambito che nessuno aveva scelto, perciò è stato chiuso. Creane uno nuovo e scegli cosa deve contenere.",
"reselectAction": "Crea un sostituto"
},
"genderOther": "Altro",
"cycleTracking": {
+2 -1
View File
@@ -5373,7 +5373,8 @@
"scopeTitle": "Co zawiera ten link",
"scopeInsuranceExcluded": "Dane ubezpieczenia nigdy nie są udostępniane przez link.",
"statusNeedsReselect": "Wybierz zawartość ponownie",
"reselectReason": "Ten link udostępniał zakres, którego nikt nie wybrał, więc został zamknięty. Utwórz nowy i wybierz jego zawartość."
"reselectReason": "Ten link udostępniał zakres, którego nikt nie wybrał, więc został zamknięty. Utwórz nowy i wybierz jego zawartość.",
"reselectAction": "Utwórz zamiennik"
},
"genderOther": "Inna",
"cycleTracking": {
@@ -0,0 +1,82 @@
/**
* The pick-again state on a link the selection upgrade closed.
*
* Migration 0273 revoked every live record share link minted before the
* selection model, because their frozen scope was "everything except mood and
* cycle" — a scope nobody chose. The owner has to see WHY the link stopped
* working and be able to replace it without retyping what they already
* decided, or the revocation reads as a fault.
*
* Mutation checks:
* - drop the `needsReselection` branch in `sharing-section.tsx` → "explains
* why the link closed" goes red.
* - drop `initialRangeDays` from `ShareLinkCreateForm`'s state seed →
* "carries the closed link's window into the replacement" goes red.
* - clamp nothing in `prefillFrom` → "clamps a lifetime past the cap" goes
* red.
*/
import { describe, expect, it, vi } from "vitest";
import { renderToStaticMarkup } from "react-dom/server";
import { I18nProvider } from "@/lib/i18n/context";
import { ShareLinkCreateForm } from "../share-link-create-form";
vi.mock("@tanstack/react-query", () => ({
useQueryClient: () => ({ invalidateQueries: vi.fn() }),
useMutation: () => ({ mutate: vi.fn(), isPending: false }),
useQuery: () => ({ data: undefined, isLoading: false }),
}));
function render(node: React.ReactElement): string {
return renderToStaticMarkup(
<I18nProvider initialLocale="en">{node}</I18nProvider>,
);
}
function numberInputValue(html: string, id: string): string | null {
const input = html.match(new RegExp(`<input[^>]*id="${id}"[^>]*>`))?.[0];
return input?.match(/value="([^"]*)"/)?.[1] ?? null;
}
describe("share-link create form — re-mint seeding", () => {
it("carries the closed link's window and lifetime into the replacement", () => {
const html = render(
<ShareLinkCreateForm
initialLabel="Cardiology clinic"
initialRangeDays={180}
initialExpiryDays={45}
/>,
);
expect(numberInputValue(html, "share-range")).toBe("180");
expect(numberInputValue(html, "share-expiry")).toBe("45");
expect(html).toContain("Cardiology clinic");
});
it("falls back to the 30-day default when nothing is carried across", () => {
const html = render(<ShareLinkCreateForm />);
expect(numberInputValue(html, "share-range")).toBe("30");
expect(numberInputValue(html, "share-expiry")).toBe("30");
});
it("offers the scope picker, with the insurance leaf absent from it", () => {
const html = render(<ShareLinkCreateForm />);
expect(html).toContain('data-testid="report-scope-picker"');
expect(html).toContain('data-testid="report-group-row-identity"');
// The server refuses the leaf with a 422; a control that cannot be
// honoured has no business rendering, so the identity group is one leaf
// wide here and two wide in the export panel.
const row = html.match(
/data-testid="report-group-row-identity"[\s\S]*?<\/div>/,
)?.[0];
expect(row).toBeDefined();
expect(row).toContain(">1/1<");
// The fenced tier renders its leaves unconditionally; none is checked.
expect(html).toContain('data-testid="report-leaf-MOOD"');
expect(html).not.toContain('data-testid="report-leaf-INSURANCE"');
});
it("hides the scope picker entirely for a documents-only share", () => {
const html = render(<ShareLinkCreateForm documentOnly />);
expect(html).not.toContain('data-testid="report-scope-picker"');
});
});
@@ -74,9 +74,12 @@ export function ReportScopePicker({
onChange(toggleLeaf(selected, leaf));
};
const handleGroup = (group: ReportGroupId) => {
const handleGroup = (
group: ReportGroupId,
leaves: readonly ReportLeafId[],
) => {
setRemembered(new Set(selected));
onChange(toggleGroup(selected, group, remembered));
onChange(toggleGroup(selected, group, leaves, remembered));
};
return (
@@ -89,11 +92,11 @@ export function ReportScopePicker({
labelKey={group.labelKey}
leaves={group.leaves}
selected={selected}
checkState={groupCheckState(group.id, selected)}
count={groupCount(group.id, selected)}
checkState={groupCheckState(group.leaves, selected)}
count={groupCount(group.leaves, selected)}
open={openGroups.has(group.id)}
panelId={`${idPrefix}-${group.id}`}
onToggleGroup={() => handleGroup(group.id)}
onToggleGroup={() => handleGroup(group.id, group.leaves)}
onToggleLeaf={handleLeaf}
onToggleOpen={() =>
setOpenGroups((prev) => {
@@ -168,6 +168,8 @@ export function ShareLinkCreateForm({
documentOnly = false,
initialDocuments,
initialLabel,
initialRangeDays,
initialExpiryDays,
onCreated,
}: {
/**
@@ -181,6 +183,14 @@ export function ShareLinkCreateForm({
initialDocuments?: PickedDocument[];
/** Optional pre-filled label (e.g. the document title). */
initialLabel?: string;
/**
* Optional pre-filled window and lifetime, in days. The sharing list passes
* these when the owner re-mints a link the selection upgrade closed, so the
* only decision left in front of them is the one that link never carried:
* what it should contain.
*/
initialRangeDays?: number;
initialExpiryDays?: number;
/** Fired after a link is minted, so an outer surface can react if needed. */
onCreated?: (created: ShareLinkCreated) => void;
}) {
@@ -188,8 +198,10 @@ export function ShareLinkCreateForm({
const queryClient = useQueryClient();
const [label, setLabel] = useState(initialLabel ?? "");
const [rangeDays, setRangeDays] = useState(DEFAULT_DAYS);
const [expiryDays, setExpiryDays] = useState(DEFAULT_DAYS);
const [rangeDays, setRangeDays] = useState(initialRangeDays ?? DEFAULT_DAYS);
const [expiryDays, setExpiryDays] = useState(
initialExpiryDays ?? DEFAULT_DAYS,
);
const [selectedDocs, setSelectedDocs] = useState<PickedDocument[]>(
initialDocuments ?? [],
);
+60 -5
View File
@@ -42,6 +42,7 @@ import { EmptyState } from "@/components/ui/empty-state";
import { SettingsCard } from "@/components/settings/settings-card";
import { SettingsCardHeader } from "@/components/settings/_card-header";
import {
MAX_DAYS,
ShareLinkCreateForm,
type ShareLinkSummary,
} from "@/components/settings/share-link-create-form";
@@ -61,12 +62,49 @@ export function SharingSection() {
);
}
/**
* Carry a closed link's window and lifetime into a new create form.
*
* The window is measured from the frozen `rangeStart` to now (a rolling link
* has no end), the lifetime from when it was created to when it would have
* expired. Both are clamped into the form's own bounds so a very old link
* cannot seed an out-of-range value.
*/
function prefillFrom(link: ShareLinkSummary) {
const day = 86_400_000;
const rangeDays = Math.max(
1,
Math.round((Date.now() - new Date(link.rangeStart).getTime()) / day),
);
const expiryDays = Math.min(
MAX_DAYS,
Math.max(
1,
Math.round(
(new Date(link.expiresAt).getTime() -
new Date(link.createdAt).getTime()) /
day,
),
),
);
return { key: Date.now(), label: link.label, rangeDays, expiryDays };
}
function ShareLinksCard() {
const { t } = useTranslations();
const { isAuthenticated } = useAuth();
const queryClient = useQueryClient();
const [showRevoked, setShowRevoked] = useState(false);
// A re-mint carries the closed link's label, window and lifetime across, so
// the only thing left to decide is the scope it never had. Keyed so the form
// remounts and picks the values up as its initial state.
const [prefill, setPrefill] = useState<{
key: number;
label: string;
rangeDays: number;
expiryDays: number;
} | null>(null);
const { data: links } = useQuery({
queryKey: queryKeys.shareLinks(),
@@ -101,7 +139,12 @@ function ShareLinksCard() {
description={t("settings.sharing.createDescription")}
/>
<ShareLinkCreateForm />
<ShareLinkCreateForm
key={prefill?.key ?? "new"}
initialLabel={prefill?.label}
initialRangeDays={prefill?.rangeDays}
initialExpiryDays={prefill?.expiryDays}
/>
<div className="space-y-2">
<h3 className="text-sm font-medium">
@@ -248,12 +291,24 @@ function ShareLinksCard() {
owner re-mints above with the label, window and expiry
still in front of them. */}
{link.needsReselection ? (
<p
className="text-muted-foreground text-xs"
<div
className="space-y-1.5"
data-testid={`share-needs-reselect-${link.id}`}
>
{t("settings.sharing.reselectReason")}
</p>
<p className="text-muted-foreground text-xs">
{t("settings.sharing.reselectReason")}
</p>
<Button
type="button"
variant="outline"
size="sm"
className="min-h-11 sm:min-h-9"
data-testid={`share-reselect-${link.id}`}
onClick={() => setPrefill(prefillFrom(link))}
>
{t("settings.sharing.reselectAction")}
</Button>
</div>
) : null}
<p className="text-muted-foreground text-xs">
<span className="font-medium">
@@ -30,24 +30,45 @@ const sensitive = REPORT_GROUPS.find((g) => g.id === "sensitive")!;
describe("scope picker state", () => {
it("derives none / mixed / all from the leaves alone", () => {
expect(groupCheckState("vitals", new Set())).toBe("none");
expect(groupCheckState("vitals", new Set([vitals.leaves[0]]))).toBe(
expect(groupCheckState(vitals.leaves, new Set())).toBe("none");
expect(groupCheckState(vitals.leaves, new Set([vitals.leaves[0]]))).toBe(
"mixed",
);
expect(groupCheckState("vitals", new Set(vitals.leaves))).toBe("all");
expect(groupCheckState(vitals.leaves, new Set(vitals.leaves))).toBe("all");
});
it("counts what is on against what exists", () => {
const count = groupCount("vitals", new Set([vitals.leaves[0]]));
it("counts what is on against what the surface renders", () => {
const count = groupCount(vitals.leaves, new Set([vitals.leaves[0]]));
expect(count).toEqual({ on: 1, total: vitals.leaves.length });
});
it("counts against the RENDERED leaves, not the catalogue group", () => {
// The share-link form hides the insurance leaf, so the identity group is
// one leaf wide there. A count over the full group would tell the owner a
// control covers something they cannot see or reach.
const identity = REPORT_GROUPS.find((g) => g.id === "identity")!;
const rendered = identity.leaves.filter((l) => l !== "INSURANCE");
const selected = new Set<ReportLeafId>(["PATIENT_IDENTITY"]);
expect(groupCount(rendered, selected)).toEqual({ on: 1, total: 1 });
expect(groupCheckState(rendered, selected)).toBe("all");
// And the whole group, for contrast.
expect(groupCount(identity.leaves, selected)).toEqual({ on: 1, total: 2 });
});
it("never adds a leaf the surface does not render", () => {
const identity = REPORT_GROUPS.find((g) => g.id === "identity")!;
const rendered = identity.leaves.filter((l) => l !== "INSURANCE");
const after = toggleGroup(new Set<ReportLeafId>(), "identity", rendered);
expect(after.has("PATIENT_IDENTITY")).toBe(true);
expect(after.has("INSURANCE")).toBe(false);
});
it("turns a mixed group fully on, and a full group fully off", () => {
const partial = new Set<ReportLeafId>([vitals.leaves[0]]);
const on = toggleGroup(partial, "vitals");
expect(groupCheckState("vitals", on)).toBe("all");
const off = toggleGroup(on, "vitals");
expect(groupCheckState("vitals", off)).toBe("none");
const on = toggleGroup(partial, "vitals", vitals.leaves);
expect(groupCheckState(vitals.leaves, on)).toBe("all");
const off = toggleGroup(on, "vitals", vitals.leaves);
expect(groupCheckState(vitals.leaves, off)).toBe("none");
});
it("restores the previous pattern when a group comes back on", () => {
@@ -55,14 +76,14 @@ describe("scope picker state", () => {
vitals.leaves[0],
vitals.leaves[1],
]);
const off = toggleGroup(original, "vitals", original);
const back = toggleGroup(off, "vitals", original);
const off = toggleGroup(original, "vitals", vitals.leaves, original);
const back = toggleGroup(off, "vitals", vitals.leaves, original);
expect([...back].sort()).toEqual([...original].sort());
});
it("lets no group control reach a fenced leaf", () => {
const before = new Set<ReportLeafId>();
const after = toggleGroup(before, "sensitive");
const after = toggleGroup(before, "sensitive", sensitive.leaves);
expect([...after]).toEqual([]);
for (const leaf of sensitive.leaves) {
expect(after.has(leaf)).toBe(false);
+11 -7
View File
@@ -24,11 +24,16 @@ export type PanelSelection = ReadonlySet<ReportLeafId>;
/** A group checkbox is on, off, or partly on. `mixed` is never a click target. */
export type GroupCheckState = "all" | "none" | "mixed";
/**
* `leaves` is the group's leaves AS THE SURFACE RENDERS THEM, which is not
* always the catalogue's: the share-link form hides the insurance leaf, and a
* count or a check state computed over the full group would then describe
* something the user cannot see or reach.
*/
export function groupCheckState(
group: ReportGroupId,
leaves: readonly ReportLeafId[],
selected: PanelSelection,
): GroupCheckState {
const leaves = REPORT_GROUPS.find((g) => g.id === group)?.leaves ?? [];
if (leaves.length === 0) return "none";
const on = leaves.filter((leaf) => selected.has(leaf)).length;
if (on === 0) return "none";
@@ -36,12 +41,11 @@ export function groupCheckState(
return "mixed";
}
/** How many of a group's leaves are chosen, and how many there are. */
/** How many of the rendered leaves are chosen, and how many there are. */
export function groupCount(
group: ReportGroupId,
leaves: readonly ReportLeafId[],
selected: PanelSelection,
): { on: number; total: number } {
const leaves = REPORT_GROUPS.find((g) => g.id === group)?.leaves ?? [];
return {
on: leaves.filter((leaf) => selected.has(leaf)).length,
total: leaves.length,
@@ -75,12 +79,12 @@ export function toggleLeaf(
export function toggleGroup(
selected: PanelSelection,
group: ReportGroupId,
leaves: readonly ReportLeafId[],
remembered?: PanelSelection,
): Set<ReportLeafId> {
if (group === SENSITIVE_GROUP_ID) return new Set(selected);
const leaves = REPORT_GROUPS.find((g) => g.id === group)?.leaves ?? [];
const next = new Set(selected);
if (groupCheckState(group, selected) === "all") {
if (groupCheckState(leaves, selected) === "all") {
for (const leaf of leaves) next.delete(leaf);
return next;
}