diff --git a/src/app/components/SupportTicketsSection.tsx b/src/app/components/SupportTicketsSection.tsx index 248551f..1691a2c 100644 --- a/src/app/components/SupportTicketsSection.tsx +++ b/src/app/components/SupportTicketsSection.tsx @@ -15,7 +15,7 @@ * THI-297) so a huge image can't blow up the layout. */ import { useEffect, useMemo, useRef, useState } from 'react'; -import { CheckCircle2, ImageIcon, Inbox, Loader2 } from 'lucide-react'; +import { CheckCircle2, ImageIcon, Inbox, Loader2, Trash2 } from 'lucide-react'; import { Badge } from './ui/badge'; import { Button } from './ui/button'; @@ -37,7 +37,8 @@ import { type StatusFilter = 'all' | SupportTicketStatus; export function SupportTicketsSection() { - const { tickets, loading, error, updatingId, updateStatus } = useSupportTickets(); + const { tickets, loading, error, updatingId, deletingId, updateStatus, deleteTicket } = + useSupportTickets(); const [filter, setFilter] = useState('all'); const counts = useMemo(() => { @@ -120,7 +121,9 @@ export function SupportTicketsSection() { updateStatus(ticket.id, next)} + onDelete={() => deleteTicket(ticket.id)} /> ))} @@ -155,11 +158,14 @@ function FilterChip({ active, onClick, children }: FilterChipProps) { interface TicketCardProps { ticket: SupportTicket; updating: boolean; + deleting: boolean; onStatusChange: (next: SupportTicketStatus) => void; + onDelete: () => void; } -function TicketCard({ ticket, updating, onStatusChange }: TicketCardProps) { +function TicketCard({ ticket, updating, deleting, onStatusChange, onDelete }: TicketCardProps) { const selectId = `ticket-status-${ticket.id}`; + const [confirming, setConfirming] = useState(false); return (
@@ -220,6 +226,50 @@ function TicketCard({ ticket, updating, onStatusChange }: TicketCardProps) { {`Résolu le ${formatDate(ticket.resolved_at)}`} )} + + {/* Delete — two-step inline confirm (no native confirm dialog). super_admin + purge: spam / obsolete / leftover test rows (THI-347 volet 3, THI-334). */} +
+ {confirming ? ( + <> + Supprimer ? + + + + ) : ( + + )} +
); diff --git a/src/lib/hooks/useSupportTickets.ts b/src/lib/hooks/useSupportTickets.ts index 099f2eb..62cf794 100644 --- a/src/lib/hooks/useSupportTickets.ts +++ b/src/lib/hooks/useSupportTickets.ts @@ -48,7 +48,9 @@ export interface UseSupportTicketsResult { loading: boolean; error: Error | null; updatingId: string | null; + deletingId: string | null; updateStatus: (id: string, next: SupportTicketStatus) => Promise; + deleteTicket: (id: string) => Promise; refresh: () => Promise; } @@ -108,9 +110,15 @@ export function useSupportTickets(): UseSupportTicketsResult { const [loading, setLoading] = useState(true); const [error, setError] = useState(null); const [updatingId, setUpdatingId] = useState(null); + const [deletingId, setDeletingId] = useState(null); // Live guard against re-entrant updates: a ref reads the current value even // before a pending setUpdatingId has flushed, unlike a closure-captured state. const updatingIdRef = useRef(null); + const deletingIdRef = useRef(null); + // Guard setState against an update/delete resolving after unmount (navigation + // away mid-flight) — same pattern as ScreenshotViewer below (code-reviewer). + const isMountedRef = useRef(true); + useEffect(() => () => { isMountedRef.current = false; }, []); const refresh = useCallback(async () => { if (!user || !supabase) { @@ -143,7 +151,9 @@ export function useSupportTickets(): UseSupportTicketsResult { setError(new Error('Authentification requise')); return false; } - if (updatingIdRef.current) return false; + // Cross-guard: never run an update while a delete on any row is in flight + // (and vice versa) — avoids an UPDATE committing on a row a DELETE removed. + if (updatingIdRef.current || deletingIdRef.current) return false; updatingIdRef.current = id; setUpdatingId(id); @@ -170,18 +180,63 @@ export function useSupportTickets(): UseSupportTicketsResult { throw new Error('Mise à jour du statut impossible. Réessaye dans un instant.'); } // Optimistic local patch — the status-change audit trigger fires server-side. - setTickets((prev) => prev.map((t) => (t.id === id ? { ...t, ...patch } : t))); + if (isMountedRef.current) { + setTickets((prev) => prev.map((t) => (t.id === id ? { ...t, ...patch } : t))); + } return true; } catch (err) { - setError(err instanceof Error ? err : new Error('Mise à jour impossible')); + if (isMountedRef.current) { + setError(err instanceof Error ? err : new Error('Mise à jour impossible')); + } return false; } finally { updatingIdRef.current = null; - setUpdatingId(null); + if (isMountedRef.current) setUpdatingId(null); + } + }, + [user], + ); + + const deleteTicket = useCallback( + async (id: string): Promise => { + if (!user || !supabase) { + setError(new Error('Authentification requise')); + return false; + } + if (deletingIdRef.current || updatingIdRef.current) return false; + + deletingIdRef.current = id; + setDeletingId(id); + setError(null); + + try { + // RLS allows DELETE when the row is own OR the caller is super_admin + // (migration 033). The hook is used by the super_admin triage UI, so a + // 42501/PGRST301 here means an unexpected role drift, not a normal path. + const { error: deleteError } = await supabase + .from('support_tickets') + .delete() + .eq('id', id); + if (deleteError) { + if (deleteError.code === '42501' || deleteError.code === 'PGRST301') { + throw new Error('Action non autorisée pour ce compte.'); + } + throw new Error('Suppression impossible. Réessaye dans un instant.'); + } + if (isMountedRef.current) setTickets((prev) => prev.filter((t) => t.id !== id)); + return true; + } catch (err) { + if (isMountedRef.current) { + setError(err instanceof Error ? err : new Error('Suppression impossible')); + } + return false; + } finally { + deletingIdRef.current = null; + if (isMountedRef.current) setDeletingId(null); } }, [user], ); - return { tickets, loading, error, updatingId, updateStatus, refresh }; + return { tickets, loading, error, updatingId, deletingId, updateStatus, deleteTicket, refresh }; } diff --git a/src/test/support/SupportTicketsSection.test.tsx b/src/test/support/SupportTicketsSection.test.tsx index c6148bc..b7fee17 100644 --- a/src/test/support/SupportTicketsSection.test.tsx +++ b/src/test/support/SupportTicketsSection.test.tsx @@ -14,7 +14,9 @@ const h = vi.hoisted(() => ({ loading: false, error: null as Error | null, updatingId: null as string | null, + deletingId: null as string | null, updateStatus: vi.fn(), + deleteTicket: vi.fn(), refresh: vi.fn(), }, getFresh: vi.fn(), @@ -48,7 +50,9 @@ beforeEach(() => { h.state.loading = false; h.state.error = null; h.state.updatingId = null; + h.state.deletingId = null; h.state.updateStatus = vi.fn().mockResolvedValue(true); + h.state.deleteTicket = vi.fn().mockResolvedValue(true); h.getFresh = vi.fn(); }); @@ -162,4 +166,24 @@ describe('SupportTicketsSection', () => { render(); expect(screen.queryByText(/Résolu le/)).not.toBeInTheDocument(); }); + + it('deletes a ticket only after the inline two-step confirm', () => { + h.state.tickets = [ticket({ id: 'tdel' })]; + render(); + // Step 1 — "Supprimer" reveals the confirm but does NOT delete yet. + fireEvent.click(screen.getByRole('button', { name: /Supprimer le signalement/ })); + expect(h.state.deleteTicket).not.toHaveBeenCalled(); + // Step 2 — "Confirmer" triggers the delete. + fireEvent.click(screen.getByRole('button', { name: /Confirmer la suppression/ })); + expect(h.state.deleteTicket).toHaveBeenCalledWith('tdel'); + }); + + it('cancels the delete confirm without deleting', () => { + h.state.tickets = [ticket({ id: 'tdel' })]; + render(); + fireEvent.click(screen.getByRole('button', { name: /Supprimer le signalement/ })); + fireEvent.click(screen.getByRole('button', { name: /Annuler la suppression/ })); + expect(h.state.deleteTicket).not.toHaveBeenCalled(); + expect(screen.getByRole('button', { name: /Supprimer le signalement/ })).toBeInTheDocument(); + }); }); diff --git a/src/test/support/useSupportTickets.test.ts b/src/test/support/useSupportTickets.test.ts index 676b8d8..d92b05f 100644 --- a/src/test/support/useSupportTickets.test.ts +++ b/src/test/support/useSupportTickets.test.ts @@ -16,6 +16,8 @@ const h = vi.hoisted(() => ({ orderMock: vi.fn(), updateMock: vi.fn(), eqMock: vi.fn(), + deleteMock: vi.fn(), + deleteEqMock: vi.fn(), createSignedUrlMock: vi.fn(), auth: { user: { id: 'admin-1' } as { id: string } | null, initialized: true }, })); @@ -59,11 +61,13 @@ beforeEach(() => { vi.clearAllMocks(); h.auth.user = { id: 'admin-1' }; h.auth.initialized = true; - h.fromMock.mockReturnValue({ select: h.selectMock, update: h.updateMock }); + h.fromMock.mockReturnValue({ select: h.selectMock, update: h.updateMock, delete: h.deleteMock }); h.selectMock.mockReturnValue({ order: h.orderMock }); h.updateMock.mockReturnValue({ eq: h.eqMock }); + h.deleteMock.mockReturnValue({ eq: h.deleteEqMock }); h.orderMock.mockResolvedValue({ data: [], error: null }); h.eqMock.mockResolvedValue({ error: null }); + h.deleteEqMock.mockResolvedValue({ error: null }); h.createSignedUrlMock.mockResolvedValue({ data: { signedUrl: `${SIGN_BASE}/u1/new.png?token=fresh` }, error: null }); }); @@ -190,3 +194,37 @@ describe('useSupportTickets — updateStatus', () => { expect(result.current.tickets[0].status).toBe('open'); }); }); + +describe('useSupportTickets — deleteTicket', () => { + it('removes the ticket from the list optimistically on success', async () => { + h.orderMock.mockResolvedValue({ data: [ticket({ id: 't1' }), ticket({ id: 't2' })], error: null }); + const { result } = renderHook(() => useSupportTickets()); + await waitFor(() => expect(result.current.loading).toBe(false)); + + let ok = false; + await act(async () => { + ok = await result.current.deleteTicket('t1'); + }); + + expect(ok).toBe(true); + expect(h.deleteMock).toHaveBeenCalled(); + expect(h.deleteEqMock).toHaveBeenCalledWith('id', 't1'); + expect(result.current.tickets.map((t) => t.id)).toEqual(['t2']); + }); + + it('maps an RLS denial (42501) to a friendly error and keeps the ticket', async () => { + h.orderMock.mockResolvedValue({ data: [ticket({ id: 't1' })], error: null }); + h.deleteEqMock.mockResolvedValue({ error: { code: '42501', message: 'denied' } }); + const { result } = renderHook(() => useSupportTickets()); + await waitFor(() => expect(result.current.loading).toBe(false)); + + let ok = true; + await act(async () => { + ok = await result.current.deleteTicket('t1'); + }); + + expect(ok).toBe(false); + expect(result.current.error?.message).toContain('autorisée'); + expect(result.current.tickets).toHaveLength(1); + }); +}); diff --git a/src/test/supportTickets.integration.test.ts b/src/test/supportTickets.integration.test.ts index 6aeb2a3..576e3c8 100644 --- a/src/test/supportTickets.integration.test.ts +++ b/src/test/supportTickets.integration.test.ts @@ -437,32 +437,64 @@ describe('support_tickets — status/resolved coherence (migration 029)', () => // ─── Test 12: super_admin DELETE policy (M2 RGPD Art. 17) ──────────────────── -describe('support_tickets — super_admin DELETE policy (migration 029)', () => { - it.skipIf(SKIP)('student cannot DELETE own ticket (no user delete policy)', async () => { +describe('support_tickets — DELETE policies (029 super_admin + 033 user-own)', () => { + it.skipIf(SKIP)('student CAN delete own ticket (migration 033 — THI-334 teardown + RGPD Art. 17)', async () => { const { data: ticket } = await studentClient .from('support_tickets') .insert({ user_id: studentUserId, type: 'bug', - description: 'Ticket for student DELETE denial test.', + description: 'Ticket for student self-DELETE test.', }) .select('id') .single(); expect(ticket?.id).toBeTruthy(); const ticketId = ticket!.id; - createdTicketIds.push(ticketId); + // Not pushed to createdTicketIds — this test deletes it itself. - // Student attempts DELETE — RLS DELETE only matches super_admin. + // migration 033 added a "user delete own" policy → the owner can now erase. const { data, error } = await studentClient .from('support_tickets') .delete() .eq('id', ticketId) .select('id'); - expect(error).toBeNull(); // RLS does not throw, returns empty set. - expect(data?.length ?? 0).toBe(0); // 0 rows actually deleted. + expect(error).toBeNull(); + expect(data?.length ?? 0).toBe(1); // own row actually deleted + + const { data: verify } = await superAdminClient + .from('support_tickets') + .select('id') + .eq('id', ticketId) + .maybeSingle(); + expect(verify).toBeNull(); + }); + + it.skipIf(SKIP)("a non-owner non-admin user CANNOT delete another user's ticket (isolation)", async () => { + const { data: ticket } = await studentClient + .from('support_tickets') + .insert({ + user_id: studentUserId, + type: 'bug', + description: 'Ticket for cross-user DELETE isolation test.', + }) + .select('id') + .single(); + expect(ticket?.id).toBeTruthy(); + const ticketId = ticket!.id; + createdTicketIds.push(ticketId); + + // teacher = different user, not super_admin → matches neither DELETE policy + // (user-delete-own needs user_id = auth.uid(); super_admin-delete-all needs role). + const { data, error } = await teacherClient + .from('support_tickets') + .delete() + .eq('id', ticketId) + .select('id'); + + expect(error).toBeNull(); // RLS returns an empty set, no throw. + expect(data?.length ?? 0).toBe(0); // 0 rows deleted — isolation preserved. - // Verify via super_admin that the row still exists. const { data: verify } = await superAdminClient .from('support_tickets') .select('id') diff --git a/supabase/migrations/033_support_tickets_delete_policies.sql b/supabase/migrations/033_support_tickets_delete_policies.sql new file mode 100644 index 0000000..def2eba --- /dev/null +++ b/supabase/migrations/033_support_tickets_delete_policies.sql @@ -0,0 +1,48 @@ +-- Migration 033 — support_tickets DELETE policies +-- THI-347 volet 3 (super_admin purge) + fix THI-334 root cause (test teardown). +-- +-- Migration 028 created the table; migration 029 (security hardening) already added +-- the `super_admin delete all` DELETE policy + `grant delete to authenticated`. What +-- was STILL missing: a `user delete own` DELETE policy — the ONLY genuinely new policy +-- this migration adds. Two consequences of its absence: +-- 1. THI-347 volet 3 : super_admin could already delete via REST (029) but the +-- triage UI had no delete button — this PR adds the button (backend was ready). +-- 2. THI-334 root cause : the support integration tests' afterAll teardown +-- (`client.from('support_tickets').delete().in('id', createdIds)`, run as the +-- test student) silently failed — no `user delete own` policy → test rows piled +-- up in prod and fired a super_admin email on each `vitest` run. +-- +-- This migration adds `user delete own` and re-states (drop+recreate, idempotent) +-- `super_admin delete all` from 029 so the file is self-contained on a fresh env. +-- RLS OR-combines DELETE policies → a row is deletable if owned OR caller super_admin. +-- - user delete own → unblocks the test teardown (student deletes its own +-- rows) AND gives users the RGPD Art.17 right to erase their own reports. +-- - super_admin delete all → triage purge (THI-347 volet 3) — origin: migration 029. +-- +-- Security : both predicates are self-scoped (auth.uid()) or role-gated +-- (get_my_role()); no cross-user surface. `anon` stays revoked (migration 028 +-- line 65 `revoke all ... from anon`). Deleting a row does NOT remove its storage +-- screenshot object — orphan cleanup is deferred (bucket is private + super_admin +-- read-only, so an orphan is inert; tracked for a later storage GC pass). + +grant delete on public.support_tickets to authenticated; + +-- drop-if-exists keeps this migration re-appliable (idempotent) on a fresh env. +drop policy if exists "support_tickets: user delete own" on public.support_tickets; +create policy "support_tickets: user delete own" + on public.support_tickets + for delete + to authenticated + using (user_id = auth.uid()); + +drop policy if exists "support_tickets: super_admin delete all" on public.support_tickets; +create policy "support_tickets: super_admin delete all" + on public.support_tickets + for delete + to authenticated + using (public.get_my_role() = 'super_admin'); + +comment on policy "support_tickets: user delete own" on public.support_tickets is + 'THI-347/THI-334. User erases own report (RGPD Art.17) + unblocks integration-test teardown DELETE.'; +comment on policy "support_tickets: super_admin delete all" on public.support_tickets is + 'THI-347 volet 3. super_admin purges any ticket from the triage UI (spam/obsolete/test rows).';