fix(conversations): link ticket mention notifications to the ticket - #73510
Conversation
Ticket internal notes are stored as comments with scope="Ticket", but the comment mention notification system had no mapping for that scope. As a result "@x mentioned you" notifications for support tickets carried no source type or URL, so clicking them navigated nowhere. Register the "Ticket" scope so mentions resolve to /support/tickets/{id}: - add a TICKET SourceType and map the scope to it (fixes the in-app/bell click target) - map the scope to the ticket path (fixes the mention email and analytics link) - handle the ticket source type on the frontend notification row Generated-By: PostHog Code Task-Id: 7ab9a563-c3af-4f59-86d1-9f77182fbb26
|
Hey @xljones! 👋 It looks like your git author email on this PR isn't your
You can fix it for this repo with: git config user.email "you@posthog.com"Or set it globally with |
🦔 Hogbox preview · ❌ build failedThe preview didn't come up for commit Previews are optional and never block merging. A failure here is often a hogland or tailnet hiccup rather than anything in your PR, so the check stays green and this comment is the status. |
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
posthog/models/comment/utils.py:25
**Ticket comment scope remains unmapped**
When a support ticket mention comes from a comment with `scope="conversations_ticket"`, this `"Ticket"` mapping does not match `comment.scope`, causing the source type and ticket URL lookups to remain empty and the notification link to continue going nowhere.
Reviews (1): Last reviewed commit: "fix(conversations): link ticket mention ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Ticket internal notes are stored as comments with scope "conversations_ticket" (the scope="Ticket" values are activity-log entries, not comments). Key the mention scope maps on "conversations_ticket" so the source type and ticket URL actually resolve. Generated-By: PostHog Code Task-Id: 7ab9a563-c3af-4f59-86d1-9f77182fbb26
The "conversations_ticket" scope literal breaks the PascalCase model-name convention the other comment scopes follow. Add a comment so the next reader knows it must match the literal the conversations product writes. Generated-By: PostHog Code Task-Id: 7ab9a563-c3af-4f59-86d1-9f77182fbb26
🤖 CI report✅ Bundle size — no changeUncompressed size of every built Total: 64.39 MiB · no change No file changed by more than 1000 B. Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report ✅ Eager graph — within budgetHow much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy
🟢 Largest files eagerly shipped from
|
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 762 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 281.5 KiB | ../node_modules/.pnpm/posthog-js@1.407.2/node_modules/posthog-js/dist/rrweb.js |
| 267.7 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 236.0 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 226.1 KiB | ../node_modules/.pnpm/posthog-js@1.407.2/node_modules/posthog-js/dist/module.js |
| 154.3 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 106.2 KiB | src/lib/api.ts |
| 94.0 KiB | ../packages/quill/packages/quill/dist/index.js |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Toolbar bundle — eager 2.18 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.18 MiB · 17 files | no change | ████░░░░░░ 38.1% of 5.72 MiB |
| Deferred (lazy) | 2.07 MiB · 33 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.1 KiB | no change | █░░░░░░░░░ 5.8% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 713.8 KiB | dist/toolbar/toolbar-app-HLTHZTTF.css |
| 545.1 KiB | dist/toolbar/chunk-chunk-JXZAQF2A.js |
| 484.2 KiB | dist/toolbar/chunk-chunk-BQIRV2TJ.js |
| 133.6 KiB | dist/toolbar/chunk-chunk-YBAFBB2V.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-T5KY5WYR.js |
| 71.0 KiB | dist/toolbar/toolbar-app-HJ3A3JN3.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-27JL52RE.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-YSBEFDVO.js |
| 20.9 KiB | dist/toolbar/chunk-chunk-2H2TQVQG.js |
| 12.2 KiB | dist/toolbar/chunk-chunk-PIK3PADE.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — 🔺 +39.0 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1353.42 MiB · 🔺 +39.0 KiB (+0.0%)
2 participants
Problem
Clicking a
@X mentioned younotification for a support ticket goes nowhere.Support ticket internal notes are stored as generic comments with
scope="Ticket", but the comment mention notification system was never wired up for that scope.SCOPE_TO_SOURCE_TYPEandSCOPE_TO_PATH_MAPPING(inposthog/models/comment/utils.py) listed every other commentable scope but notTicket, so ticket mentions produced a notification with nosource_typeand no resolvable path. On the frontend,buildNotificationSourcePaththen returnsnull, so the notification row marks itself read on click but never navigates.Why: a teammate flagged (via Slack) that clicking a ticket-mention notification did nothing. Mentions on tickets should open the ticket.
Changes
Register the
Ticketscope so mentions resolve to/support/tickets/{id}:TICKETSourceTypeand map the scope to it — fixes the in-app/bell notification click targetSCOPE_TO_PATH_MAPPING— fixes the mention email link and the analyticsitem_url, which had the same latent gapticketsource type in the frontend notification source-path builderThe ticket id is already stored as the comment's
item_id, which matchessupportTicketDetail(ticketId), so no data changes are needed.How did you test this code?
Added a
buildNotificationSourcePathunit case asserting aticketsource type + id resolves to/support/tickets/{id}(the existing tests already cover every other source type; ticket was the missing one). I did not run the app or the full suite in this environment.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Authored by the PostHog Slack app from a Slack thread. Skills invoked: the repo
sending-notificationsskill (confirmed this adds a newSourceType, not a newNotificationType, so no migration is needed — the model'ssource_typeis a plainCharField). Traced the broken link from the notification title format back throughsend_mention_notifications→ the scope maps → the frontendSOURCE_TYPE_TO_PATH, and confirmed the same gap affects the mention email path.Created with PostHog from a Slack thread