Merge feature/ticket-edit-persistence into develop
Ticket #9 : persistance de l'édition d'un ticket côté frontend (useTicketDetail + TicketDetail), tests Vitest tickets verts (13/13). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@ -9,7 +9,7 @@
|
|||||||
* context" prefill lets the user hand an agent "work on #N" (F7).
|
* context" prefill lets the user hand an agent "work on #N" (F7).
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import { useEffect, useState } from "react";
|
import { useEffect, useRef, useState } from "react";
|
||||||
|
|
||||||
import type { TicketLinkKind } from "@/domain";
|
import type { TicketLinkKind } from "@/domain";
|
||||||
import { Button, Input, Spinner, cn } from "@/shared";
|
import { Button, Input, Spinner, cn } from "@/shared";
|
||||||
@ -88,8 +88,12 @@ export function TicketDetail({
|
|||||||
const { agents, nameOf } = useProjectAgents(projectId);
|
const { agents, nameOf } = useProjectAgents(projectId);
|
||||||
const t = vm.ticket;
|
const t = vm.ticket;
|
||||||
|
|
||||||
// Local draft state, re-seeded whenever the loaded ticket version changes so a
|
// Local draft state. Re-seeded only on a genuine (re)load of the ticket —
|
||||||
// conflict-reload or an external edit refreshes the fields.
|
// initial load, an external-event refresh, or a conflict reload — tracked via
|
||||||
|
// the hook's `reloadCount`. It is deliberately NOT re-seeded on an ordinary
|
||||||
|
// optimistic mutation (e.g. changing the priority), which bumps the ticket's
|
||||||
|
// version but must not clobber an in-progress, unsaved title/description edit
|
||||||
|
// (#9).
|
||||||
const [title, setTitle] = useState("");
|
const [title, setTitle] = useState("");
|
||||||
const [description, setDescription] = useState("");
|
const [description, setDescription] = useState("");
|
||||||
const [carnet, setCarnet] = useState("");
|
const [carnet, setCarnet] = useState("");
|
||||||
@ -97,20 +101,71 @@ export function TicketDetail({
|
|||||||
const [linkKind, setLinkKind] = useState<TicketLinkKind>("relatesTo");
|
const [linkKind, setLinkKind] = useState<TicketLinkKind>("relatesTo");
|
||||||
const [assignPick, setAssignPick] = useState("");
|
const [assignPick, setAssignPick] = useState("");
|
||||||
const [copied, setCopied] = useState(false);
|
const [copied, setCopied] = useState(false);
|
||||||
|
// Confirmation shown when the user tries to close with unsaved changes.
|
||||||
|
const [confirmClose, setConfirmClose] = useState(false);
|
||||||
|
|
||||||
const version = t?.version;
|
const reloadCount = vm.reloadCount;
|
||||||
|
const seededReload = useRef(-1);
|
||||||
|
// The persisted values the draft was last seeded from. Lets a benign reload
|
||||||
|
// (echo of one's own priority change, or an external edit to an untouched
|
||||||
|
// field) adopt fresh values field-by-field WITHOUT clobbering a field the user
|
||||||
|
// is currently editing (#9).
|
||||||
|
const baseline = useRef<{
|
||||||
|
title: string;
|
||||||
|
description: string;
|
||||||
|
carnet: string;
|
||||||
|
} | null>(null);
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!t) return;
|
if (!t) return;
|
||||||
|
// Re-seed once per (re)load. An optimistic mutation (e.g. a priority change)
|
||||||
|
// changes `t` without bumping `reloadCount`, so this guard short-circuits and
|
||||||
|
// preserves the draft (#9).
|
||||||
|
if (seededReload.current === reloadCount) return;
|
||||||
|
seededReload.current = reloadCount;
|
||||||
|
const nextCarnet = t.carnet ?? "";
|
||||||
|
const base = baseline.current;
|
||||||
|
// Force a full reset on the initial load and on a conflict reload (the local
|
||||||
|
// edit is stale — the user must re-apply). A benign reload preserves any
|
||||||
|
// field the user has edited since the last seed.
|
||||||
|
const forced = base === null || vm.conflict;
|
||||||
|
if (forced) {
|
||||||
setTitle(t.title);
|
setTitle(t.title);
|
||||||
setDescription(t.description);
|
setDescription(t.description);
|
||||||
setCarnet(t.carnet ?? "");
|
setCarnet(nextCarnet);
|
||||||
}, [t, version]);
|
} else {
|
||||||
|
setTitle((cur) => (cur === base.title ? t.title : cur));
|
||||||
|
setDescription((cur) => (cur === base.description ? t.description : cur));
|
||||||
|
setCarnet((cur) => (cur === base.carnet ? nextCarnet : cur));
|
||||||
|
}
|
||||||
|
baseline.current = {
|
||||||
|
title: t.title,
|
||||||
|
description: t.description,
|
||||||
|
carnet: nextCarnet,
|
||||||
|
};
|
||||||
|
}, [t, reloadCount, vm.conflict]);
|
||||||
|
|
||||||
const dirtyFields =
|
const dirtyFields =
|
||||||
!!t && (title !== t.title || description !== t.description);
|
!!t && (title !== t.title || description !== t.description);
|
||||||
const dirtyCarnet = !!t && carnet !== (t.carnet ?? "");
|
const dirtyCarnet = !!t && carnet !== (t.carnet ?? "");
|
||||||
|
const dirty = dirtyFields || dirtyCarnet;
|
||||||
const assignable = agents.filter((a) => !t?.assignedAgentIds.includes(a.id));
|
const assignable = agents.filter((a) => !t?.assignedAgentIds.includes(a.id));
|
||||||
|
|
||||||
|
/** Close guard: prompt before discarding unsaved edits, else close directly. */
|
||||||
|
function requestClose() {
|
||||||
|
if (dirty) setConfirmClose(true);
|
||||||
|
else onClose();
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Save whatever is dirty, then close only if every save succeeded. */
|
||||||
|
async function saveAndClose() {
|
||||||
|
let ok = true;
|
||||||
|
if (dirtyFields) ok = (await vm.updateFields({ title, description })) && ok;
|
||||||
|
if (ok && dirtyCarnet) ok = (await vm.saveCarnet(carnet)) && ok;
|
||||||
|
setConfirmClose(false);
|
||||||
|
// On a failed save (e.g. a version conflict) stay open so the banner shows.
|
||||||
|
if (ok) onClose();
|
||||||
|
}
|
||||||
|
|
||||||
async function copyDelegation() {
|
async function copyDelegation() {
|
||||||
if (!t) return;
|
if (!t) return;
|
||||||
const ok = await copyToClipboard(
|
const ok = await copyToClipboard(
|
||||||
@ -149,7 +204,7 @@ export function TicketDetail({
|
|||||||
>
|
>
|
||||||
{copied ? "Copied" : "Delegation context"}
|
{copied ? "Copied" : "Delegation context"}
|
||||||
</Button>
|
</Button>
|
||||||
<Button size="sm" variant="ghost" onClick={onClose}>
|
<Button size="sm" variant="ghost" onClick={requestClose}>
|
||||||
Close
|
Close
|
||||||
</Button>
|
</Button>
|
||||||
</div>
|
</div>
|
||||||
@ -445,6 +500,56 @@ export function TicketDetail({
|
|||||||
</Section>
|
</Section>
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
|
{/* ── Unsaved-changes confirmation on close (#9) ── */}
|
||||||
|
{confirmClose && (
|
||||||
|
<div
|
||||||
|
role="dialog"
|
||||||
|
aria-modal="true"
|
||||||
|
aria-label="unsaved changes"
|
||||||
|
className="fixed inset-0 z-[60] flex items-center justify-center bg-black/55 p-4"
|
||||||
|
>
|
||||||
|
<div className="w-full max-w-sm rounded-lg border border-border bg-surface p-5 shadow-xl">
|
||||||
|
<h4 className="text-sm font-semibold text-content">
|
||||||
|
Modifications non sauvegardées
|
||||||
|
</h4>
|
||||||
|
<p className="mt-2 text-sm text-muted">
|
||||||
|
Vous avez des modifications non sauvegardées. Voulez-vous les
|
||||||
|
sauvegarder avant de fermer ?
|
||||||
|
</p>
|
||||||
|
<div className="mt-4 flex flex-wrap items-center justify-end gap-2">
|
||||||
|
<Button
|
||||||
|
size="sm"
|
||||||
|
variant="ghost"
|
||||||
|
aria-label="cancel close"
|
||||||
|
disabled={vm.busy}
|
||||||
|
onClick={() => setConfirmClose(false)}
|
||||||
|
>
|
||||||
|
Annuler
|
||||||
|
</Button>
|
||||||
|
<Button
|
||||||
|
size="sm"
|
||||||
|
variant="ghost"
|
||||||
|
aria-label="close without saving"
|
||||||
|
disabled={vm.busy}
|
||||||
|
onClick={onClose}
|
||||||
|
className="text-danger hover:text-danger"
|
||||||
|
>
|
||||||
|
Fermer sans sauvegarder
|
||||||
|
</Button>
|
||||||
|
<Button
|
||||||
|
size="sm"
|
||||||
|
aria-label="save and close"
|
||||||
|
loading={vm.busy}
|
||||||
|
disabled={vm.busy}
|
||||||
|
onClick={() => void saveAndClose()}
|
||||||
|
>
|
||||||
|
Sauvegarder
|
||||||
|
</Button>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
@ -227,6 +227,126 @@ describe("TicketsView", () => {
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("preserves an in-progress description edit across a priority change (#9)", async () => {
|
||||||
|
const t = await ticket.create(PROJECT_ID, { title: "Persisted" });
|
||||||
|
renderView(ticket, system, agent);
|
||||||
|
|
||||||
|
fireEvent.click(await screen.findByText("Persisted"));
|
||||||
|
const dialog = await screen.findByRole("dialog");
|
||||||
|
|
||||||
|
// Start editing the description WITHOUT saving.
|
||||||
|
const desc = within(dialog).getByLabelText("Description") as HTMLTextAreaElement;
|
||||||
|
fireEvent.change(desc, { target: { value: "draft in progress" } });
|
||||||
|
// Also edit the title to cover both draft fields.
|
||||||
|
const titleInput = within(dialog).getByLabelText("Title") as HTMLInputElement;
|
||||||
|
fireEvent.change(titleInput, { target: { value: "Persisted (wip)" } });
|
||||||
|
|
||||||
|
// Change the priority — an immediate-apply mutation that re-fetches the ticket
|
||||||
|
// and bumps its version. The unsaved draft must survive.
|
||||||
|
fireEvent.change(within(dialog).getByLabelText("ticket priority"), {
|
||||||
|
target: { value: "high" },
|
||||||
|
});
|
||||||
|
|
||||||
|
// The backend applied the priority bump…
|
||||||
|
await waitFor(async () => {
|
||||||
|
const fresh = await ticket.read(PROJECT_ID, t.ref);
|
||||||
|
expect(fresh.priority).toBe("high");
|
||||||
|
expect(fresh.version).toBe(2);
|
||||||
|
});
|
||||||
|
|
||||||
|
// …and the in-progress edits are still there (not reverted).
|
||||||
|
expect(
|
||||||
|
(within(dialog).getByLabelText("Description") as HTMLTextAreaElement).value,
|
||||||
|
).toBe("draft in progress");
|
||||||
|
expect(
|
||||||
|
(within(dialog).getByLabelText("Title") as HTMLInputElement).value,
|
||||||
|
).toBe("Persisted (wip)");
|
||||||
|
// The description was never persisted.
|
||||||
|
const persisted = await ticket.read(PROJECT_ID, t.ref);
|
||||||
|
expect(persisted.description).toBe("");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("prompts before closing with unsaved changes, and closes directly when clean (#9)", async () => {
|
||||||
|
const t = await ticket.create(PROJECT_ID, { title: "CloseGuard" });
|
||||||
|
const onClose = vi.fn();
|
||||||
|
const gateways = { ticket, system, agent } as unknown as Gateways;
|
||||||
|
render(
|
||||||
|
<DIProvider gateways={gateways}>
|
||||||
|
<TicketDetail
|
||||||
|
projectId={PROJECT_ID}
|
||||||
|
ticketRef={t.ref}
|
||||||
|
onClose={onClose}
|
||||||
|
onOpenRef={() => {}}
|
||||||
|
/>
|
||||||
|
</DIProvider>,
|
||||||
|
);
|
||||||
|
|
||||||
|
const detail = await screen.findByRole("dialog", { name: `ticket ${t.ref}` });
|
||||||
|
|
||||||
|
// Clean state → Close fires onClose directly, no confirmation dialog.
|
||||||
|
fireEvent.click(within(detail).getByText("Close"));
|
||||||
|
expect(onClose).toHaveBeenCalledTimes(1);
|
||||||
|
expect(screen.queryByRole("dialog", { name: "unsaved changes" })).toBeNull();
|
||||||
|
|
||||||
|
// Now make the draft dirty and try to close again.
|
||||||
|
fireEvent.change(within(detail).getByLabelText("Description"), {
|
||||||
|
target: { value: "unsaved body" },
|
||||||
|
});
|
||||||
|
fireEvent.click(within(detail).getByText("Close"));
|
||||||
|
|
||||||
|
// The confirmation appears and onClose was NOT called again.
|
||||||
|
const confirm = await screen.findByRole("dialog", { name: "unsaved changes" });
|
||||||
|
expect(onClose).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
|
// "Annuler" dismisses the popup and keeps the editor open.
|
||||||
|
fireEvent.click(within(confirm).getByLabelText("cancel close"));
|
||||||
|
await waitFor(() =>
|
||||||
|
expect(screen.queryByRole("dialog", { name: "unsaved changes" })).toBeNull(),
|
||||||
|
);
|
||||||
|
expect(onClose).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
|
// Re-open the confirmation and choose "Fermer sans sauvegarder".
|
||||||
|
fireEvent.click(within(detail).getByText("Close"));
|
||||||
|
const confirm2 = await screen.findByRole("dialog", { name: "unsaved changes" });
|
||||||
|
fireEvent.click(within(confirm2).getByLabelText("close without saving"));
|
||||||
|
expect(onClose).toHaveBeenCalledTimes(2);
|
||||||
|
// The unsaved description was discarded (never persisted).
|
||||||
|
const fresh = await ticket.read(PROJECT_ID, t.ref);
|
||||||
|
expect(fresh.description).toBe("");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("saves unsaved changes then closes when choosing « Sauvegarder » (#9)", async () => {
|
||||||
|
const t = await ticket.create(PROJECT_ID, { title: "SaveOnClose" });
|
||||||
|
const onClose = vi.fn();
|
||||||
|
const gateways = { ticket, system, agent } as unknown as Gateways;
|
||||||
|
render(
|
||||||
|
<DIProvider gateways={gateways}>
|
||||||
|
<TicketDetail
|
||||||
|
projectId={PROJECT_ID}
|
||||||
|
ticketRef={t.ref}
|
||||||
|
onClose={onClose}
|
||||||
|
onOpenRef={() => {}}
|
||||||
|
/>
|
||||||
|
</DIProvider>,
|
||||||
|
);
|
||||||
|
|
||||||
|
const detail = await screen.findByRole("dialog", { name: `ticket ${t.ref}` });
|
||||||
|
fireEvent.change(within(detail).getByLabelText("Description"), {
|
||||||
|
target: { value: "final body" },
|
||||||
|
});
|
||||||
|
fireEvent.click(within(detail).getByText("Close"));
|
||||||
|
|
||||||
|
const confirm = await screen.findByRole("dialog", { name: "unsaved changes" });
|
||||||
|
fireEvent.click(within(confirm).getByLabelText("save and close"));
|
||||||
|
|
||||||
|
// The change is persisted and the panel closes.
|
||||||
|
await waitFor(async () => {
|
||||||
|
const fresh = await ticket.read(PROJECT_ID, t.ref);
|
||||||
|
expect(fresh.description).toBe("final body");
|
||||||
|
});
|
||||||
|
await waitFor(() => expect(onClose).toHaveBeenCalledTimes(1));
|
||||||
|
});
|
||||||
|
|
||||||
it("copies a delegation context prompt (F7)", async () => {
|
it("copies a delegation context prompt (F7)", async () => {
|
||||||
const writeText = stubClipboard();
|
const writeText = stubClipboard();
|
||||||
const t = await ticket.create(PROJECT_ID, { title: "Delegate me" });
|
const t = await ticket.create(PROJECT_ID, { title: "Delegate me" });
|
||||||
|
|||||||
@ -23,6 +23,14 @@ export interface TicketDetailViewModel {
|
|||||||
/** Set when the last write lost an optimistic-concurrency race (F3). */
|
/** Set when the last write lost an optimistic-concurrency race (F3). */
|
||||||
conflict: boolean;
|
conflict: boolean;
|
||||||
busy: boolean;
|
busy: boolean;
|
||||||
|
/**
|
||||||
|
* Monotonic counter bumped **only** when the ticket is (re)loaded from the
|
||||||
|
* gateway — initial load, an external-event refresh, or a conflict reload. It
|
||||||
|
* is deliberately **not** bumped by an ordinary optimistic mutation (e.g. a
|
||||||
|
* priority change), so a consumer can re-seed its local edit draft on genuine
|
||||||
|
* reloads without clobbering an in-progress edit on every version bump (#9).
|
||||||
|
*/
|
||||||
|
reloadCount: number;
|
||||||
refresh: () => Promise<void>;
|
refresh: () => Promise<void>;
|
||||||
updateFields: (input: {
|
updateFields: (input: {
|
||||||
title?: string;
|
title?: string;
|
||||||
@ -62,6 +70,7 @@ export function useTicketDetail(
|
|||||||
const [error, setError] = useState<string | null>(null);
|
const [error, setError] = useState<string | null>(null);
|
||||||
const [conflict, setConflict] = useState(false);
|
const [conflict, setConflict] = useState(false);
|
||||||
const [busy, setBusy] = useState(false);
|
const [busy, setBusy] = useState(false);
|
||||||
|
const [reloadCount, setReloadCount] = useState(0);
|
||||||
|
|
||||||
const refresh = useCallback(async () => {
|
const refresh = useCallback(async () => {
|
||||||
setBusy(true);
|
setBusy(true);
|
||||||
@ -69,6 +78,11 @@ export function useTicketDetail(
|
|||||||
try {
|
try {
|
||||||
setTicket(await gateway.read(projectId, ref, true));
|
setTicket(await gateway.read(projectId, ref, true));
|
||||||
setConflict(false);
|
setConflict(false);
|
||||||
|
// Signal a benign (re)load so consumers can refresh their edit draft; an
|
||||||
|
// optimistic mutation below never bumps this (#9). A benign reload leaves
|
||||||
|
// `conflict` false, so the draft is preserved field-by-field; a conflict
|
||||||
|
// reload (see `run`) keeps `conflict` true to force a clean re-apply.
|
||||||
|
setReloadCount((c) => c + 1);
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
setError(describe(e));
|
setError(describe(e));
|
||||||
} finally {
|
} finally {
|
||||||
@ -119,11 +133,19 @@ export function useTicketDetail(
|
|||||||
return true;
|
return true;
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
if (isVersionConflict(e)) {
|
if (isVersionConflict(e)) {
|
||||||
setConflict(true);
|
// Conflict reload: reload the fresh ticket but keep `conflict` true so
|
||||||
|
// the consumer force-re-seeds the draft (re-apply cleanly) rather than
|
||||||
|
// preserving the now-stale edit. Bump `reloadCount` to trigger it.
|
||||||
setError(
|
setError(
|
||||||
"This ticket was modified elsewhere. It has been reloaded — re-apply your change.",
|
"This ticket was modified elsewhere. It has been reloaded — re-apply your change.",
|
||||||
);
|
);
|
||||||
await refresh();
|
try {
|
||||||
|
setTicket(await gateway.read(projectId, ref, true));
|
||||||
|
} catch (reloadErr) {
|
||||||
|
setError(describe(reloadErr));
|
||||||
|
}
|
||||||
|
setConflict(true);
|
||||||
|
setReloadCount((c) => c + 1);
|
||||||
} else {
|
} else {
|
||||||
setError(describe(e));
|
setError(describe(e));
|
||||||
}
|
}
|
||||||
@ -132,7 +154,7 @@ export function useTicketDetail(
|
|||||||
setBusy(false);
|
setBusy(false);
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
[ticket, gateway, refresh],
|
[ticket, gateway, projectId, ref],
|
||||||
);
|
);
|
||||||
|
|
||||||
const updateFields: TicketDetailViewModel["updateFields"] = useCallback(
|
const updateFields: TicketDetailViewModel["updateFields"] = useCallback(
|
||||||
@ -169,6 +191,7 @@ export function useTicketDetail(
|
|||||||
error,
|
error,
|
||||||
conflict,
|
conflict,
|
||||||
busy,
|
busy,
|
||||||
|
reloadCount,
|
||||||
refresh,
|
refresh,
|
||||||
updateFields,
|
updateFields,
|
||||||
saveCarnet,
|
saveCarnet,
|
||||||
|
|||||||
Reference in New Issue
Block a user