diff --git a/frontend/src/features/tickets/TicketDetail.tsx b/frontend/src/features/tickets/TicketDetail.tsx index bd7f5ed..ae9c349 100644 --- a/frontend/src/features/tickets/TicketDetail.tsx +++ b/frontend/src/features/tickets/TicketDetail.tsx @@ -9,7 +9,7 @@ * 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 { Button, Input, Spinner, cn } from "@/shared"; @@ -88,8 +88,12 @@ export function TicketDetail({ const { agents, nameOf } = useProjectAgents(projectId); const t = vm.ticket; - // Local draft state, re-seeded whenever the loaded ticket version changes so a - // conflict-reload or an external edit refreshes the fields. + // Local draft state. Re-seeded only on a genuine (re)load of the ticket — + // 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 [description, setDescription] = useState(""); const [carnet, setCarnet] = useState(""); @@ -97,20 +101,71 @@ export function TicketDetail({ const [linkKind, setLinkKind] = useState("relatesTo"); const [assignPick, setAssignPick] = useState(""); 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(() => { if (!t) return; - setTitle(t.title); - setDescription(t.description); - setCarnet(t.carnet ?? ""); - }, [t, version]); + // 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); + setDescription(t.description); + setCarnet(nextCarnet); + } 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 = !!t && (title !== t.title || description !== t.description); const dirtyCarnet = !!t && carnet !== (t.carnet ?? ""); + const dirty = dirtyFields || dirtyCarnet; 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() { if (!t) return; const ok = await copyToClipboard( @@ -149,7 +204,7 @@ export function TicketDetail({ > {copied ? "Copied" : "Delegation context"} - @@ -445,6 +500,56 @@ export function TicketDetail({ )} + + {/* ── Unsaved-changes confirmation on close (#9) ── */} + {confirmClose && ( +
+
+

+ Modifications non sauvegardées +

+

+ Vous avez des modifications non sauvegardées. Voulez-vous les + sauvegarder avant de fermer ? +

+
+ + + +
+
+
+ )} ); } diff --git a/frontend/src/features/tickets/tickets.test.tsx b/frontend/src/features/tickets/tickets.test.tsx index 7b1663f..8a5db37 100644 --- a/frontend/src/features/tickets/tickets.test.tsx +++ b/frontend/src/features/tickets/tickets.test.tsx @@ -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( + + {}} + /> + , + ); + + 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( + + {}} + /> + , + ); + + 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 () => { const writeText = stubClipboard(); const t = await ticket.create(PROJECT_ID, { title: "Delegate me" }); diff --git a/frontend/src/features/tickets/useTicketDetail.ts b/frontend/src/features/tickets/useTicketDetail.ts index 3c7efef..781bf23 100644 --- a/frontend/src/features/tickets/useTicketDetail.ts +++ b/frontend/src/features/tickets/useTicketDetail.ts @@ -23,6 +23,14 @@ export interface TicketDetailViewModel { /** Set when the last write lost an optimistic-concurrency race (F3). */ conflict: 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; updateFields: (input: { title?: string; @@ -62,6 +70,7 @@ export function useTicketDetail( const [error, setError] = useState(null); const [conflict, setConflict] = useState(false); const [busy, setBusy] = useState(false); + const [reloadCount, setReloadCount] = useState(0); const refresh = useCallback(async () => { setBusy(true); @@ -69,6 +78,11 @@ export function useTicketDetail( try { setTicket(await gateway.read(projectId, ref, true)); 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) { setError(describe(e)); } finally { @@ -119,11 +133,19 @@ export function useTicketDetail( return true; } catch (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( "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 { setError(describe(e)); } @@ -132,7 +154,7 @@ export function useTicketDetail( setBusy(false); } }, - [ticket, gateway, refresh], + [ticket, gateway, projectId, ref], ); const updateFields: TicketDetailViewModel["updateFields"] = useCallback( @@ -169,6 +191,7 @@ export function useTicketDetail( error, conflict, busy, + reloadCount, refresh, updateFields, saveCarnet,