fix(channels): keep the backend task channel in sync on rename and delete#3228
fix(channels): keep the backend task channel in sync on rename and delete#3228adamleithp wants to merge 1 commit into
Conversation
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
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
|
Reviews (1): Last reviewed commit: "fix(channels): keep backend task channel..." | Re-trigger Greptile |
| function taskChannel( | ||
| id: string, | ||
| name: string, | ||
| channel_type: TaskChannel["channel_type"] = "public", | ||
| ): TaskChannel { | ||
| return { id, name, channel_type, created_at: "2026-01-01T00:00:00Z" }; |
There was a problem hiding this comment.
taskChannel factory defined twice
The taskChannel helper function is defined identically inside both describe("useChannelMutations rename") and describe("useChannelMutations delete"), violating OnceAndOnlyOnce. Lifting it to module scope (alongside the existing folder helper at line 25) would remove the duplication without changing any test behavior.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| const tasks = await client.getTasks({ channel: backendChannel.id }); | ||
| // Best-effort per task — one failed soft delete shouldn't strand | ||
| // the rest or block the channel delete (the failed task stays | ||
| // attached to the soft-deleted channel, still recoverable). | ||
| await Promise.allSettled( | ||
| tasks.map((task) => client.deleteTask(task.id)), | ||
| ); |
There was a problem hiding this comment.
getTasks silently caps at 500 tasks
getTasks hard-codes limit: 500 and returns only data.results without paginating further — so any channel with more than 500 tasks will have the excess tasks silently skipped in this loop. The channel itself is still soft-deleted afterwards, so all data remains recoverable, but the updated UI copy ("Tasks in this channel are deleted with it") would be inaccurate for large channels. Either paginate the fetch or document the 500-task cap in a code comment so future readers don't assume completeness.
Problem
Channels exist in two places: the desktop file-system folder (the sidebar entry) and a backend task channel that owns the task feed and thread messages — and the UI maps one onto the other by name. That made two channel actions lossy:
Changes
renameTaskChannel/deleteTaskChannelto the api-client (the backend endpoints already existed; the client never called them).How did you test this?
useChannels.test.tsxcovering the rename lockstep/rollback paths and the delete ordering/failure paths; full@posthog/uisuite passes.turbo typecheckfor@posthog/api-client+@posthog/ui, Biome clean.Automatic notifications
Created with PostHog Code