From 11d9b459244628f6bf4e7ff3a1e9aaf936b39b4f Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 15 Jun 2026 11:49:51 +0100 Subject: [PATCH 1/2] fix(tasks): don't seed the slack-origin list cache on task create useCreateTask optimistically inserted the new task into every task-list cache via setQueriesData on the taskKeys.lists() prefix, which also matches the slack-origin list read by useSlackTasks. The sidebar derives a task's origin (and Slack icon) by id membership in that list because the summaries endpoint omits origin_product, so a freshly created, non-slack task briefly rendered as a Slack task until the list refetched. Scope the optimistic insert to lists that aren't filtered by origin_product so the slack-origin cache stays clean. Generated-By: PostHog Code Task-Id: d2ed2d1b-2570-49a9-aa60-87e235a904f1 --- .../tasks/useTaskCrudMutations.test.tsx | 47 ++++++++++++++++++- .../features/tasks/useTaskCrudMutations.ts | 15 +++++- 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/packages/ui/src/features/tasks/useTaskCrudMutations.test.tsx b/packages/ui/src/features/tasks/useTaskCrudMutations.test.tsx index 0bb0397a8..29e937727 100644 --- a/packages/ui/src/features/tasks/useTaskCrudMutations.test.tsx +++ b/packages/ui/src/features/tasks/useTaskCrudMutations.test.tsx @@ -1,3 +1,4 @@ +import type { Task } from "@posthog/shared/domain-types"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { renderHook } from "@testing-library/react"; import type { ReactNode } from "react"; @@ -27,7 +28,8 @@ vi.mock("@posthog/di/react", () => ({ useService: () => deletionService, })); -import { useDeleteTask } from "./useTaskCrudMutations"; +import { taskKeys } from "./taskKeys"; +import { useCreateTask, useDeleteTask } from "./useTaskCrudMutations"; function wrapper({ children }: { children: ReactNode }) { const queryClient = new QueryClient(); @@ -36,6 +38,20 @@ function wrapper({ children }: { children: ReactNode }) { ); } +function createTask(overrides: Partial = {}): Task { + return { + id: "new-task", + task_number: 1, + slug: "new-task", + title: "New task", + description: "New task", + created_at: "2026-06-15T00:00:00.000Z", + updated_at: "2026-06-15T00:00:00.000Z", + origin_product: "user_created", + ...overrides, + }; +} + describe("useDeleteTask.deleteWithConfirm", () => { beforeEach(() => { vi.clearAllMocks(); @@ -71,3 +87,32 @@ describe("useDeleteTask.deleteWithConfirm", () => { expect(ok).toBe(false); }); }); + +describe("useCreateTask.invalidateTasks", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("seeds plain list caches but not the slack-origin list", () => { + const queryClient = new QueryClient(); + const plainKey = taskKeys.list(); + const slackKey = taskKeys.list({ originProduct: "slack" }); + queryClient.setQueryData(plainKey, []); + queryClient.setQueryData(slackKey, []); + + const localWrapper = ({ children }: { children: ReactNode }) => ( + {children} + ); + const { result } = renderHook(() => useCreateTask(), { + wrapper: localWrapper, + }); + + result.current.invalidateTasks(createTask()); + + // The new, non-slack task lands in the plain list... + expect(queryClient.getQueryData(plainKey)).toHaveLength(1); + // ...but never pollutes the slack-origin list, which the sidebar reads to + // brand task icons by id membership. + expect(queryClient.getQueryData(slackKey)).toHaveLength(0); + }); +}); diff --git a/packages/ui/src/features/tasks/useTaskCrudMutations.ts b/packages/ui/src/features/tasks/useTaskCrudMutations.ts index 8e977f03f..1217cbddb 100644 --- a/packages/ui/src/features/tasks/useTaskCrudMutations.ts +++ b/packages/ui/src/features/tasks/useTaskCrudMutations.ts @@ -18,8 +18,21 @@ export function useCreateTask() { const invalidateTasks = (newTask?: Task) => { if (newTask) { + // Only seed list caches that aren't filtered by origin_product. The + // slack-origin list (used by useSlackTasks) is read by the sidebar to + // brand a task's icon by id membership; inserting a freshly created, + // non-slack task there would make it briefly render as a Slack task + // until the list refetches. queryClient.setQueriesData( - { queryKey: taskKeys.lists() }, + { + queryKey: taskKeys.lists(), + predicate: (query) => { + const filters = query.queryKey[2] as + | { originProduct?: string } + | undefined; + return !filters?.originProduct; + }, + }, (old) => insertTaskDedup(old, newTask), ); } From 6185233685b530ef0a770e846f88f2289e3d5057 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 15 Jun 2026 11:58:43 +0100 Subject: [PATCH 2/2] refactor(tasks): keep list-key shape knowledge in taskKeys Address qa-swarm review (paul + xp convergent): the create-task seed predicate reached into the query key by positional index (queryKey[2]) and re-declared a partial filters type, duplicating key-shape knowledge that belongs with taskKeys. A future key-shape change would silently reintroduce the Slack-icon bug with no test failure. - Add `TaskListFilters` type and a `taskKeys.filtersOf(queryKey)` accessor so the shape lives OnceAndOnlyOnce beside `list()`. - Name the predicate's intent (`isOriginScopedList`) and finish the comment so code and intent agree (any origin-scoped list, not just slack). - Parameterise the seed test across plain/repository/slack lists and add a test asserting all lists are still invalidated (refetched). Generated-By: PostHog Code Task-Id: d2ed2d1b-2570-49a9-aa60-87e235a904f1 --- packages/ui/src/features/tasks/taskKeys.ts | 19 ++++--- .../tasks/useTaskCrudMutations.test.tsx | 54 +++++++++++++++---- .../features/tasks/useTaskCrudMutations.ts | 19 +++---- 3 files changed, 67 insertions(+), 25 deletions(-) diff --git a/packages/ui/src/features/tasks/taskKeys.ts b/packages/ui/src/features/tasks/taskKeys.ts index 894907235..d805aec7f 100644 --- a/packages/ui/src/features/tasks/taskKeys.ts +++ b/packages/ui/src/features/tasks/taskKeys.ts @@ -1,12 +1,19 @@ +export interface TaskListFilters { + repository?: string; + createdBy?: number; + originProduct?: string; + internal?: boolean; +} + export const taskKeys = { all: ["tasks"] as const, lists: () => [...taskKeys.all, "list"] as const, - list: (filters?: { - repository?: string; - createdBy?: number; - originProduct?: string; - internal?: boolean; - }) => [...taskKeys.lists(), filters] as const, + list: (filters?: TaskListFilters) => [...taskKeys.lists(), filters] as const, + // Extract the filters object from a `list` query key. Keeps knowledge of the + // key's shape (filters live in the last slot) here, next to `list`, instead + // of letting consumers reach in by positional index. + filtersOf: (queryKey: readonly unknown[]): TaskListFilters | undefined => + queryKey[queryKey.length - 1] as TaskListFilters | undefined, allSummaries: () => [...taskKeys.all, "summaries"] as const, summaries: (ids: string[]) => [...taskKeys.allSummaries(), [...ids].sort()] as const, diff --git a/packages/ui/src/features/tasks/useTaskCrudMutations.test.tsx b/packages/ui/src/features/tasks/useTaskCrudMutations.test.tsx index 29e937727..c63b4c9c6 100644 --- a/packages/ui/src/features/tasks/useTaskCrudMutations.test.tsx +++ b/packages/ui/src/features/tasks/useTaskCrudMutations.test.tsx @@ -93,12 +93,47 @@ describe("useCreateTask.invalidateTasks", () => { vi.clearAllMocks(); }); - it("seeds plain list caches but not the slack-origin list", () => { + it.each([ + { name: "plain list", filters: undefined, expectedLength: 1 }, + { + name: "repository-scoped list", + filters: { repository: "owner/repo" }, + expectedLength: 1, + }, + { + name: "slack-origin list", + filters: { originProduct: "slack" }, + expectedLength: 0, + }, + ])( + "seeds the $name with the new task ($expectedLength entr(y/ies))", + ({ filters, expectedLength }) => { + const queryClient = new QueryClient(); + const key = taskKeys.list(filters); + queryClient.setQueryData(key, []); + + const localWrapper = ({ children }: { children: ReactNode }) => ( + + {children} + + ); + const { result } = renderHook(() => useCreateTask(), { + wrapper: localWrapper, + }); + + result.current.invalidateTasks(createTask()); + + // Origin-less lists mirror the new task; origin-scoped lists (read by the + // sidebar to brand icons by id membership) must not be seeded. + expect(queryClient.getQueryData(key)).toHaveLength( + expectedLength, + ); + }, + ); + + it("still invalidates every list, including the slack-origin one", () => { const queryClient = new QueryClient(); - const plainKey = taskKeys.list(); - const slackKey = taskKeys.list({ originProduct: "slack" }); - queryClient.setQueryData(plainKey, []); - queryClient.setQueryData(slackKey, []); + const invalidateSpy = vi.spyOn(queryClient, "invalidateQueries"); const localWrapper = ({ children }: { children: ReactNode }) => ( {children} @@ -109,10 +144,9 @@ describe("useCreateTask.invalidateTasks", () => { result.current.invalidateTasks(createTask()); - // The new, non-slack task lands in the plain list... - expect(queryClient.getQueryData(plainKey)).toHaveLength(1); - // ...but never pollutes the slack-origin list, which the sidebar reads to - // brand task icons by id membership. - expect(queryClient.getQueryData(slackKey)).toHaveLength(0); + // Seeding is scoped, but the refetch is not: all lists (slack included) are + // invalidated so they reconcile with the server. A future refactor must not + // "fix" the no-seed expectation above by dropping a list from refetch. + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: taskKeys.lists() }); }); }); diff --git a/packages/ui/src/features/tasks/useTaskCrudMutations.ts b/packages/ui/src/features/tasks/useTaskCrudMutations.ts index 1217cbddb..70111964e 100644 --- a/packages/ui/src/features/tasks/useTaskCrudMutations.ts +++ b/packages/ui/src/features/tasks/useTaskCrudMutations.ts @@ -18,19 +18,20 @@ export function useCreateTask() { const invalidateTasks = (newTask?: Task) => { if (newTask) { - // Only seed list caches that aren't filtered by origin_product. The - // slack-origin list (used by useSlackTasks) is read by the sidebar to - // brand a task's icon by id membership; inserting a freshly created, - // non-slack task there would make it briefly render as a Slack task - // until the list refetches. + // Only seed list caches that aren't scoped to a specific origin_product. + // An origin-scoped list (e.g. the slack-origin list behind useSlackTasks) + // is read by the sidebar to brand a task's icon by id membership, so + // seeding a freshly created, non-slack task into it would make that task + // briefly render as a Slack task until the list refetches. Origin-less + // lists, by contrast, should mirror every new task. queryClient.setQueriesData( { queryKey: taskKeys.lists(), predicate: (query) => { - const filters = query.queryKey[2] as - | { originProduct?: string } - | undefined; - return !filters?.originProduct; + const isOriginScopedList = Boolean( + taskKeys.filtersOf(query.queryKey)?.originProduct, + ); + return !isOriginScopedList; }, }, (old) => insertTaskDedup(old, newTask),