Skip to content

Commit 2cdb169

Browse files
fix(ui): stop deleted archived tasks from reappearing in sidebar (#3144)
Co-authored-by: Charles Vien <charles.v@posthog.com>
1 parent ac66d2d commit 2cdb169

2 files changed

Lines changed: 182 additions & 13 deletions

File tree

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
import type {
2+
ContextMenuOutcome,
3+
DeleteOutcome,
4+
RestoreOutcome,
5+
} from "@posthog/core/archive/archivedTasksController";
6+
import { WORKSPACE_QUERY_KEY } from "@posthog/ui/features/workspace/identifiers";
7+
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
8+
import { act, renderHook } from "@testing-library/react";
9+
import type { ReactNode } from "react";
10+
import { beforeEach, describe, expect, it, vi } from "vitest";
11+
12+
const ARCHIVE_FILTER = { queryKey: [["archive"]] };
13+
14+
const controller = vi.hoisted(() => ({
15+
restore: vi.fn(),
16+
remove: vi.fn(),
17+
runContextMenuAction: vi.fn(),
18+
}));
19+
20+
vi.mock("@posthog/di/react", () => ({
21+
useService: () => controller,
22+
}));
23+
24+
vi.mock("@posthog/host-router/react", () => ({
25+
useHostTRPC: () => ({
26+
archive: {
27+
pathFilter: () => ARCHIVE_FILTER,
28+
},
29+
}),
30+
}));
31+
32+
import { useUnarchiveTask } from "./useUnarchiveTask";
33+
34+
let queryClient: QueryClient;
35+
function wrapper({ children }: { children: ReactNode }) {
36+
return (
37+
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
38+
);
39+
}
40+
41+
describe("useUnarchiveTask", () => {
42+
beforeEach(() => {
43+
vi.clearAllMocks();
44+
queryClient = new QueryClient({
45+
defaultOptions: { queries: { retry: false } },
46+
});
47+
});
48+
49+
it("invalidates the workspace, archive and tasks caches together when an archived task is deleted", async () => {
50+
// Regression: delete once skipped WORKSPACE_QUERY_KEY, leaving stale sidebar rows.
51+
controller.remove.mockResolvedValue({ kind: "deleted" } as DeleteOutcome);
52+
const invalidateSpy = vi.spyOn(queryClient, "invalidateQueries");
53+
const refetchSpy = vi.spyOn(queryClient, "refetchQueries");
54+
const { result } = renderHook(() => useUnarchiveTask(), { wrapper });
55+
56+
await act(async () => {
57+
await result.current.remove("t1");
58+
});
59+
60+
expect(invalidateSpy).toHaveBeenCalledWith({
61+
queryKey: WORKSPACE_QUERY_KEY,
62+
});
63+
expect(invalidateSpy).toHaveBeenCalledWith(ARCHIVE_FILTER);
64+
expect(refetchSpy).toHaveBeenCalledWith({ queryKey: ["tasks"] });
65+
});
66+
67+
it.each<[string, DeleteOutcome, boolean]>([
68+
["deleted", { kind: "deleted" }, true],
69+
["error", { kind: "error", message: "nope" }, false],
70+
])(
71+
"remove() with outcome %s invalidates the workspace query: %s",
72+
async (_name, outcome, shouldInvalidate) => {
73+
controller.remove.mockResolvedValue(outcome);
74+
const invalidateSpy = vi.spyOn(queryClient, "invalidateQueries");
75+
const { result } = renderHook(() => useUnarchiveTask(), { wrapper });
76+
77+
await act(async () => {
78+
await result.current.remove("t1");
79+
});
80+
81+
if (shouldInvalidate) {
82+
expect(invalidateSpy).toHaveBeenCalledWith({
83+
queryKey: WORKSPACE_QUERY_KEY,
84+
});
85+
} else {
86+
expect(invalidateSpy).not.toHaveBeenCalled();
87+
}
88+
},
89+
);
90+
91+
it.each<[string, RestoreOutcome, boolean]>([
92+
["restored", { kind: "restored", navigateToTaskId: "t1" }, true],
93+
[
94+
"branch-not-found",
95+
{ kind: "branch-not-found", taskId: "t1", branchName: "b" },
96+
false,
97+
],
98+
["error", { kind: "error", message: "nope" }, false],
99+
])(
100+
"restore() with outcome %s invalidates the workspace query: %s",
101+
async (_name, outcome, shouldInvalidate) => {
102+
controller.restore.mockResolvedValue(outcome);
103+
const invalidateSpy = vi.spyOn(queryClient, "invalidateQueries");
104+
const { result } = renderHook(() => useUnarchiveTask(), { wrapper });
105+
106+
await act(async () => {
107+
await result.current.restore("t1", true);
108+
});
109+
110+
if (shouldInvalidate) {
111+
expect(invalidateSpy).toHaveBeenCalledWith({
112+
queryKey: WORKSPACE_QUERY_KEY,
113+
});
114+
} else {
115+
expect(invalidateSpy).not.toHaveBeenCalled();
116+
}
117+
},
118+
);
119+
120+
it.each<[string, ContextMenuOutcome, boolean]>([
121+
["noop", { kind: "noop" }, false],
122+
["menu-error", { kind: "menu-error", message: "nope" }, false],
123+
[
124+
"restore -> restored",
125+
{
126+
kind: "restore",
127+
outcome: { kind: "restored", navigateToTaskId: "t1" },
128+
},
129+
true,
130+
],
131+
[
132+
"restore -> branch-not-found",
133+
{
134+
kind: "restore",
135+
outcome: {
136+
kind: "branch-not-found",
137+
taskId: "t1",
138+
branchName: "b",
139+
},
140+
},
141+
false,
142+
],
143+
[
144+
"delete -> deleted",
145+
{ kind: "delete", outcome: { kind: "deleted" } },
146+
true,
147+
],
148+
[
149+
"delete -> error",
150+
{ kind: "delete", outcome: { kind: "error", message: "nope" } },
151+
false,
152+
],
153+
])(
154+
"runContextMenuAction() with outcome %s invalidates the workspace query: %s",
155+
async (_name, outcome, shouldInvalidate) => {
156+
controller.runContextMenuAction.mockResolvedValue(outcome);
157+
const invalidateSpy = vi.spyOn(queryClient, "invalidateQueries");
158+
const { result } = renderHook(() => useUnarchiveTask(), { wrapper });
159+
160+
await act(async () => {
161+
await result.current.runContextMenuAction("t1", "Task 1", true);
162+
});
163+
164+
if (shouldInvalidate) {
165+
expect(invalidateSpy).toHaveBeenCalledWith({
166+
queryKey: WORKSPACE_QUERY_KEY,
167+
});
168+
} else {
169+
expect(invalidateSpy).not.toHaveBeenCalled();
170+
}
171+
},
172+
);
173+
});

packages/ui/src/features/archive/useUnarchiveTask.ts

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,18 +32,14 @@ export function useUnarchiveTask(): UseUnarchiveTask {
3232
const trpc = useHostTRPC();
3333
const queryClient = useQueryClient();
3434

35-
const invalidateArchiveQueries = useCallback(async () => {
35+
const invalidateTaskListCaches = useCallback(async () => {
3636
await Promise.all([
37+
queryClient.invalidateQueries({ queryKey: WORKSPACE_QUERY_KEY }),
3738
queryClient.invalidateQueries(trpc.archive.pathFilter()),
3839
queryClient.refetchQueries({ queryKey: ["tasks"] }),
3940
]);
4041
}, [queryClient, trpc]);
4142

42-
const invalidateOnRestore = useCallback(async () => {
43-
await queryClient.invalidateQueries({ queryKey: WORKSPACE_QUERY_KEY });
44-
await invalidateArchiveQueries();
45-
}, [queryClient, invalidateArchiveQueries]);
46-
4743
const restore = useCallback(
4844
async (
4945
taskId: string,
@@ -52,22 +48,22 @@ export function useUnarchiveTask(): UseUnarchiveTask {
5248
) => {
5349
const outcome = await controller.restore(taskId, hasTask, options);
5450
if (outcome.kind === "restored") {
55-
await invalidateOnRestore();
51+
await invalidateTaskListCaches();
5652
}
5753
return outcome;
5854
},
59-
[controller, invalidateOnRestore],
55+
[controller, invalidateTaskListCaches],
6056
);
6157

6258
const remove = useCallback(
6359
async (taskId: string) => {
6460
const outcome = await controller.remove(taskId);
6561
if (outcome.kind === "deleted") {
66-
await invalidateArchiveQueries();
62+
await invalidateTaskListCaches();
6763
}
6864
return outcome;
6965
},
70-
[controller, invalidateArchiveQueries],
66+
[controller, invalidateTaskListCaches],
7167
);
7268

7369
const runContextMenuAction = useCallback(
@@ -78,16 +74,16 @@ export function useUnarchiveTask(): UseUnarchiveTask {
7874
hasTask,
7975
);
8076
if (outcome.kind === "restore" && outcome.outcome.kind === "restored") {
81-
await invalidateOnRestore();
77+
await invalidateTaskListCaches();
8278
} else if (
8379
outcome.kind === "delete" &&
8480
outcome.outcome.kind === "deleted"
8581
) {
86-
await invalidateArchiveQueries();
82+
await invalidateTaskListCaches();
8783
}
8884
return outcome;
8985
},
90-
[controller, invalidateOnRestore, invalidateArchiveQueries],
86+
[controller, invalidateTaskListCaches],
9187
);
9288

9389
return { restore, remove, runContextMenuAction };

0 commit comments

Comments
 (0)