From 837247b3701b73818f76969bd1c6d626e3f7c535 Mon Sep 17 00:00:00 2001 From: Thierry Date: Sun, 7 Jun 2026 00:10:28 +0200 Subject: [PATCH 1/2] fix(support): closing a ticket violated the resolved-consistency constraint [HOTFIX] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changing a support ticket to "Fermé" (closed) from the super_admin triage UI failed with "Mise à jour du statut impossible" — no ticket could ever be closed. Root cause (verified against prod schema): the DB CHECK constraint `support_tickets_resolved_consistency` treats BOTH 'resolved' AND 'closed' as terminal states that require resolved_at + resolved_by to be non-null (and null for 'open'/'in_progress'). But updateStatus only set resolved_* for 'resolved' (`isResolved`), clearing them on every other transition — so a move to 'closed' sent resolved_at=null, resolved_by=null and the UPDATE was rejected by the constraint (a generic, non-42501 error → the generic FR message). Fix: treat 'resolved' and 'closed' as terminal. Preserve the existing resolution stamp when closing an already-resolved ticket (don't clobber when/who resolved it); stamp now/self when entering a terminal state directly from open/in_progress. Added `tickets` to the callback deps to read the current row. - Reported by @thierry on prod (real ticket 21f3df1e, resolved → could not close). - 2 regression tests added (close preserves stamp; close-from-open stamps now/self). - type-check + lint clean, 17/17 support hook tests pass. No DB/RLS change. Co-Authored-By: Claude Opus 4.8 --- src/lib/hooks/useSupportTickets.ts | 19 ++++++---- src/test/support/useSupportTickets.test.ts | 40 ++++++++++++++++++++++ 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/src/lib/hooks/useSupportTickets.ts b/src/lib/hooks/useSupportTickets.ts index 62cf794..2809559 100644 --- a/src/lib/hooks/useSupportTickets.ts +++ b/src/lib/hooks/useSupportTickets.ts @@ -159,13 +159,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 = tickets.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 { @@ -194,7 +201,7 @@ export function useSupportTickets(): UseSupportTicketsResult { if (isMountedRef.current) setUpdatingId(null); } }, - [user], + [user, tickets], ); const deleteTicket = useCallback( 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' } }); From 20cec50a5b8a66c6e1ff44e787b9ba9f992675f0 Mon Sep 17 00:00:00 2001 From: Thierry Date: Sun, 7 Jun 2026 00:15:56 +0200 Subject: [PATCH 2/2] refactor(support): read current ticket via ref, not a useCallback dep (Sourcery #377) Sourcery flagged that adding `tickets` to updateStatus's dep array rebuilds the callback on every ticket change, re-rendering every TicketCard it is passed to. Mirror `tickets` into a `ticketsRef` (synced by an effect, same pattern as the existing isMountedRef/updatingIdRef) and read the current row from the ref, so deps stay [user]. Behaviour unchanged: closing an already-resolved ticket still preserves its resolution stamp. 17/17 tests pass, type-check + lint clean. Co-Authored-By: Claude Opus 4.8 --- src/lib/hooks/useSupportTickets.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/lib/hooks/useSupportTickets.ts b/src/lib/hooks/useSupportTickets.ts index 2809559..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) { @@ -168,7 +175,7 @@ export function useSupportTickets(): UseSupportTicketsResult { // already-resolved ticket; stamp now/self when entering a terminal state // directly from open/in_progress. const isTerminal = next === 'resolved' || next === 'closed'; - const current = tickets.find((t) => t.id === id); + const current = ticketsRef.current.find((t) => t.id === id); const patch = { status: next, resolved_at: isTerminal ? (current?.resolved_at ?? new Date().toISOString()) : null, @@ -201,7 +208,7 @@ export function useSupportTickets(): UseSupportTicketsResult { if (isMountedRef.current) setUpdatingId(null); } }, - [user, tickets], + [user], ); const deleteTicket = useCallback(