mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-12 10:59:06 +02:00
* fix(editor): an in-flight upload placeholder is never content, and is drawn once
Two defects with one cause: a placeholder for an upload in progress was both
serialised into the draft body and drawn a second time as a chip.
The document IS the persisted draft (getMarkdown -> setDraft), so serialising
an in-flight node turns it into text that outlives the upload:
- fileCard emitted `!file[x.pdf]()`. Its own tokenizer cannot parse an empty
href back, so the line survived reopen as dead literal text, sat next to
the real link the write-back appended, and shipped with the comment.
- image emitted its process-local `blob:` URL, which ContentEditor then
scrubbed back out with a regex on every serialise.
Both renderMarkdown implementations now emit nothing while `attrs.uploading`
is set (or no URL exists). A node becomes content the moment it holds a real
URL and never before, which is strictly stronger than scrubbing after the
fact — so BLOB_IMAGE_RE / stripBlobUrls are deleted rather than extended.
Separately, ComposerUploadChips rendered every non-`uploaded` entry, including
ones whose placeholder node is right there in the editor. Every upload started
from a live mount inserts a node first (uploadAndInsertFile is the uploader's
only caller), so those chips were the same upload drawn twice, in two visual
languages, shifting layout as they appeared and vanished. useCoordinatedUploads
now exposes `orphanUploads` — the entries inherited from the persisted draft,
whose originating mount is gone and whose node died with it. That is the case
the chip strip was introduced for, and now the only one it covers.
`getMarkdown()` deliberately stays untrimmed (see its safety-net test); only
its stripBlobUrls wrapper is gone.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(editor): keep a failed upload visible after the chip/node split
Self-review catch on the previous commit: suppressing the chip for every
upload this mount started also suppressed it for FAILED ones. The document
cannot stand in for those — uploadAndInsertFile removes the placeholder node
on failure — so the outcome was left to a toast that has already gone.
The rule is not "started here" but "the document is showing it", and the
document only ever shows a live placeholder: still `uploading` AND started by
this mount. `failed` / `interrupted` always get a chip, `uploaded` never does
(the editor and AttachmentList render those), which also makes an
`orphanUploads.length` gate mean what the call sites assume.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(editor): keep the chip when the user deletes a running upload's placeholder
Code-review catch: "started by this mount" is necessary but not sufficient for
"the document is showing it". Cmd+Z right after a paste removes the placeholder
node while the upload keeps running — and gate.isBlocked keeps blocking send on
the store entry regardless of the node — so the previous filter left a dead send
button with nothing on screen explaining it.
The filter now also consults editorGate.uploading, which is the document's own
answer to "am I showing a placeholder right now" (sourced from the uploading-node
scan via onUploadingChange). Started-here AND still shown is what suppresses a
chip; either half failing brings it back.
Also drops a stale stripBlobUrls reference from the use-upload-gate docstring —
that helper no longer exists.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(editor): a failed upload leaves nothing behind
The failure chip carried no information the toast had not already given at the
moment it happened, and it could not act on it: the bytes were never persisted,
so there is nothing to retry, and the file is still on disk to re-attach. Its
only affordance was a dismiss ✕.
It cost more than that. The entry lives in the persisted draft, so it survived
reload and reopen until dismissed by hand — and `isMeaningful` counts uploads,
so a single flaky request kept an otherwise-empty draft alive for the full
30-day TTL. Uploading again did not clear it either: a new upload is a new
clientUploadId.
Failures now remove their placeholder outright instead of marking it. Both
failure paths (size check, coordinator settle) collapse into that one rule,
which also folds the paste-as-file recovery into the shared branch rather than
duplicating it.
`interrupted` keeps its chip: it is discovered a session later, when the user
no longer remembers attaching anything. `orphanUploads` still handles `failed`
because an older client may have persisted one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* refactor(editor): one upload, one node, from start to finish
The chip strip existed because the document could not answer for an upload it
was not showing. Give it that ability and the strip has no reason to exist.
Three changes make one model:
- ONE IDENTITY. The node's `uploadId` and the draft's `clientUploadId` were
two independently minted random values, because the node is inserted before
the handler that created the draft record runs. `uploadAndInsertFile` now
mints the id up front and hands it to the uploader, which adopts it. Asking
"is this upload in the document" becomes a lookup instead of an inference.
- REBUILD ON MOUNT. A placeholder is never serialised (it is not content), so
it dies with the document that drew it and a reopened composer showed no
trace of an upload still running. The draft record is enough to draw it
again. Once per id per mount: a placeholder the user deleted mid-upload
stays deleted (MUL-5181), and the guard is what stops the next store write
from undoing that. Skipped entirely while chat pins its document to another
draft — `uploads` follows the selected key, the document does not.
- SETTLE IN PLACE. The write-back replaces the placeholder where the user last
saw it instead of appending the link at the end. A card promotes to an image
when that is what arrived; the rebuild path only ever has a filename, so it
cannot know in advance.
With that, the chips are deleted outright, along with `orphanUploads` and the
three-condition rule that approximated all of the above. `interrupted` goes
too: nothing could act on it, no surface rendered it after this change, and
`isMeaningful` counted it — one dead record kept an empty draft alive for the
full TTL. The attachment's absence from the body is the signal to re-attach.
SubmitButton's `busy` now spins rather than only greying out, so an upload
with no other on-screen trace (a composer still rebuilding, a placeholder the
user deleted) does not read as a dead control.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(editor): whoever draws a placeholder registers it, not whoever finds it
Review catch on the rebuild effect. An upload started by the current mount had
its node drawn synchronously by uploadAndInsertFile, but its id only entered
`rebuiltUploadIdsRef` once the effect ran and happened to find that node. In
between, a delete (Cmd+Z right after a paste) left the effect looking at an
unmarked `uploading` record with no node — so it drew a second one, undoing a
removal MUL-5181 says must stick, and letting the settle land an attachment
the user had taken out.
The id is now registered where it is minted: an id handed into handleUpload
means the editor already drew the node. The window is sub-frame and needs a
keystroke inside one render pass, but "whoever draws it registers it" is a
rule, where "the effect will notice in time" was a race.
The composer mocks called `onUploadFile(file)` with no id, so they were not
exercising the one-id contract at all — every mount-started upload looked
inherited to the hook. They now mint and pass one like the real handle does,
which is what lets the new regression test see the difference.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
219 lines
8.1 KiB
TypeScript
219 lines
8.1 KiB
TypeScript
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import { act, fireEvent, render, screen, waitFor } from "@testing-library/react";
|
|
import { forwardRef, useImperativeHandle, useRef } from "react";
|
|
|
|
let storedDraftMessage = "saved draft";
|
|
let liveEditorMarkdown = "";
|
|
const feedbackMocks = vi.hoisted(() => ({ mutateAsync: vi.fn() }));
|
|
// Deferred controlling the mock editor's in-flight upload: `reset` arms a new
|
|
// pending upload, `resolve` lands it so a test can watch the gate re-open.
|
|
const pendingUpload = vi.hoisted(() => {
|
|
const deferred = {
|
|
promise: Promise.resolve() as Promise<void>,
|
|
resolve: () => {},
|
|
reset() {
|
|
deferred.promise = new Promise<void>((res) => {
|
|
deferred.resolve = res;
|
|
});
|
|
},
|
|
};
|
|
return deferred;
|
|
});
|
|
|
|
vi.mock("react-i18next", () => ({
|
|
useTranslation: () => ({ t: (key: string) => key, i18n: { changeLanguage: vi.fn() } }),
|
|
Trans: ({ children }: { children: any }) => children,
|
|
initReactI18next: { type: "3rdParty", init: vi.fn() },
|
|
}));
|
|
|
|
vi.mock("../i18n", () => ({
|
|
useT: () => ({
|
|
t: (selector: (resources: any) => string) =>
|
|
selector({
|
|
feedback: {
|
|
title: "Feedback",
|
|
github_hint_prefix: "Prefer GitHub? ",
|
|
github_hint_link: "Open an issue",
|
|
placeholder: "Tell us what happened",
|
|
toast_uploading: "Uploading",
|
|
toast_too_long: "Too long",
|
|
toast_sent: "Sent",
|
|
toast_failed: "Failed",
|
|
sending: "Sending",
|
|
send: "Send",
|
|
},
|
|
// The `editor` namespace's shared upload-gate copy. This mock ignores
|
|
// the namespace argument, so both bundles live in one object.
|
|
upload: {
|
|
in_progress: "Uploading…",
|
|
},
|
|
}),
|
|
}),
|
|
}));
|
|
|
|
vi.mock("@multica/core/paths", () => ({ useCurrentWorkspace: () => ({ id: "ws1" }) }));
|
|
vi.mock("@multica/core/hooks/use-file-upload", () => ({
|
|
useFileUpload: () => ({ uploadWithToast: vi.fn() }),
|
|
}));
|
|
vi.mock("@multica/core/api", () => ({ api: {} }));
|
|
vi.mock("sonner", () => ({ toast: { info: vi.fn(), error: vi.fn(), success: vi.fn() } }));
|
|
vi.mock("@multica/core/feedback", () => ({
|
|
FEEDBACK_KINDS: ["bug", "feature", "general", "praise"] as const,
|
|
useCreateFeedback: () => ({ isPending: false, mutateAsync: feedbackMocks.mutateAsync }),
|
|
useFeedbackDraftStore: (selector: any) =>
|
|
selector({ draft: { message: storedDraftMessage }, setDraft: vi.fn(), clearDraft: vi.fn() }),
|
|
}));
|
|
vi.mock("../editor", async () => {
|
|
// Real submit gate (pure React) driven by the mock editor's
|
|
// `hasActiveUploads` / `onUploadingChange`.
|
|
const uploadGate = await vi.importActual<typeof import("../editor/use-upload-gate")>(
|
|
"../editor/use-upload-gate",
|
|
);
|
|
const ContentEditor = forwardRef(({ defaultValue, onSubmit, onUploadingChange }: any, ref) => {
|
|
liveEditorMarkdown = defaultValue;
|
|
// Mirrors the real editor: the placeholder node is in the doc from before
|
|
// the await until the upload settles, and the host hears about it through
|
|
// onUploadingChange rather than polling.
|
|
const inFlightRef = useRef(0);
|
|
useImperativeHandle(ref, () => ({
|
|
hasActiveUploads: () => inFlightRef.current > 0,
|
|
// Placeholder rebuild contract: the real handle draws a card for an
|
|
// upload the document is not showing and reports whether it landed.
|
|
// Mocks track ids only — no document to draw into.
|
|
insertUploadPlaceholder: () => true,
|
|
settleUploadPlaceholder: () => false,
|
|
getMarkdown: () => liveEditorMarkdown,
|
|
uploadFile: async () => {
|
|
inFlightRef.current += 1;
|
|
if (inFlightRef.current === 1) onUploadingChange?.(true);
|
|
try {
|
|
await pendingUpload.promise;
|
|
} finally {
|
|
inFlightRef.current -= 1;
|
|
if (inFlightRef.current === 0) onUploadingChange?.(false);
|
|
}
|
|
},
|
|
}));
|
|
return (
|
|
<textarea
|
|
aria-label="feedback editor"
|
|
defaultValue={defaultValue}
|
|
onChange={(event) => { liveEditorMarkdown = event.currentTarget.value; }}
|
|
onKeyDown={(event) => {
|
|
if (event.key === "Enter" && event.metaKey) void onSubmit?.();
|
|
}}
|
|
/>
|
|
);
|
|
});
|
|
ContentEditor.displayName = "MockContentEditor";
|
|
return {
|
|
...uploadGate,
|
|
useEditorUpload: () => ({
|
|
uploadWithToast: vi.fn(),
|
|
upload: vi.fn(),
|
|
uploading: false,
|
|
}),
|
|
ContentEditor,
|
|
useFileDropZone: () => ({ isDragOver: false, dropZoneProps: {} }),
|
|
FileDropOverlay: () => null,
|
|
FileUploadButton: () => <button type="button">Upload</button>,
|
|
};
|
|
});
|
|
|
|
import { FeedbackModal } from "./feedback";
|
|
|
|
describe("FeedbackModal", () => {
|
|
beforeEach(() => {
|
|
vi.spyOn(console, "error").mockImplementation(() => {});
|
|
feedbackMocks.mutateAsync.mockReset().mockResolvedValue(undefined);
|
|
});
|
|
|
|
afterEach(() => {
|
|
vi.restoreAllMocks();
|
|
});
|
|
|
|
it("uses a crash-report initialMessage when there is no saved draft", () => {
|
|
storedDraftMessage = "";
|
|
|
|
render(<FeedbackModal onClose={vi.fn()} initialMessage="kind: desktop_route_error" />);
|
|
|
|
expect(screen.getByLabelText("feedback editor")).toHaveValue("kind: desktop_route_error");
|
|
});
|
|
|
|
it("does not overwrite an existing feedback draft when crash report context is provided", () => {
|
|
storedDraftMessage = "saved draft";
|
|
|
|
render(<FeedbackModal onClose={vi.fn()} initialMessage="kind: desktop_route_error" />);
|
|
|
|
expect(screen.getByLabelText("feedback editor")).toHaveValue(
|
|
"saved draft\n\n---\n\nkind: desktop_route_error",
|
|
);
|
|
});
|
|
|
|
it("submits the editor's latest markdown before debounced state catches up", async () => {
|
|
storedDraftMessage = "";
|
|
render(<FeedbackModal onClose={vi.fn()} />);
|
|
|
|
const editor = screen.getByLabelText("feedback editor");
|
|
fireEvent.change(editor, { target: { value: "fresh feedback" } });
|
|
fireEvent.keyDown(editor, { key: "Enter", metaKey: true });
|
|
|
|
await waitFor(() => {
|
|
expect(feedbackMocks.mutateAsync).toHaveBeenCalledWith(
|
|
expect.objectContaining({ message: "fresh feedback" }),
|
|
);
|
|
});
|
|
});
|
|
|
|
// MUL-4808 — Feedback refused to submit mid-upload inside the handler, but
|
|
// the Send button stayed enabled, so the only signal was a toast fired after
|
|
// a click that looked like it should have worked.
|
|
describe("upload submit gate", () => {
|
|
function startPendingUpload() {
|
|
pendingUpload.reset();
|
|
// The modal renders through a portal, so its file input lives on
|
|
// document.body rather than under render()'s container.
|
|
const input = document.body.querySelector('input[type="file"]');
|
|
if (!input) throw new Error("Expected a file input to render");
|
|
fireEvent.change(input, {
|
|
target: { files: [new File(["x"], "shot.png", { type: "image/png" })] },
|
|
});
|
|
}
|
|
|
|
it("disables Send and shows Uploading… while an upload is in flight", async () => {
|
|
// Seeded through the draft so `message` is non-empty on mount — that
|
|
// isolates the disabled state to the upload gate rather than the
|
|
// empty-content check.
|
|
storedDraftMessage = "here's a screenshot";
|
|
render(<FeedbackModal onClose={vi.fn()} />);
|
|
|
|
startPendingUpload();
|
|
|
|
const send = await screen.findByRole("button", { name: "Uploading…" });
|
|
await waitFor(() => expect(send).toBeDisabled());
|
|
expect(send).toHaveAttribute("aria-busy", "true");
|
|
|
|
await act(async () => { pendingUpload.resolve(); });
|
|
await waitFor(() =>
|
|
expect(screen.getByRole("button", { name: "Send" })).not.toBeDisabled(),
|
|
);
|
|
});
|
|
|
|
it("blocks the Cmd+Enter path while an upload is in flight", async () => {
|
|
storedDraftMessage = "here's a screenshot";
|
|
render(<FeedbackModal onClose={vi.fn()} />);
|
|
const editor = screen.getByLabelText("feedback editor");
|
|
|
|
startPendingUpload();
|
|
|
|
fireEvent.keyDown(editor, { key: "Enter", metaKey: true });
|
|
await Promise.resolve();
|
|
expect(feedbackMocks.mutateAsync).not.toHaveBeenCalled();
|
|
|
|
await act(async () => { pendingUpload.resolve(); });
|
|
fireEvent.keyDown(editor, { key: "Enter", metaKey: true });
|
|
await waitFor(() => expect(feedbackMocks.mutateAsync).toHaveBeenCalled());
|
|
});
|
|
});
|
|
});
|