Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 53 additions & 3 deletions src/app/components/SupportTicketsSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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<StatusFilter>('all');

const counts = useMemo(() => {
Expand Down Expand Up @@ -120,7 +121,9 @@ export function SupportTicketsSection() {
<TicketCard
ticket={ticket}
updating={updatingId === ticket.id}
deleting={deletingId === ticket.id}
onStatusChange={(next) => updateStatus(ticket.id, next)}
onDelete={() => deleteTicket(ticket.id)}
/>
</li>
))}
Expand Down Expand Up @@ -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 (
<Card variant="tl-surface" className="px-5 py-4 gap-3">
<div className="flex flex-wrap items-start justify-between gap-3">
Expand Down Expand Up @@ -220,6 +226,50 @@ function TicketCard({ ticket, updating, onStatusChange }: TicketCardProps) {
{`Résolu le ${formatDate(ticket.resolved_at)}`}
</span>
)}

{/* Delete — two-step inline confirm (no native confirm dialog). super_admin
purge: spam / obsolete / leftover test rows (THI-347 volet 3, THI-334). */}
<div className="ml-auto flex items-center gap-2">
{confirming ? (
<>
<span className="text-xs text-[var(--github-text-secondary)] font-mono">Supprimer ?</span>
<Button
type="button"
variant="ghost-gh"
size="sm"
className="min-h-11 font-mono text-xs text-[var(--github-red)] hover:text-[var(--github-red)] hover:bg-[var(--github-red)]/10"
onClick={onDelete}
disabled={deleting}
aria-label={`Confirmer la suppression du signalement #${ticket.id.slice(0, 8)}`}
>
{deleting ? 'Suppression…' : 'Confirmer'}
</Button>
<Button
type="button"
variant="ghost-gh"
size="sm"
className="min-h-11 font-mono text-xs"
onClick={() => setConfirming(false)}
disabled={deleting}
aria-label={`Annuler la suppression du signalement #${ticket.id.slice(0, 8)}`}
>
Annuler
</Button>
</>
) : (
<Button
type="button"
variant="ghost-gh"
size="sm"
className="min-h-11 gap-1.5 font-mono text-xs text-[var(--github-text-secondary)] hover:text-[var(--github-red)]"
onClick={() => setConfirming(true)}
aria-label={`Supprimer le signalement #${ticket.id.slice(0, 8)}`}
>
<Trash2 size={14} aria-hidden="true" />
Supprimer
</Button>
)}
</div>
</div>
</Card>
);
Expand Down
65 changes: 60 additions & 5 deletions src/lib/hooks/useSupportTickets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ export interface UseSupportTicketsResult {
loading: boolean;
error: Error | null;
updatingId: string | null;
deletingId: string | null;
updateStatus: (id: string, next: SupportTicketStatus) => Promise<boolean>;
deleteTicket: (id: string) => Promise<boolean>;
refresh: () => Promise<void>;
}

Expand Down Expand Up @@ -108,9 +110,15 @@ export function useSupportTickets(): UseSupportTicketsResult {
const [loading, setLoading] = useState(true);
const [error, setError] = useState<Error | null>(null);
const [updatingId, setUpdatingId] = useState<string | null>(null);
const [deletingId, setDeletingId] = useState<string | null>(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<string | null>(null);
const deletingIdRef = useRef<string | null>(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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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<boolean> => {
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 };
}
24 changes: 24 additions & 0 deletions src/test/support/SupportTicketsSection.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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();
});

Expand Down Expand Up @@ -162,4 +166,24 @@ describe('SupportTicketsSection', () => {
render(<SupportTicketsSection />);
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(<SupportTicketsSection />);
// 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(<SupportTicketsSection />);
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();
});
});
40 changes: 39 additions & 1 deletion src/test/support/useSupportTickets.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
}));
Expand Down Expand Up @@ -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 });
});

Expand Down Expand Up @@ -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);
});
});
48 changes: 40 additions & 8 deletions src/test/supportTickets.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
Loading
Loading