Skip to content

Commit 60f3ab2

Browse files
authored
fix(frontend): stop flow deletes from clobbering newly created flows (#13975)
fix(frontend): read fresh flow list in delete success
1 parent 6bd0e05 commit 60f3ab2

4 files changed

Lines changed: 103 additions & 4 deletions

File tree

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
import { renderHook } from "@testing-library/react";
2+
import type { FlowType } from "@/types/flow";
3+
import useDeleteFlow from "../use-delete-flow";
4+
5+
const mockSetFlows = jest.fn();
6+
const mockMutate = jest.fn();
7+
8+
const flowsState: { flows: FlowType[] } = { flows: [] };
9+
10+
jest.mock("@/controllers/API/queries/flows/use-delete-delete-flows", () => ({
11+
useDeleteDeleteFlows: () => ({ mutate: mockMutate, isPending: false }),
12+
}));
13+
14+
type Selector<T> = (state: T) => unknown;
15+
type FlowsManagerState = { flows: FlowType[]; setFlows: jest.Mock };
16+
17+
jest.mock("@/stores/flowsManagerStore", () => {
18+
const store = Object.assign(
19+
(selector: Selector<FlowsManagerState>) =>
20+
selector({ flows: flowsState.flows, setFlows: mockSetFlows }),
21+
{ getState: () => ({ flows: flowsState.flows, setFlows: mockSetFlows }) },
22+
);
23+
return { __esModule: true, default: store };
24+
});
25+
26+
jest.mock("@/stores/typesStore", () => ({
27+
useTypesStore: { setState: jest.fn() },
28+
}));
29+
30+
jest.mock("@/utils/reactflowUtils", () => ({
31+
processFlows: (flows: FlowType[]) => ({ data: {}, flows }),
32+
extractFieldsFromComponenents: () => ({}),
33+
}));
34+
35+
const makeFlow = (id: string): FlowType =>
36+
({ id, name: id, data: { nodes: [], edges: [], viewport: {} } }) as FlowType;
37+
38+
describe("useDeleteFlow", () => {
39+
beforeEach(() => {
40+
jest.clearAllMocks();
41+
flowsState.flows = [];
42+
});
43+
44+
it("should_remove_only_deleted_flows_when_delete_succeeds", async () => {
45+
flowsState.flows = [makeFlow("flow-a"), makeFlow("flow-b")];
46+
mockMutate.mockImplementation((_vars, { onSuccess }) => onSuccess());
47+
48+
const { result } = renderHook(() => useDeleteFlow());
49+
await result.current.deleteFlow({ id: "flow-a" });
50+
51+
expect(mockSetFlows).toHaveBeenCalledWith([
52+
expect.objectContaining({ id: "flow-b" }),
53+
]);
54+
});
55+
56+
it("should_keep_flow_created_while_delete_was_in_flight", async () => {
57+
flowsState.flows = [makeFlow("placeholder")];
58+
// Simulate a flow landing in the store (e.g. a template flow created by
59+
// the welcome overlay handoff) before the DELETE response arrives.
60+
mockMutate.mockImplementation((_vars, { onSuccess }) => {
61+
flowsState.flows = [makeFlow("placeholder"), makeFlow("template-flow")];
62+
onSuccess();
63+
});
64+
65+
const { result } = renderHook(() => useDeleteFlow());
66+
await result.current.deleteFlow({ id: "placeholder" });
67+
68+
expect(mockSetFlows).toHaveBeenCalledWith([
69+
expect.objectContaining({ id: "template-flow" }),
70+
]);
71+
});
72+
73+
it("should_reject_when_delete_fails", async () => {
74+
flowsState.flows = [makeFlow("flow-a")];
75+
const error = new Error("boom");
76+
mockMutate.mockImplementation((_vars, { onError }) => onError(error));
77+
78+
const { result } = renderHook(() => useDeleteFlow());
79+
80+
await expect(result.current.deleteFlow({ id: "flow-a" })).rejects.toThrow(
81+
"boom",
82+
);
83+
expect(mockSetFlows).not.toHaveBeenCalled();
84+
});
85+
});

src/frontend/src/hooks/flows/use-delete-flow.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ const useDeleteFlow = () => {
1616
}: {
1717
id: string | string[];
1818
}): Promise<void> => {
19-
const flows = useFlowsManagerStore.getState().flows;
2019
return new Promise<void>((resolve, reject) => {
2120
if (!Array.isArray(id)) {
2221
id = [id];
@@ -25,6 +24,9 @@ const useDeleteFlow = () => {
2524
{ flow_ids: id },
2625
{
2726
onSuccess: () => {
27+
// Fresh read: a pre-mutation snapshot would drop flows created
28+
// while the DELETE was in flight, bouncing FlowPage to /all.
29+
const flows = useFlowsManagerStore.getState().flows;
2830
const { data, flows: myFlows } = processFlows(
2931
(flows ?? []).filter((flow) => !id.includes(flow.id)),
3032
);

src/frontend/tests/core/integrations/starter-projects-outdated-components.spec.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { expect, test } from "../../fixtures";
22
import { awaitBootstrapTest } from "../../utils/await-bootstrap-test";
3+
import { TIMEOUTS } from "../../utils/constants/timeouts";
34
import { openTemplatesModal } from "../../utils/flow/new-project-flow";
45

56
// The starter-project templates are checked for outdated components in four
@@ -70,7 +71,12 @@ QUARTERS.forEach((label, quarter) => {
7071
await expect(page.getByTestId("mainpage_title")).toBeVisible({
7172
timeout: 30000,
7273
});
73-
await openTemplatesModal(page);
74+
// TIMEOUTS.long: two shard workers share one SQLite backend, and
75+
// each iteration's placeholder-flow create/delete churn can queue
76+
// the "New Flow" POST behind the 30s busy_timeout. A standard (30s)
77+
// wait expires exactly when the lock clears — see nightly run
78+
// 28833493062 ("database is locked" + welcome/modal race timeout).
79+
await openTemplatesModal(page, { modalTimeout: TIMEOUTS.long });
7480
await page.waitForLoadState("domcontentloaded");
7581
await page.getByTestId("side_nav_options_all-templates").click();
7682
await expect(

src/frontend/tests/utils/flow/new-project-flow.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,15 @@ export const openTemplatesModal = async (
6464
const welcomeSelector = '[data-testid="flow-builder-welcome-panel"]';
6565
const modalSelector = `[data-testid="${TID.modalTitle}"]`;
6666

67+
// The race must honor the caller's modalTimeout: clicking "New Flow" first
68+
// POSTs the placeholder flow, and on CI shards where parallel workers share
69+
// one SQLite backend that write can queue up to the 30s busy_timeout —
70+
// callers in write-heavy loops pass TIMEOUTS.long to ride that window out.
71+
const raceTimeout = options?.modalTimeout ?? TIMEOUTS.standard;
72+
6773
await Promise.race([
68-
page.waitForSelector(welcomeSelector, { timeout: TIMEOUTS.standard }),
69-
page.waitForSelector(modalSelector, { timeout: TIMEOUTS.standard }),
74+
page.waitForSelector(welcomeSelector, { timeout: raceTimeout }),
75+
page.waitForSelector(modalSelector, { timeout: raceTimeout }),
7076
]);
7177

7278
if ((await page.locator(welcomeSelector).count()) > 0) {

0 commit comments

Comments
 (0)