diff --git a/packages/views/issues/components/comment-card.tsx b/packages/views/issues/components/comment-card.tsx index 33a0e5908a..653a71c548 100644 --- a/packages/views/issues/components/comment-card.tsx +++ b/packages/views/issues/components/comment-card.tsx @@ -394,11 +394,19 @@ export function useEditAttachmentState( // remount (desktop tab switch / virtualization) can restore them via // startEdit — only into an existing content draft, always writing the // current value (incl. `[]`). See CommentInput for the rationale. + // + // `content` is a dependency so this re-runs when the (debounced) content + // draft is first created: an upload result sets `pendingAttachments` before + // that flush, so `pendingAttachments` alone would evaluate the guard while no + // draft exists yet and never persist the attachment. The draft-existence + // guard also keeps startEdit (content = the comment body, no draft yet) from + // fabricating a content-less draft. useEffect(() => { if (!editing) return; + if (content.trim().length === 0) return; if (useCommentDraftStore.getState().getDraft(draftKey) === undefined) return; setDraft(draftKey, { attachments: pendingAttachments }); - }, [editing, pendingAttachments, draftKey, setDraft]); + }, [editing, content, pendingAttachments, draftKey, setDraft]); useEffect(() => { if (!editing) return; diff --git a/packages/views/issues/components/comment-draft-persistence.test.tsx b/packages/views/issues/components/comment-draft-persistence.test.tsx index 9b69714bd4..a49fdc9605 100644 --- a/packages/views/issues/components/comment-draft-persistence.test.tsx +++ b/packages/views/issues/components/comment-draft-persistence.test.tsx @@ -10,12 +10,14 @@ import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import type { Attachment, TimelineEntry } from "@multica/core/types"; import { useCommentDraftStore } from "@multica/core/issues/stores"; import { CommentInput } from "./comment-input"; +import { ReplyInput } from "./reply-input"; import { useEditAttachmentState } from "./comment-card"; // One controllable trigger agent so a suppression chip renders. Hoisted + // stable so the composer's "reconcile suppressed against visible agents" effect // runs once and toggling doesn't churn it. const mockAgents = vi.hoisted(() => [{ id: "ag-1", name: "Walt", source: "mention_agent" }]); +const uploadWithToast = vi.hoisted(() => vi.fn()); vi.mock("../hooks/use-comment-trigger-preview", () => ({ useCommentTriggerPreview: () => ({ agents: mockAgents }), @@ -50,7 +52,24 @@ vi.mock("./comment-trigger-chips", () => ({ vi.mock("@multica/core/api", () => ({ api: {} })); vi.mock("@multica/core/hooks/use-file-upload", () => ({ - useFileUpload: () => ({ uploadWithToast: vi.fn() }), + useFileUpload: () => ({ uploadWithToast }), +})); + +vi.mock("@multica/ui/components/common/file-upload-button", () => ({ + FileUploadButton: ({ onSelect }: { onSelect: (f: File) => void }) => ( + + ), +})); + +vi.mock("../../common/actor-avatar", () => ({ + ActorAvatar: () => null, + AgentStatusDot: () => null, })); vi.mock("../../i18n", () => ({ @@ -68,9 +87,11 @@ vi.mock("../../editor", () => ({ { defaultValue, onUpdate, + onUploadFile, }: { defaultValue?: string; onUpdate?: (markdown: string) => void; + onUploadFile?: (file: File) => Promise; }, ref: Ref, ) { @@ -82,7 +103,12 @@ vi.mock("../../editor", () => ({ }, focus: () => {}, blur: () => {}, - uploadFile: async () => {}, + // The upload result lands, but the content flush is DEBOUNCED — we + // deliberately do NOT call onUpdate here, so tests drive the + // "upload result first, content draft after" ordering explicitly. + uploadFile: async (file: File) => { + await onUploadFile?.(file); + }, hasActiveUploads: () => false, })); return ( @@ -121,6 +147,7 @@ function makeAttachment(id: string, url: string): Attachment { beforeEach(() => { localStorage.clear(); useCommentDraftStore.setState({ drafts: {} }); + uploadWithToast.mockReset(); }); describe("comment composer suppression persistence", () => { @@ -254,3 +281,107 @@ describe("edit composer attachment persistence", () => { ); }); }); + +// The real timing: an upload result lands (pendingAttachments set) BEFORE the +// debounced content flush creates the draft. The persist effect must re-run +// when the draft is created — not only when pendingAttachments changes — or the +// attachment is never persisted. +describe("attachment persist survives the upload-before-content-draft ordering", () => { + it("comment-input: an upload landing before the debounced content draft is still persisted", async () => { + const att = makeAttachment("att-1", "http://x/att-1.png"); + uploadWithToast.mockResolvedValue(att); + render( + , + ); + + // Upload result lands first — pendingAttachments set, no content draft yet. + await act(async () => { + fireEvent.click(screen.getByTestId("upload-btn")); + }); + expect(useCommentDraftStore.getState().getDraft("new:issue-1")).toBeUndefined(); + + // Debounced content flush arrives → draft created → attachment persisted. + fireEvent.change(screen.getByTestId("editor"), { + target: { value: "body http://x/att-1.png" }, + }); + await waitFor(() => + expect( + useCommentDraftStore.getState().getDraft("new:issue-1")?.attachments, + ).toEqual([att]), + ); + }); + + it("reply-input: same ordering, persisted into the reply draft", async () => { + const att = makeAttachment("att-2", "http://x/att-2.png"); + uploadWithToast.mockResolvedValue(att); + const draftKey = "reply:issue-1:c-root" as const; + render( + , + ); + + await act(async () => { + fireEvent.click(screen.getByTestId("upload-btn")); + }); + expect(useCommentDraftStore.getState().getDraft(draftKey)).toBeUndefined(); + + fireEvent.change(screen.getByTestId("editor"), { + target: { value: "reply http://x/att-2.png" }, + }); + await waitFor(() => + expect(useCommentDraftStore.getState().getDraft(draftKey)?.attachments).toEqual([ + att, + ]), + ); + }); + + it("comment-card edit: an upload during edit before the content flush is persisted", async () => { + const att = makeAttachment("att-3", "http://x/att-3.png"); + uploadWithToast.mockResolvedValue(att); + const entry = { + id: "c-1", + content: "original", + attachments: [], + parent_id: null, + } as unknown as TimelineEntry; + + let api: EditApi | null = null; + render( + { + api = a; + }} + />, + ); + + act(() => { + api!.startEdit(); + }); + // Upload result lands during edit — no content flush yet, so no edit draft. + await act(async () => { + await api!.handleUpload(new File(["x"], "a.png")); + }); + expect( + useCommentDraftStore.getState().getDraft("edit:issue-1:c-1"), + ).toBeUndefined(); + + // The debounced onUpdate: setContent + setDraft(content) → draft created. + act(() => { + api!.setContent("edited http://x/att-3.png"); + api!.setDraft(api!.draftKey, { content: "edited http://x/att-3.png" }); + }); + await waitFor(() => + expect( + useCommentDraftStore.getState().getDraft("edit:issue-1:c-1")?.attachments, + ).toEqual([att]), + ); + }); +}); diff --git a/packages/views/issues/components/comment-input.tsx b/packages/views/issues/components/comment-input.tsx index e661499cbd..f8ea6855d7 100644 --- a/packages/views/issues/components/comment-input.tsx +++ b/packages/views/issues/components/comment-input.tsx @@ -97,10 +97,16 @@ function CommentInput({ issueId, onSubmit }: CommentInputProps) { // write INTO an existing content draft — never create a content-less draft // just to record these — and always write the current value (incl. `[]`) so // clearing a suppression leaves no stale array to re-apply on remount. + // + // `content` is a dependency so this also re-runs when the (debounced) content + // draft is first created: an upload result sets `pendingAttachments` before + // that flush, so depending on `pendingAttachments` alone would evaluate the + // guard while no draft exists yet and never persist the attachment. useEffect(() => { + if (content.trim().length === 0) return; if (useCommentDraftStore.getState().getDraft(draftKey) === undefined) return; setDraft(draftKey, { attachments: pendingAttachments }); - }, [pendingAttachments, draftKey, setDraft]); + }, [content, pendingAttachments, draftKey, setDraft]); useEffect(() => { if (useCommentDraftStore.getState().getDraft(draftKey) === undefined) return; diff --git a/packages/views/issues/components/reply-input.tsx b/packages/views/issues/components/reply-input.tsx index 227e89230c..d9961bd24e 100644 --- a/packages/views/issues/components/reply-input.tsx +++ b/packages/views/issues/components/reply-input.tsx @@ -115,12 +115,15 @@ function ReplyInput({ // Persist attachments + suppressed choices — only when a draftKey is present // AND a content draft already exists (never create a content-less draft). // Always write the current value (incl. `[]`) so clearing a suppression - // leaves no stale array to re-apply on remount. + // leaves no stale array to re-apply on remount. `content` is a dependency so + // this also re-runs when the (debounced) content draft is first created — + // an upload result can land before that flush (see CommentInput). useEffect(() => { if (!draftKey) return; + if (content.trim().length === 0) return; if (useCommentDraftStore.getState().getDraft(draftKey) === undefined) return; setDraft(draftKey, { attachments: pendingAttachments }); - }, [draftKey, pendingAttachments, setDraft]); + }, [draftKey, content, pendingAttachments, setDraft]); useEffect(() => { if (!draftKey) return;