From 699c0ed17dfc37be46014b319d88d8454ca8c0e7 Mon Sep 17 00:00:00 2001 From: Simon Schulte Date: Tue, 9 Jun 2026 14:20:33 +0200 Subject: [PATCH] Fix discover ticket modal add flow --- web/src/App.tsx | 74 ++++++++++++++----- web/src/DiscoverTicketsModal.tsx | 19 ++++- web/src/__tests__/App.test.tsx | 66 +++++++++++++++++ .../__tests__/DiscoverTicketsModal.test.tsx | 13 ++++ 4 files changed, 151 insertions(+), 21 deletions(-) diff --git a/web/src/App.tsx b/web/src/App.tsx index 20407e9..4a9fcdf 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -582,18 +582,46 @@ export function App() { }, 250); } - async function queueAction(fn: () => Promise): Promise { + async function queueAction( + fn: () => Promise, + onError?: (message: string) => void + ): Promise { setError(""); + if (onError) { + onError(""); + } try { const accepted = await fn(); optimisticTicketJob(accepted); return accepted; } catch (err) { - setError(err instanceof Error ? err.message : "action failed"); + const message = err instanceof Error ? err.message : "action failed"; + if (onError) { + onError(message); + } else { + setError(message); + } return null; } } + function isPendingAdd(repoPath: string, ticketNumber: string): boolean { + return pendingAddedTickets.includes(pendingTicketKey(repoPath, ticketNumber)); + } + + async function enqueueTicketRun( + repoPath: string, + ticketNumber: string, + baseBranch?: string, + onError?: (message: string) => void + ): Promise { + const pendingKey = pendingTicketKey(repoPath, ticketNumber); + setPendingAddedTickets((current) => [...current, pendingKey]); + const accepted = await queueAction(() => runTicket(repoPath, ticketNumber, baseBranch), onError); + setPendingAddedTickets((current) => current.filter((key) => key !== pendingKey)); + return accepted; + } + async function submitAddTicket() { const repoPath = newTicketRepoPath.trim(); const ticketNumber = newTicketNumber.trim(); @@ -604,8 +632,7 @@ export function App() { return; } - const pendingKey = pendingTicketKey(repoPath, ticketNumber); - if (pendingAddedTickets.includes(pendingKey)) { + if (isPendingAdd(repoPath, ticketNumber)) { setAddTicketError(`ticket ${ticketNumber} is already being added to AutoPR for this repository`); return; } @@ -622,9 +649,7 @@ export function App() { return; } - setPendingAddedTickets((current) => [...current, pendingKey]); - const accepted = await queueAction(() => runTicket(repoPath, ticketNumber, baseBranch)); - setPendingAddedTickets((current) => current.filter((key) => key !== pendingKey)); + const accepted = await enqueueTicketRun(repoPath, ticketNumber, baseBranch); if (accepted) { closeAddTicketDialog(); scheduleFullRefresh(); @@ -758,20 +783,30 @@ export function App() { setDiscoverError(""); setDiscoverLoading(true); setShowDiscoverModal(true); - void discoverTickets(repoPath) - .then((found) => { setDiscoveredTickets(found); }) + void Promise.all([discoverTickets(repoPath), listTickets(repoPath)]) + .then(([found, tracked]) => { + const trackedNumbers = new Set(tracked.map((ticket) => ticket.ticket_number)); + setDiscoveredTickets( + found.filter((ticket) => !trackedNumbers.has(ticket.ticket_number) && !isPendingAdd(repoPath, ticket.ticket_number)) + ); + }) .catch((err) => { setDiscoverError(err instanceof Error ? err.message : "discovery failed"); }) .finally(() => { setDiscoverLoading(false); }); } - function handleDiscoverAdd(ticketNumber: string) { - setShowDiscoverModal(false); - setError(""); - setAddTicketError(""); - setNewTicketRepoPath(discoverRepoPath); - setNewTicketNumber(ticketNumber); - setShowAddTicketDialog(true); - void refreshRepositories(); + async function handleDiscoverAdd(ticketNumber: string) { + if (!discoverRepoPath) { + return; + } + if (isPendingAdd(discoverRepoPath, ticketNumber)) { + setDiscoverError(`ticket ${ticketNumber} is already being added to AutoPR for this repository`); + return; + } + const ok = await enqueueTicketRun(discoverRepoPath, ticketNumber, undefined, setDiscoverError); + if (ok) { + setDiscoveredTickets((current) => current.filter((ticket) => ticket.ticket_number !== ticketNumber)); + scheduleFullRefresh(); + } } return ( @@ -865,7 +900,10 @@ export function App() { tickets={discoveredTickets} loading={discoverLoading} error={discoverError} - onAdd={handleDiscoverAdd} + pendingTicketNumbers={discoveredTickets + .filter((ticket) => isPendingAdd(discoverRepoPath, ticket.ticket_number)) + .map((ticket) => ticket.ticket_number)} + onAdd={(ticketNumber) => { void handleDiscoverAdd(ticketNumber); }} onClose={() => setShowDiscoverModal(false)} /> ) : null} diff --git a/web/src/DiscoverTicketsModal.tsx b/web/src/DiscoverTicketsModal.tsx index 7c7825f..bcde13e 100644 --- a/web/src/DiscoverTicketsModal.tsx +++ b/web/src/DiscoverTicketsModal.tsx @@ -5,11 +5,20 @@ type DiscoverTicketsModalProps = { tickets: DiscoveredTicket[]; loading: boolean; error: string; + pendingTicketNumbers: string[]; onAdd: (ticketNumber: string) => void; onClose: () => void; }; -export function DiscoverTicketsModal({ repoPath, tickets, loading, error, onAdd, onClose }: DiscoverTicketsModalProps) { +export function DiscoverTicketsModal({ + repoPath, + tickets, + loading, + error, + pendingTicketNumbers, + onAdd, + onClose +}: DiscoverTicketsModalProps) { return (
event.stopPropagation()}> @@ -26,8 +35,12 @@ export function DiscoverTicketsModal({ repoPath, tickets, loading, error, onAdd,
  • {ticket.ticket_number} {ticket.title} -
    • ))} diff --git a/web/src/__tests__/App.test.tsx b/web/src/__tests__/App.test.tsx index 39609ec..ce1ba0a 100644 --- a/web/src/__tests__/App.test.tsx +++ b/web/src/__tests__/App.test.tsx @@ -356,3 +356,69 @@ describe("App", () => { }); }); }); + +describe("App discover tickets", () => { + it("filters tracked tickets and keeps the discover modal open after adding", async () => { + apiMocks.getHealth.mockResolvedValue({ discover_tickets_configured: true }); + apiMocks.listTickets.mockImplementation(async (repoPath?: string) => { + if (repoPath === "/repo1") { + return [ + { + repo_id: "repo1", + repo_path: "/repo1", + ticket_number: "GH-4", + title: "Already tracked", + status: "waiting", + busy: false, + approved: false, + updated_at: "2024-01-01T00:00:00Z" + } + ]; + } + return [ + { + repo_id: "repo1", + repo_path: "/repo1", + ticket_number: "GH-5", + title: "Structured feedback", + status: "waiting", + busy: false, + approved: false, + updated_at: "2024-01-01T00:00:00Z" + } + ]; + }); + apiMocks.discoverTickets.mockResolvedValue([ + { ticket_number: "GH-4", title: "Already tracked" }, + { ticket_number: "GH-11", title: "Fresh discover ticket" } + ]); + apiMocks.runTicket.mockResolvedValue({ + status: "accepted", + job_id: "job-2", + action: "run_ticket", + repo_id: "repo1", + repo_path: "/repo1" + }); + + render(); + + fireEvent.click(await screen.findByRole("button", { name: "Discover Tickets" })); + + expect(await screen.findByText("Fresh discover ticket")).toBeInTheDocument(); + expect(apiMocks.discoverTickets).toHaveBeenCalledWith("/repo1"); + expect(apiMocks.listTickets).toHaveBeenCalledWith("/repo1"); + expect(screen.queryByText("Already tracked")).not.toBeInTheDocument(); + + fireEvent.click(screen.getByRole("button", { name: "Add" })); + + await waitFor(() => { + expect(apiMocks.runTicket).toHaveBeenCalledWith("/repo1", "GH-11", undefined); + }); + await waitFor(() => { + expect(screen.queryByText("Fresh discover ticket")).not.toBeInTheDocument(); + }); + + expect(screen.getByRole("button", { name: "Close" })).toBeInTheDocument(); + expect(screen.queryByText("Schedule a ticket run for a repository.")).not.toBeInTheDocument(); + }); +}); diff --git a/web/src/__tests__/DiscoverTicketsModal.test.tsx b/web/src/__tests__/DiscoverTicketsModal.test.tsx index c8ca14e..1fc7e3e 100644 --- a/web/src/__tests__/DiscoverTicketsModal.test.tsx +++ b/web/src/__tests__/DiscoverTicketsModal.test.tsx @@ -7,6 +7,7 @@ const defaultProps = { tickets: [], loading: false, error: "", + pendingTicketNumbers: [], onAdd: vi.fn(), onClose: vi.fn(), }; @@ -46,6 +47,18 @@ describe("DiscoverTicketsModal", () => { expect(onAdd).toHaveBeenCalledWith("GH-4"); }); + it("disables pending add buttons", () => { + render( + + ); + + expect(screen.getByRole("button", { name: "Adding..." })).toBeDisabled(); + }); + it("calls onClose when the backdrop is clicked", () => { const onClose = vi.fn(); const { container } = render();