Skip to content

Commit 7133baf

Browse files
authored
fix(channels): keep backend task channel in sync on rename and delete
Renaming a channel folder left the backend task channel behind (it is resolved by name), so the feed re-provisioned an empty channel and existing tasks/messages were orphaned. Rename now moves the backend channel by id in lockstep with the folder, with rollback if the folder rename fails. Deleting a channel now soft-deletes the backend channel's tasks and the channel itself (both soft deletes server-side, so data is recoverable) before removing the folder, instead of leaving them orphaned on an unreachable channel. Generated-By: PostHog Code Task-Id: 3ac7dc89-890b-4013-bf1f-5dc40476f093
1 parent d391b25 commit 7133baf

5 files changed

Lines changed: 338 additions & 12 deletions

File tree

packages/api-client/src/posthog-client.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2286,6 +2286,50 @@ export class PostHogAPIClient {
22862286
return (await response.json()) as TaskChannel[];
22872287
}
22882288

2289+
// Rename a public channel by id, so its task feed follows the new name.
2290+
async renameTaskChannel(id: string, name: string): Promise<TaskChannel> {
2291+
const teamId = await this.getTeamId();
2292+
const urlPath = `/api/projects/${teamId}/task_channels/${encodeURIComponent(id)}/`;
2293+
const response = await this.api.fetcher.fetch({
2294+
method: "patch",
2295+
url: new URL(`${this.api.baseUrl}${urlPath}`),
2296+
path: urlPath,
2297+
overrides: { body: JSON.stringify({ name }) },
2298+
});
2299+
if (!response.ok) {
2300+
// The backend rejects invalid/taken names with a JSON `detail` message.
2301+
const detail = await response
2302+
.json()
2303+
.then((body) => (body as { detail?: string }).detail)
2304+
.catch(() => undefined);
2305+
throw new Error(
2306+
`Failed to rename task channel: ${detail ?? response.statusText}`,
2307+
);
2308+
}
2309+
return (await response.json()) as TaskChannel;
2310+
}
2311+
2312+
// Delete a public channel by id. Soft-delete server-side, so the channel's
2313+
// tasks and messages are recoverable.
2314+
async deleteTaskChannel(id: string): Promise<void> {
2315+
const teamId = await this.getTeamId();
2316+
const urlPath = `/api/projects/${teamId}/task_channels/${encodeURIComponent(id)}/`;
2317+
const response = await this.api.fetcher.fetch({
2318+
method: "delete",
2319+
url: new URL(`${this.api.baseUrl}${urlPath}`),
2320+
path: urlPath,
2321+
});
2322+
if (!response.ok) {
2323+
const detail = await response
2324+
.json()
2325+
.then((body) => (body as { detail?: string }).detail)
2326+
.catch(() => undefined);
2327+
throw new Error(
2328+
`Failed to delete task channel: ${detail ?? response.statusText}`,
2329+
);
2330+
}
2331+
}
2332+
22892333
// Resolve-or-create a public channel by name (idempotent server-side).
22902334
async resolveTaskChannel(name: string): Promise<TaskChannel> {
22912335
const teamId = await this.getTeamId();

packages/ui/src/features/canvas/components/ChannelsList.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ function useChannelActions(channel: Channel): {
117117
),
118118
]);
119119

