feat(sidebar): show Slack icon for tasks from Slack threads#2281
feat(sidebar): show Slack icon for tasks from Slack threads#2281andrewm4894 wants to merge 4 commits into
Conversation
Threads `origin_product` through the sidebar task list and command palette so tasks created from Slack (origin_product === "slack") show a Slack icon instead of the generic chat-bubble / cloud icon. Cloud tasks that came from Slack keep their running / completed / failed color coding on the Slack logo. The /tasks/summaries/ endpoint doesn't currently include origin_product, so we fetch the slack-origin subset separately and intersect by id in useSidebarData. Drive-by: dedupe a duplicate INBOX_VIEWED entry in analytics.ts (which was breaking the typecheck pre-commit hook on main) and drop a stale no-property INBOX_VIEWED call in navigationStore; the canonical track-with-properties call in InboxSignalsTab is what we want to keep. Generated-By: PostHog Code Task-Id: 030bba98-3ca5-403e-82f2-812c8158910b
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ca718908d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/code/src/renderer/features/tasks/hooks/useTasks.ts:78-87
`useSlackTasks` doesn't accept an `options` parameter, so callers can't conditionally disable it (e.g. when `showAllUsers=true`). Adding an `options` argument — consistent with `useTaskSummaries` — allows the caller in `useSidebarData` to skip the fetch when it's redundant.
```suggestion
export function useSlackTasks(options?: { enabled?: boolean }) {
return useAuthenticatedQuery<Task[]>(
taskKeys.list({ originProduct: "slack" }),
(client) =>
client.getTasks({ originProduct: "slack" }) as unknown as Promise<Task[]>,
{
refetchInterval: TASK_LIST_POLL_INTERVAL_MS,
enabled: options?.enabled ?? true,
},
);
}
```
### Issue 2 of 2
apps/code/src/renderer/features/sidebar/hooks/useSidebarData.ts:137
`useSlackTasks` is called unconditionally even when `showAllUsers=true`. In that path, `rawTasks` is built from `fullTasks` which already maps `origin_product: t.origin_product` directly, so `slackTaskIds` is never actually needed to derive `originProduct` (the `task.origin_product ?? ...` short-circuits). The hook still fires on mount and re-polls every 30 s, and `slackTaskIds` is in the `taskData` dependency array, so any refetch triggers a full `taskData` recomputation even though the result is unaffected. Disabling the hook when `showAllUsers` is true eliminates the redundant request and memo churn.
```suggestion
const { data: slackTasks = [] } = useSlackTasks({ enabled: !showAllUsers });
```
Reviews (1): Last reviewed commit: "feat(sidebar): show Slack icon for tasks..." | Re-trigger Greptile |
| { showAllUsers, showInternal }, | ||
| { enabled: showAllUsers }, | ||
| ); | ||
| const { data: slackTasks = [] } = useSlackTasks(); |
There was a problem hiding this comment.
useSlackTasks is called unconditionally even when showAllUsers=true. In that path, rawTasks is built from fullTasks which already maps origin_product: t.origin_product directly, so slackTaskIds is never actually needed to derive originProduct (the task.origin_product ?? ... short-circuits). The hook still fires on mount and re-polls every 30 s, and slackTaskIds is in the taskData dependency array, so any refetch triggers a full taskData recomputation even though the result is unaffected. Disabling the hook when showAllUsers is true eliminates the redundant request and memo churn.
| const { data: slackTasks = [] } = useSlackTasks(); | |
| const { data: slackTasks = [] } = useSlackTasks({ enabled: !showAllUsers }); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/features/sidebar/hooks/useSidebarData.ts
Line: 137
Comment:
`useSlackTasks` is called unconditionally even when `showAllUsers=true`. In that path, `rawTasks` is built from `fullTasks` which already maps `origin_product: t.origin_product` directly, so `slackTaskIds` is never actually needed to derive `originProduct` (the `task.origin_product ?? ...` short-circuits). The hook still fires on mount and re-polls every 30 s, and `slackTaskIds` is in the `taskData` dependency array, so any refetch triggers a full `taskData` recomputation even though the result is unaffected. Disabling the hook when `showAllUsers` is true eliminates the redundant request and memo churn.
```suggestion
const { data: slackTasks = [] } = useSlackTasks({ enabled: !showAllUsers });
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Adopted in 0e28236 — useSlackTasks({ enabled: !showAllUsers, showInternal }). The showAllUsers path keeps short-circuiting on task.origin_product from fullTasks, so no fetch and no memo churn there.
Address bot review feedback on the Slack icon PR:
- `useSlackTasks` now accepts `{ enabled, showInternal }` so callers can both
gate the fetch and forward the same visibility scope the sidebar uses.
- `useSidebarData` disables the slack query when `showAllUsers=true` — that
path already maps `origin_product` off `fullTasks`, so the extra request and
the resulting `taskData` memo churn are redundant.
- Pass `showInternal` through so staff viewing internal tasks still get the
Slack icon on internal slack-origin tasks.
Generated-By: PostHog Code
Task-Id: 030bba98-3ca5-403e-82f2-812c8158910b
joshsny
left a comment
There was a problem hiding this comment.
thanks for adding this, think we should change to originProduct so we can use this for other sources too
| return ( | ||
| <CloudStatusIcon | ||
| taskRunStatus={taskRunStatus} | ||
| isFromSlack={isFromSlack} |
There was a problem hiding this comment.
it'd be nice to plumb this as originProduct so that it can be extended to other products also
There was a problem hiding this comment.
Good shout — done in 1d295a2. CloudStatusIcon now takes originProduct (string), and there's a small ORIGIN_PRODUCT_META map at the top of the file keyed by origin_product value, so adding another product is one line. The default-fallback branch (line 230-ish) uses the same lookup, so any other product registered in the map automatically gets a labeled icon there too.
Per review (joshsny): swap the boolean `isFromSlack` prop on `CloudStatusIcon` for the raw `originProduct` string, and look up the icon + label from a small `ORIGIN_PRODUCT_META` map. Adding a new branded origin product (email, support, …) is now a one-line addition rather than another boolean prop. Generated-By: PostHog Code Task-Id: 030bba98-3ca5-403e-82f2-812c8158910b
Clicking the Slack icon on a slack-origin task now opens the originating thread in the user's default browser, so the user can jump back into Slack to ping someone or continue the conversation without leaving and re-finding the thread manually. Plumbing: - `task.latest_run.state.slack_thread_url` is a pre-built URL on the full Task. `useSlackTasks` already fetches full Tasks, so for the summaries sidebar path we build a `Map<taskId, url>` from that query. For the showAllUsers path, the URL is pulled directly out of the rawTasks mapping. - Threaded `slackThreadUrl` through `TaskData → TaskRow → TaskItem → TaskIcon` and also through `CommandMenu`'s `TaskCommandIcon`. - Added an `IconWrapper` / `IconLink` helper inside `TaskIcon` that renders the icon as a `role="link"` span (a real `<a>` would be invalid inside the parent `SidebarItem` `<button>`). Clicks `stopPropagation` so opening the thread doesn't also navigate to the task, and Enter / Space trigger the same `trpcClient.os.openExternal` call. - Tooltip flips to "Open Slack thread" when the icon is clickable. Generated-By: PostHog Code Task-Id: 030bba98-3ca5-403e-82f2-812c8158910b
Problem
Tasks created from Slack threads look identical to tasks created in-app. From the Slack thread:
Changes
origin_product === "slack"in the sidebarTaskIcon(used by both the sidebar list and the command palette).ph-pulseanimation while running). Tooltip becomesSlack (running)/Slack (completed)/ etc.From Slack threadtooltip.originProductthroughTaskRow → TaskItem → TaskIconand throughCommandMenu'sTaskCommandIcon.useSlackTasks()hook. The/tasks/summaries/endpoint doesn't includeorigin_product, so we fetch the slack-origin subset separately (getTasks({ originProduct: "slack" })) and intersect by id inuseSidebarDatato flag tasks. For theshowAllUserspath (which already uses full tasks), we readorigin_productdirectly off the task.Drive-by
INBOX_VIEWEDentry inanalytics.tsintroduced in feat(inbox): instrument analytics for engagement events #2228 — it was failing typecheck (TS1117/TS2717) and blocking the localpre-commithook.track(ANALYTICS_EVENTS.INBOX_VIEWED)call innavigationStorethat didn't pass the required properties. The canonical track-with-properties call inInboxSignalsTabalready fires when the inbox actually loads.How did you test this?
tsc -p tsconfig.web.json --noEmitno longer reports the analytics duplicate /navigationStoreerrors that were on main.pnpm exec vitest runover the touched feature folders — 23 / 23 sidebar tests pass. (One pre-existing test suite —useTaskPrStatus.test.ts— fails to resolve@posthog/electron-trpc/renderer; same onmain, unrelated.)pnpm exec biome checkover the 6 touched files — clean.Publish to changelog?
no
Created with PostHog Code