diff --git a/apps/desktop/src/renderer/src/components/issue-window.tsx b/apps/desktop/src/renderer/src/components/issue-window.tsx index 9719f366c17..c3445607403 100644 --- a/apps/desktop/src/renderer/src/components/issue-window.tsx +++ b/apps/desktop/src/renderer/src/components/issue-window.tsx @@ -82,7 +82,10 @@ function IssueWindowRoute() { - window.desktopAPI.closeWindow()} /> + window.desktopAPI.closeWindow()} + onDone={() => window.desktopAPI.closeWindow()} + /> diff --git a/apps/desktop/src/renderer/src/pages/issue-detail-page.tsx b/apps/desktop/src/renderer/src/pages/issue-detail-page.tsx index 81e34992ca9..ec8af16c405 100644 --- a/apps/desktop/src/renderer/src/pages/issue-detail-page.tsx +++ b/apps/desktop/src/renderer/src/pages/issue-detail-page.tsx @@ -4,7 +4,7 @@ import { useWorkspaceId } from "@multica/core/hooks"; import { useCanonicalIssue } from "@multica/core/issues/canonical-id"; import { useDocumentTitle } from "@/hooks/use-document-title"; -export function IssueDetailPage({ onDelete }: { onDelete?: () => void }) { +export function IssueDetailPage({ onDelete, onDone }: { onDelete?: () => void; onDone?: () => void }) { const { id } = useParams<{ id: string }>(); const wsId = useWorkspaceId(); // `id` may be an identifier (`MUL-123`); resolving here means the title @@ -18,5 +18,5 @@ export function IssueDetailPage({ onDelete }: { onDelete?: () => void }) { // Render errors bubble to the root route errorElement (DesktopRouteErrorPage), // which contains the crash inside the tab content pane. No page-level boundary // here — a whole-page wrapper duplicates the route-level error UI. - return ; + return ; } diff --git a/packages/views/issues/components/issue-detail-route.tsx b/packages/views/issues/components/issue-detail-route.tsx index a275dbb78d6..394742ffac2 100644 --- a/packages/views/issues/components/issue-detail-route.tsx +++ b/packages/views/issues/components/issue-detail-route.tsx @@ -15,6 +15,7 @@ interface IssueDetailRouteProps { */ routeId: string; onDelete?: () => void; + onDone?: () => void; } /** @@ -74,7 +75,7 @@ function useCommentHighlightHash(): { hash: string; commentId?: string } { * the route and only the route — the inbox renders `IssueDetail` in a side * panel, where replacing the URL would navigate the user out of the inbox. */ -export function IssueDetailRoute({ routeId, onDelete }: IssueDetailRouteProps) { +export function IssueDetailRoute({ routeId, onDelete, onDone }: IssueDetailRouteProps) { const wsId = useWorkspaceId(); const { canonicalId, issue, isResolving, notFound } = useCanonicalIssue(wsId, routeId); const highlight = useCommentHighlightHash(); @@ -93,6 +94,7 @@ export function IssueDetailRoute({ routeId, onDelete }: IssueDetailRouteProps) { ); diff --git a/packages/views/issues/components/issue-detail.test.tsx b/packages/views/issues/components/issue-detail.test.tsx index 91123c36bf0..e7b7697a060 100644 --- a/packages/views/issues/components/issue-detail.test.tsx +++ b/packages/views/issues/components/issue-detail.test.tsx @@ -116,6 +116,8 @@ vi.mock("@multica/core/paths", async () => { }; }); +const mockBackOrReplace = vi.hoisted(() => vi.fn()); + // Mock navigation vi.mock("../../navigation", () => ({ AppLink: ({ children, href, ...props }: any) => ( @@ -128,7 +130,7 @@ vi.mock("../../navigation", () => ({ pathname: "/issues/issue-1", getShareableUrl: (p: string) => `https://app.multica.com${p}`, }), - useBackOrReplace: () => vi.fn(), + useBackOrReplace: () => mockBackOrReplace, NavigationProvider: ({ children }: { children: React.ReactNode }) => children, })); @@ -630,12 +632,12 @@ function createTestQueryClient() { }); } -function renderIssueDetail(issueId = "issue-1") { +function renderIssueDetail(issueId = "issue-1", onDone?: () => void) { const queryClient = createTestQueryClient(); return render( - + , ); @@ -715,6 +717,7 @@ describe("IssueDetail (shared)", () => { mockViewport.isMobile = false; // Default: issue loads successfully mockApiObj.getIssue.mockResolvedValue(mockIssue); + mockApiObj.updateIssue.mockReset().mockResolvedValue(mockIssue); // /timeline returns the entries flat in chronological order (oldest first). mockApiObj.listTimeline.mockResolvedValue(mockTimeline); mockApiObj.listIssueReactions.mockResolvedValue([]); @@ -738,6 +741,47 @@ describe("IssueDetail (shared)", () => { mockApiObj.getProject.mockReset(); }); + it("shows Done on a regular detail page and closes only after saving", async () => { + let finish!: (issue: Issue) => void; + mockApiObj.updateIssue.mockReturnValue(new Promise((resolve) => { finish = resolve; })); + renderIssueDetail(); + const button = await screen.findByRole("button", { name: "Mark as done" }); + fireEvent.click(button); + await waitFor(() => expect(mockApiObj.updateIssue).toHaveBeenCalled()); + expect(mockApiObj.updateIssue.mock.calls[0]?.[1]).toMatchObject({ status: "done" }); + expect(button).toBeDisabled(); + expect(mockBackOrReplace).not.toHaveBeenCalled(); + await act(async () => { finish({ ...mockIssue, status: "done" }); }); + await waitFor(() => expect(mockBackOrReplace).toHaveBeenCalledWith("/test/issues")); + }); + + it("closes its hosting card after saving Done", async () => { + const onDone = vi.fn(); + mockApiObj.updateIssue.mockResolvedValue({ ...mockIssue, status: "done" }); + renderIssueDetail("issue-1", onDone); + fireEvent.click(await screen.findByRole("button", { name: "Mark as done" })); + await waitFor(() => expect(onDone).toHaveBeenCalledOnce()); + expect(mockBackOrReplace).not.toHaveBeenCalled(); + }); + + it("closes an already completed issue without writing its status again", async () => { + mockApiObj.getIssue.mockResolvedValue({ ...mockIssue, status: "done" }); + renderIssueDetail(); + fireEvent.click(await screen.findByRole("button", { name: "Mark as done" })); + expect(mockApiObj.updateIssue).not.toHaveBeenCalled(); + expect(mockBackOrReplace).toHaveBeenCalledWith("/test/issues"); + }); + + it("keeps the card open when marking Done fails", async () => { + mockApiObj.updateIssue.mockRejectedValue(new Error("Save failed")); + renderIssueDetail(); + const button = await screen.findByRole("button", { name: "Mark as done" }); + fireEvent.click(button); + await waitFor(() => expect(toast.error).toHaveBeenCalledWith("Save failed")); + expect(mockBackOrReplace).not.toHaveBeenCalled(); + await waitFor(() => expect(button).not.toBeDisabled()); + }); + it("counts comment files as deliverables, a re-upload once as v2, never the description's (MUL-7649)", async () => { const file = (id: string, over: Partial): Attachment => ({ id, diff --git a/packages/views/issues/components/issue-detail.tsx b/packages/views/issues/components/issue-detail.tsx index 0dd3b1caa54..163ab0b95aa 100644 --- a/packages/views/issues/components/issue-detail.tsx +++ b/packages/views/issues/components/issue-detail.tsx @@ -22,6 +22,7 @@ import { ChevronLeft, ChevronRight, CircleCheck, + LoaderCircle, Milestone, MoreHorizontal, PanelRight, @@ -2439,6 +2440,21 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr // Called before the `if (!issue)` early return so hook order stays stable. const actions = useIssueActions(issue); const handleUpdateField = actions.updateField; + const backOrReplace = useBackOrReplace(); + const [completingIssue, setCompletingIssue] = useState(false); + const completeAndClose = () => { + if (!issue || completingIssue) return; + const close = () => onDone ? onDone() : backOrReplace(paths.issues()); + if (issueBehavesAs(issue, "done")) { + close(); + return; + } + setCompletingIssue(true); + handleUpdateField({ status: "done" }, { + onSuccess: close, + onSettled: () => setCompletingIssue(false), + }); + }; // Labels live in their own query (not on the issue body) — fetch the count // here so seeding can decide whether the "Labels" optional row should be @@ -3164,23 +3180,17 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr It self-hides when no agent is active. */} - {onDone && !issueBehavesAsAny(issue, ["done", "closed"]) && ( - - { handleUpdateField({ status: "done" }); onDone?.(); }} - > - - - } - /> - {t(($) => $.detail.mark_done_tooltip)} - - )} + {onDone && issueBehavesAs(issue, "done") && ( ({ leadingAction, trailingActions, onDelete, + onDone, }: { issueId: string; variant: string; leadingAction: ReactNode; trailingActions: ReactNode; onDelete: () => void; + onDone: () => void; }) => { if (issueId === "boom") throw new Error("Could not render this issue"); return ( @@ -38,6 +40,7 @@ vi.mock("./issue-detail", () => ({ + ); }, @@ -192,6 +195,14 @@ describe("IssuePeekHost", () => { editor.remove(); }); + it("closes the preview when the detail finishes marking Done", async () => { + renderHost(); + openCard("i-1"); + fireEvent.click(screen.getByRole("button", { name: "done" })); + await waitForClosed(); + expect(navigation.replace).not.toHaveBeenCalled(); + }); + it("closes from the close button and when the issue is deleted", async () => { renderHost(); openCard("i-1"); diff --git a/packages/views/issues/components/issue-peek.tsx b/packages/views/issues/components/issue-peek.tsx index 685ffdbfa5e..0972ca23507 100644 --- a/packages/views/issues/components/issue-peek.tsx +++ b/packages/views/issues/components/issue-peek.tsx @@ -193,6 +193,7 @@ const IssuePeekPanel = memo(function IssuePeekPanel({ issueId }: { issueId: stri leadingAction={} trailingActions={} onDelete={actions.close} + onDone={actions.close} />