diff --git a/src/lib/hooks/useSupportTickets.ts b/src/lib/hooks/useSupportTickets.ts index 62cf794..5ca5512 100644 --- a/src/lib/hooks/useSupportTickets.ts +++ b/src/lib/hooks/useSupportTickets.ts @@ -119,6 +119,13 @@ export function useSupportTickets(): UseSupportTicketsResult { // away mid-flight) — same pattern as ScreenshotViewer below (code-reviewer). const isMountedRef = useRef(true); useEffect(() => () => { isMountedRef.current = false; }, []); + // Mirror `tickets` into a ref so updateStatus can read the current row (to + // preserve an existing resolution stamp when closing) WITHOUT taking `tickets` + // as a useCallback dep — that would rebuild the callback on every ticket change + // and re-render every TicketCard it's passed to (Sourcery PR #377). The ref is + // always current by the time a user-triggered updateStatus runs. + const ticketsRef = useRef([]); + useEffect(() => { ticketsRef.current = tickets; }, [tickets]); const refresh = useCallback(async () => { if (!user || !supabase) { @@ -159,13 +166,20 @@ export function useSupportTickets(): UseSupportTicketsResult { setUpdatingId(id); setError(null); - // resolved_* are only meaningful for the resolved state; clear them on any - // other transition so a re-opened ticket doesn't keep a stale resolver. - const isResolved = next === 'resolved'; + // 'resolved' AND 'closed' are BOTH terminal states: the DB constraint + // `support_tickets_resolved_consistency` requires resolved_at + resolved_by + // to be non-null for either, and null for open/in_progress. Clearing them + // on a transition to 'closed' violated the constraint → the UPDATE was + // rejected and the UI showed "Mise à jour impossible" (closing any ticket + // was broken). Preserve an existing resolution stamp when closing an + // already-resolved ticket; stamp now/self when entering a terminal state + // directly from open/in_progress. + const isTerminal = next === 'resolved' || next === 'closed'; + const current = ticketsRef.current.find((t) => t.id === id); const patch = { status: next, - resolved_at: isResolved ? new Date().toISOString() : null, - resolved_by: isResolved ? user.id : null, + resolved_at: isTerminal ? (current?.resolved_at ?? new Date().toISOString()) : null, + resolved_by: isTerminal ? (current?.resolved_by ?? user.id) : null, }; try { diff --git a/src/test/support/useSupportTickets.test.ts b/src/test/support/useSupportTickets.test.ts index d92b05f..52c4479 100644 --- a/src/test/support/useSupportTickets.test.ts +++ b/src/test/support/useSupportTickets.test.ts @@ -177,6 +177,46 @@ describe('useSupportTickets — updateStatus', () => { expect(result.current.tickets[0].resolved_at).toBeNull(); }); + it('keeps the original resolved_at + resolved_by when CLOSING an already-resolved ticket', async () => { + // Regression: 'closed' is a terminal state too. The DB constraint + // support_tickets_resolved_consistency requires resolved_at/by non-null for + // 'closed' — clearing them (old isResolved-only logic) violated it and broke + // closing any ticket. We must preserve the existing resolution stamp. + h.orderMock.mockResolvedValue({ + data: [ticket({ status: 'resolved', resolved_at: '2026-06-05T04:27:00Z', resolved_by: 'admin-9' })], + error: null, + }); + const { result } = renderHook(() => useSupportTickets()); + await waitFor(() => expect(result.current.loading).toBe(false)); + + await act(async () => { + await result.current.updateStatus('t1', 'closed'); + }); + + expect(h.updateMock).toHaveBeenCalledWith({ + status: 'closed', + resolved_at: '2026-06-05T04:27:00Z', + resolved_by: 'admin-9', + }); + expect(result.current.tickets[0].status).toBe('closed'); + expect(result.current.tickets[0].resolved_at).toBe('2026-06-05T04:27:00Z'); + }); + + it('stamps resolved_at + resolved_by when closing directly from a non-terminal state', async () => { + h.orderMock.mockResolvedValue({ data: [ticket({ status: 'open' })], error: null }); + const { result } = renderHook(() => useSupportTickets()); + await waitFor(() => expect(result.current.loading).toBe(false)); + + await act(async () => { + await result.current.updateStatus('t1', 'closed'); + }); + + const patch = h.updateMock.mock.calls[0][0] as { status: string; resolved_at: string | null; resolved_by: string | null }; + expect(patch.status).toBe('closed'); + expect(typeof patch.resolved_at).toBe('string'); + expect(patch.resolved_by).toBe('admin-1'); + }); + it('maps an RLS denial (42501) to a friendly FR error', async () => { h.orderMock.mockResolvedValue({ data: [ticket()], error: null }); h.eqMock.mockResolvedValue({ error: { code: '42501', message: 'permission denied' } });