Skip to content
Draft
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
5 changes: 4 additions & 1 deletion apps/desktop/src/renderer/src/components/issue-window.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,10 @@ function IssueWindowRoute() {
<IssueWindowNavigationProvider>
<WorkspacePresencePrefetch />
<IssueWindowFrame>
<IssueDetailPage onDelete={() => window.desktopAPI.closeWindow()} />
<IssueDetailPage
onDelete={() => window.desktopAPI.closeWindow()}
onDone={() => window.desktopAPI.closeWindow()}
/>
</IssueWindowFrame>
<ModalRegistry />
</IssueWindowNavigationProvider>
Expand Down
4 changes: 2 additions & 2 deletions apps/desktop/src/renderer/src/pages/issue-detail-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 <IssueDetailRoute routeId={id} onDelete={onDelete} />;
return <IssueDetailRoute routeId={id} onDelete={onDelete} onDone={onDone} />;
}
4 changes: 3 additions & 1 deletion packages/views/issues/components/issue-detail-route.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ interface IssueDetailRouteProps {
*/
routeId: string;
onDelete?: () => void;
onDone?: () => void;
}

/**
Expand Down Expand Up @@ -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();
Expand All @@ -93,6 +94,7 @@ export function IssueDetailRoute({ routeId, onDelete }: IssueDetailRouteProps) {
<IssueDetail
issueId={canonicalId}
onDelete={onDelete}
onDone={onDone}
highlightCommentId={highlight.commentId}
/>
);
Expand Down
50 changes: 47 additions & 3 deletions packages/views/issues/components/issue-detail.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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) => (
Expand All @@ -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,
}));

Expand Down Expand Up @@ -630,12 +632,12 @@ function createTestQueryClient() {
});
}

function renderIssueDetail(issueId = "issue-1") {
function renderIssueDetail(issueId = "issue-1", onDone?: () => void) {
const queryClient = createTestQueryClient();
return render(
<I18nProvider locale="en" resources={TEST_RESOURCES}>
<QueryClientProvider client={queryClient}>
<IssueDetail issueId={issueId} />
<IssueDetail issueId={issueId} onDone={onDone} />
</QueryClientProvider>
</I18nProvider>,
);
Expand Down Expand Up @@ -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([]);
Expand All @@ -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<Issue>((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>): Attachment => ({
id,
Expand Down
44 changes: 27 additions & 17 deletions packages/views/issues/components/issue-detail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
ChevronLeft,
ChevronRight,
CircleCheck,
LoaderCircle,
Milestone,
MoreHorizontal,
PanelRight,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -3164,23 +3180,17 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr
It self-hides when no agent is active. */}
<IssueAgentHeaderChip issueId={id} />
<IssueWakeupHeaderChip issueId={id} onOpen={openWakeups} />
{onDone && !issueBehavesAsAny(issue, ["done", "closed"]) && (
<Tooltip>
<TooltipTrigger
render={
<Button
variant="ghost"
size="icon-sm"
className="text-muted-foreground"
onClick={() => { handleUpdateField({ status: "done" }); onDone?.(); }}
>
<CircleCheck />
</Button>
}
/>
<TooltipContent side="bottom">{t(($) => $.detail.mark_done_tooltip)}</TooltipContent>
</Tooltip>
)}
<Button
variant="outline"
size="sm"
className="border-success/40 text-success hover:text-success"
disabled={completingIssue}
aria-busy={completingIssue}
onClick={completeAndClose}
>
{completingIssue ? <LoaderCircle className="animate-spin" /> : <CircleCheck />}
{t(($) => $.detail.mark_done_tooltip)}
</Button>
{onDone && issueBehavesAs(issue, "done") && (
<Tooltip>
<TooltipTrigger
Expand Down
11 changes: 11 additions & 0 deletions packages/views/issues/components/issue-peek.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@ vi.mock("./issue-detail", () => ({
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 (
Expand All @@ -38,6 +40,7 @@ vi.mock("./issue-detail", () => ({
<button type="button" onClick={onDelete}>
delete
</button>
<button type="button" onClick={onDone}>done</button>
</div>
);
},
Expand Down Expand Up @@ -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");
Expand Down
1 change: 1 addition & 0 deletions packages/views/issues/components/issue-peek.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ const IssuePeekPanel = memo(function IssuePeekPanel({ issueId }: { issueId: stri
leadingAction={<IssuePeekNav />}
trailingActions={<IssuePeekTrailingActions issueId={issueId} />}
onDelete={actions.close}
onDone={actions.close}
/>
</ErrorBoundary>
</motion.aside>
Expand Down
Loading