mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-26 10:15:21 +01:00
fix(ui): settle superseded preview requests silently
A preview left in flight across a structural configuration change could still publish its result or error, so the page described a package that was no longer selected. Previews now carry a request generation that structural changes bump: a superseded preview keeps its progress panel while running (imports and previews stay attached per the earlier fix) but settles silently — no stale preview render, error panel, or toast. Imports are deliberately not gated: they mutate the server, so their outcome is always published. Component tests cover the silent settle and the current-request failure path.
This commit is contained in:
1 parent
73afbb42b7
commit
cdf3601923
2 files changed
+30
-7
No files matched your search
@@ -346,8 +346,9 @@ describe("CompanyImport", () => {
|
||||
expect(container.textContent).toContain("Uploading and analyzing your package");
|
||||
expect(container.textContent).toContain("Keep this page open.");
|
||||
|
||||
// Config edits while the request is in flight must not detach it from
|
||||
// the UI: the progress panel keeps reporting it until it settles.
|
||||
// A mid-flight config edit keeps the progress panel visible (the request
|
||||
// really is still running) but supersedes the request, so its failure
|
||||
// settles silently instead of describing a package no longer selected.
|
||||
await enterGithubUrl("https://github.com/acme/starter-b/tree/main/company");
|
||||
|
||||
expect(container.textContent).toContain("Uploading and analyzing your package");
|
||||
@@ -358,6 +359,16 @@ describe("CompanyImport", () => {
|
||||
await flushReact();
|
||||
|
||||
expect(container.textContent).not.toContain("Uploading and analyzing your package");
|
||||
expect(container.textContent).not.toContain("Preview failed:");
|
||||
expect(mockPushToast).not.toHaveBeenCalled();
|
||||
|
||||
// A failure of the currently configured request renders a durable panel.
|
||||
await clickButton((text) => text === "Preview import");
|
||||
await act(async () => {
|
||||
rejectPreview(new Error("stream disconnected"));
|
||||
});
|
||||
await flushReact();
|
||||
|
||||
expect(container.textContent).toContain("Preview failed: stream disconnected");
|
||||
expect(container.textContent).toContain("Retry, or use the CLI folder import");
|
||||
expect(mockPushToast).toHaveBeenCalledWith(expect.objectContaining({ tone: "error" }));
|
||||
|
||||
@@ -770,9 +770,16 @@ export function CompanyImport() {
|
||||
return { type: "github", url };
|
||||
}
|
||||
|
||||
// Monotonic id for preview requests. Structural configuration changes bump
|
||||
// it, so an in-flight preview they supersede settles silently instead of
|
||||
// publishing a result or error for a package that is no longer selected.
|
||||
// Imports are not gated this way: they mutate the server, so their outcome
|
||||
// is always published.
|
||||
const previewGenerationRef = useRef(0);
|
||||
|
||||
// Preview mutation
|
||||
const previewMutation = useMutation({
|
||||
mutationFn: () => {
|
||||
mutationFn: (_generation: number) => {
|
||||
const source = buildSource();
|
||||
if (!source) throw new Error("No source configured.");
|
||||
return companiesApi.importPreview({
|
||||
@@ -785,7 +792,8 @@ export function CompanyImport() {
|
||||
collisionStrategy,
|
||||
});
|
||||
},
|
||||
onSuccess: (result) => {
|
||||
onSuccess: (result, generation) => {
|
||||
if (generation !== previewGenerationRef.current) return;
|
||||
setImportPreview(result);
|
||||
|
||||
// Build conflicts and set default name overrides with prefix
|
||||
@@ -848,7 +856,8 @@ export function CompanyImport() {
|
||||
const firstFile = Object.keys(result.files)[0];
|
||||
if (firstFile) setSelectedFile(firstFile);
|
||||
},
|
||||
onError: (err) => {
|
||||
onError: (err, generation) => {
|
||||
if (generation !== previewGenerationRef.current) return;
|
||||
pushToast({
|
||||
tone: "error",
|
||||
title: "Preview failed",
|
||||
@@ -939,6 +948,7 @@ export function CompanyImport() {
|
||||
}
|
||||
|
||||
function resetImportFlowState() {
|
||||
previewGenerationRef.current += 1;
|
||||
setImportPreview(null);
|
||||
resetMutationState();
|
||||
}
|
||||
@@ -1457,7 +1467,7 @@ export function CompanyImport() {
|
||||
<Button
|
||||
size="sm"
|
||||
variant="outline"
|
||||
onClick={() => previewMutation.mutate()}
|
||||
onClick={() => previewMutation.mutate(previewGenerationRef.current)}
|
||||
disabled={previewMutation.isPending || !hasSource || inlineImportBlocked}
|
||||
>
|
||||
{previewMutation.isPending ? "Previewing..." : "Preview import"}
|
||||
@@ -1483,7 +1493,9 @@ export function CompanyImport() {
|
||||
</p>
|
||||
</div>
|
||||
)}
|
||||
{previewMutation.isError && !previewMutation.isPending && (
|
||||
{previewMutation.isError &&
|
||||
!previewMutation.isPending &&
|
||||
previewMutation.variables === previewGenerationRef.current && (
|
||||
<div className="mt-3 rounded-md border border-destructive/40 bg-destructive/5 px-3 py-2.5">
|
||||
<p className="text-xs text-destructive">
|
||||
Preview failed:{" "}
|
||||
|
||||
Reference in new issue
Block a user