120-
await deleteChannel(channel.id);
120+
await deleteChannel(channel.id, channel.name);
121121
removeStar();
122122
track(ANALYTICS_EVENTS.CHANNEL_ACTION, {
123123
action_type: "delete",
@@ -444,8 +444,8 @@ function ChannelSection({ channel }: { channel: Channel }) {
444444
Every canvas saved in this channel is permanently deleted.
445445
</li>
446446
<li>
447-
Filed tasks are removed from the channel, but the tasks
448-
themselves are not deleted.
447+
Tasks in this channel are deleted with it. Their data is
448+
retained, so they can be restored later.
449449
</li>
450450
</ul>
451451
</AlertDialogDescription>

packages/ui/src/features/canvas/components/RenameChannelModal.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export function RenameChannelModal({
3939
const submit = async () => {
4040
if (!trimmed || unchanged || validationError || isRenaming) return;
4141
try {
42-
await renameChannel(channel.id, trimmed);
42+
await renameChannel(channel.id, trimmed, channel.name);
4343
track(ANALYTICS_EVENTS.CHANNEL_ACTION, {
4444
action_type: "rename",
4545
surface: "sidebar",

packages/ui/src/features/canvas/hooks/useChannels.test.tsx

Lines changed: 210 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { Schemas } from "@posthog/api-client";
2+
import type { TaskChannel } from "@posthog/shared/domain-types";
23
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
34
import { act, renderHook, waitFor } from "@testing-library/react";
45
import type { ReactNode } from "react";
@@ -7,6 +8,13 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
78
const mockClient = vi.hoisted(() => ({
89
getDesktopFileSystemChannels: vi.fn(),
910
createDesktopFileSystemChannel: vi.fn(),
11+
renameDesktopFileSystemChannel: vi.fn(),
12+
deleteDesktopFileSystem: vi.fn(),
13+
getTaskChannels: vi.fn(),
14+
renameTaskChannel: vi.fn(),
15+
deleteTaskChannel: vi.fn(),
16+
getTasks: vi.fn(),
17+
deleteTask: vi.fn(),
1018
}));
1119
vi.mock("@posthog/ui/features/auth/authClient", () => ({
1220
useOptionalAuthenticatedClient: () => mockClient,
@@ -99,3 +107,205 @@ describe("useChannelMutations", () => {
99107
expect(list.result.current.channels.map((c) => c.id)).toEqual(["1"]);
100108
});
101109
});
110+
111+
describe("useChannelMutations rename", () => {
112+
function taskChannel(
113+
id: string,
114+
name: string,
115+
channel_type: TaskChannel["channel_type"] = "public",
116+
): TaskChannel {
117+
return { id, name, channel_type, created_at: "2026-01-01T00:00:00Z" };
118+
}
119+
120+
beforeEach(() => {
121+
vi.clearAllMocks();
122+
queryClient = new QueryClient({
123+
defaultOptions: { queries: { retry: false } },
124+
});
125+
});
126+
127+
it("renames the backend task channel alongside the folder", async () => {
128+
// The feed's backend channel is looked up by name, so a folder rename
129+
// must carry it along or the channel's tasks/messages are orphaned.
130+
mockClient.getTaskChannels.mockResolvedValue([
131+
taskChannel("bc-1", "mobile"),
132+
taskChannel("bc-2", "web"),
133+
]);
134+
mockClient.renameTaskChannel.mockResolvedValue(
135+
taskChannel("bc-1", "mobile-app"),
136+
);
137+
mockClient.renameDesktopFileSystemChannel.mockResolvedValue(
138+
folder("1", "mobile-app"),
139+
);
140+
141+
const mutations = renderHook(() => useChannelMutations(), { wrapper });
142+
await act(async () => {
143+
await mutations.result.current.renameChannel("1", "mobile-app", "mobile");
144+
});
145+
146+
expect(mockClient.renameTaskChannel).toHaveBeenCalledWith(
147+
"bc-1",
148+
"mobile-app",
149+
);
150+
expect(mockClient.renameDesktopFileSystemChannel).toHaveBeenCalledWith(
151+
"1",
152+
"mobile-app",
153+
);
154+
});
155+
156+
it("skips the backend rename when no backend channel has the old name", async () => {
157+
mockClient.getTaskChannels.mockResolvedValue([taskChannel("bc-2", "web")]);
158+
mockClient.renameDesktopFileSystemChannel.mockResolvedValue(
159+
folder("1", "mobile-app"),
160+
);
161+
162+
const mutations = renderHook(() => useChannelMutations(), { wrapper });
163+
await act(async () => {
164+
await mutations.result.current.renameChannel("1", "mobile-app", "mobile");
165+
});
166+
167+
expect(mockClient.renameTaskChannel).not.toHaveBeenCalled();
168+
expect(mockClient.renameDesktopFileSystemChannel).toHaveBeenCalledWith(
169+
"1",
170+
"mobile-app",
171+
);
172+
});
173+
174+
it("does not rename the folder when the backend rename fails", async () => {
175+
mockClient.getTaskChannels.mockResolvedValue([
176+
taskChannel("bc-1", "mobile"),
177+
]);
178+
mockClient.renameTaskChannel.mockRejectedValue(new Error("name taken"));
179+
180+
const mutations = renderHook(() => useChannelMutations(), { wrapper });
181+
await expect(
182+
act(() =>
183+
mutations.result.current.renameChannel("1", "mobile-app", "mobile"),
184+
),
185+
).rejects.toThrow("name taken");
186+
187+
expect(mockClient.renameDesktopFileSystemChannel).not.toHaveBeenCalled();
188+
});
189+
190+
it("reverts the backend rename when the folder rename fails", async () => {
191+
mockClient.getTaskChannels.mockResolvedValue([
192+
taskChannel("bc-1", "mobile"),
193+
]);
194+
mockClient.renameTaskChannel.mockResolvedValue(
195+
taskChannel("bc-1", "mobile-app"),
196+
);
197+
mockClient.renameDesktopFileSystemChannel.mockRejectedValue(
198+
new Error("folder rename failed"),
199+
);
200+
201+
const mutations = renderHook(() => useChannelMutations(), { wrapper });
202+
await expect(
203+
act(() =>
204+
mutations.result.current.renameChannel("1", "mobile-app", "mobile"),
205+
),
206+
).rejects.toThrow("folder rename failed");
207+
208+
expect(mockClient.renameTaskChannel).toHaveBeenNthCalledWith(
209+
1,
210+
"bc-1",
211+
"mobile-app",
212+
);
213+
expect(mockClient.renameTaskChannel).toHaveBeenNthCalledWith(
214+
2,
215+
"bc-1",
216+
"mobile",
217+
);
218+
});
219+
});
220+
221+
describe("useChannelMutations delete", () => {
222+
function taskChannel(
223+
id: string,
224+
name: string,
225+
channel_type: TaskChannel["channel_type"] = "public",
226+
): TaskChannel {
227+
return { id, name, channel_type, created_at: "2026-01-01T00:00:00Z" };
228+
}
229+
230+
beforeEach(() => {
231+
vi.clearAllMocks();
232+
queryClient = new QueryClient({
233+
defaultOptions: { queries: { retry: false } },
234+
});
235+
});
236+
237+
it("soft-deletes the backend channel's tasks and the channel before the folder", async () => {
238+
mockClient.getTaskChannels.mockResolvedValue([
239+
taskChannel("bc-1", "mobile"),
240+
taskChannel("bc-2", "web"),
241+
]);
242+
mockClient.getTasks.mockResolvedValue([{ id: "t-1" }, { id: "t-2" }]);
243+
mockClient.deleteTask.mockResolvedValue(undefined);
244+
mockClient.deleteTaskChannel.mockResolvedValue(undefined);
245+
mockClient.deleteDesktopFileSystem.mockResolvedValue(undefined);
246+
247+
const mutations = renderHook(() => useChannelMutations(), { wrapper });
248+
await act(async () => {
249+
await mutations.result.current.deleteChannel("1", "mobile");
250+
});
251+
252+
expect(mockClient.getTasks).toHaveBeenCalledWith({ channel: "bc-1" });
253+
expect(mockClient.deleteTask).toHaveBeenCalledWith("t-1");
254+
expect(mockClient.deleteTask).toHaveBeenCalledWith("t-2");
255+
expect(mockClient.deleteTaskChannel).toHaveBeenCalledWith("bc-1");
256+
expect(mockClient.deleteDesktopFileSystem).toHaveBeenCalledWith("1");
257+
});
258+
259+
it("deletes just the folder when no backend channel matches the name", async () => {
260+
mockClient.getTaskChannels.mockResolvedValue([taskChannel("bc-2", "web")]);
261+
mockClient.deleteDesktopFileSystem.mockResolvedValue(undefined);
262+
263+
const mutations = renderHook(() => useChannelMutations(), { wrapper });
264+
await act(async () => {
265+
await mutations.result.current.deleteChannel("1", "mobile");
266+
});
267+
268+
expect(mockClient.getTasks).not.toHaveBeenCalled();
269+
expect(mockClient.deleteTaskChannel).not.toHaveBeenCalled();
270+
expect(mockClient.deleteDesktopFileSystem).toHaveBeenCalledWith("1");
271+
});
272+
273+
it("still deletes the backend channel when a task soft delete fails", async () => {
274+
// Task deletes are best-effort: a straggler stays attached to the
275+
// soft-deleted channel (recoverable) rather than blocking the delete.
276+
mockClient.getTaskChannels.mockResolvedValue([
277+
taskChannel("bc-1", "mobile"),
278+
]);
279+
mockClient.getTasks.mockResolvedValue([{ id: "t-1" }, { id: "t-2" }]);
280+
mockClient.deleteTask
281+
.mockRejectedValueOnce(new Error("task delete failed"))
282+
.mockResolvedValueOnce(undefined);
283+
mockClient.deleteTaskChannel.mockResolvedValue(undefined);
284+
mockClient.deleteDesktopFileSystem.mockResolvedValue(undefined);
285+
286+
const mutations = renderHook(() => useChannelMutations(), { wrapper });
287+
await act(async () => {
288+
await mutations.result.current.deleteChannel("1", "mobile");
289+
});
290+
291+
expect(mockClient.deleteTaskChannel).toHaveBeenCalledWith("bc-1");
292+
expect(mockClient.deleteDesktopFileSystem).toHaveBeenCalledWith("1");
293+
});
294+
295+
it("does not delete the folder when the backend channel delete fails", async () => {
296+
mockClient.getTaskChannels.mockResolvedValue([
297+
taskChannel("bc-1", "mobile"),
298+
]);
299+
mockClient.getTasks.mockResolvedValue([]);
300+
mockClient.deleteTaskChannel.mockRejectedValue(
301+
new Error("channel delete failed"),
302+
);
303+
304+
const mutations = renderHook(() => useChannelMutations(), { wrapper });
305+
await expect(
306+
act(() => mutations.result.current.deleteChannel("1", "mobile")),
307+
).rejects.toThrow("channel delete failed");
308+
309+
expect(mockClient.deleteDesktopFileSystem).not.toHaveBeenCalled();
310+
});
311+
});

0 commit comments

Comments
 (0)