feat(inbox): share filters via URL params - #70699
Conversation
Mirror the inbox filter state (reviewer scope, source product, priority, sort) to the URL query string so a specific view can be shared via a link. On load a link is authoritative for the filters it carries; a bare /inbox falls back to the persisted state and reflects it back into the URL so the current view stays shareable. Generated-By: PostHog Code Task-Id: f4904064-96b4-4bcb-b45e-6abf5d5f38f2
|
Hey @andrewm4894! 👋 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 · ✅ ready▶ Open the preview
commit |
🤖 CI report
|
| File | Size | Δ vs base |
|---|---|---|
posthog-app/src/scenes/bootApp.js |
5.2 KiB | 🔺 +5.2 KiB (new) |
posthog-app/src/scenes/App.js |
22.4 KiB | 🟢 -4.5 KiB (-16.6%) |
posthog-app/src/scenes/inbox/InboxScene.js |
653.8 KiB | 🔺 +2.5 KiB (+0.4%) |
posthog-app/src/scenes/AuthenticatedShell.js |
220.9 KiB | 🔺 +1.3 KiB (+0.6%) |
Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report
✅ Eager graph — within budget
How 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 import() / React.lazy chunks are not counted.
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.21 MiB · 22 files | 🔺 +94 B (+0.0%) | ███░░░░░░░ 28.2% of 4.29 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.12 MiB · 2,974 files | 🔺 +2.4 KiB (+0.0%) | █████████░ 87.7% of 9.25 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
Largest files eagerly shipped from src/index.tsx
| 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 |
|---|---|
| 278.6 KiB | ../node_modules/.pnpm/posthog-js@1.399.5/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 |
| 234.9 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 222.1 KiB | ../node_modules/.pnpm/posthog-js@1.399.5/node_modules/posthog-js/dist/module.js |
| 164.0 KiB | src/queries/validators.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 |
| 105.9 KiB | src/lib/api.ts |
| 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
⚠️ Dist folder size — 🔺 +1.44 MiB (+0.1%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1285.87 MiB · 🔺 +1.44 MiB (+0.1%)
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| import api from 'lib/api' | ||
| import { urls } from 'scenes/urls' | ||
|
|
||
| import { INBOX_PRIORITY_OPTIONS, INBOX_SOURCE_OPTIONS } from '../filterOptions' |
There was a problem hiding this comment.
Importing ../filterOptions here creates a runtime cycle because filterOptions.tsx already imports the inbox sort types from this logic file. When the inbox scene loads the filter logic, the module can re-enter itself through filterOptions, leaving one side with partially initialized exports and breaking inbox initialization depending on module evaluation order.
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!
There was a problem hiding this comment.
Fixed in 71c8f69. Made filterOptions.tsx'''s import of the sort types type-only (import type { InboxSortDirection, InboxSortField }), so it'''s erased at build and the runtime cycle is gone. The logic keeps importing the value constants from filterOptions — only the reverse edge was the runtime one.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Note 🤖 stamphog reviewed Two substantive Codex review comments on the current head remain unresolved and unaddressed (unlike the other three, which the author fixed and replied to): sort validation checks field and direction independently, so a shared link with an unsupported combination (e.g. priority:desc) is accepted even though no UI sort option produces it, causing the displayed sort label to mismatch the actual list order; and hydrating from a URL doesn't clear a stale
Gate mechanics and policy version
Updated in place — this replaces 1 earlier stamphog review(s) on this PR. |
- Hydrate a shared link's filters via a single atomic setFilters action wired to one reportListLogic refresh, instead of fanning out separate field actions. Fixes source/priority-only links not refetching in an already-mounted session, and the multi-request race between partial filter states. - Validate the teammate scope id (isUUIDLike) so a malformed shared link falls back to the default scope rather than forwarding a bad reviewer UUID to the list API. - Make filterOptions' sort-type import type-only to break the runtime import cycle with inboxFiltersLogic. Generated-By: PostHog Code Task-Id: f4904064-96b4-4bcb-b45e-6abf5d5f38f2
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71c8f69c47
ℹ️ 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".
| let sortDirection = DEFAULT_SORT_DIRECTION | ||
| if (typeof searchParams.sort === 'string') { | ||
| const [field, direction] = searchParams.sort.split(':') | ||
| if (VALID_SORT_FIELDS.has(field) && (direction === 'asc' || direction === 'desc')) { |
There was a problem hiding this comment.
When a shared URL contains a syntactically valid but unsupported combination like sort=priority:desc or sort=updated_at:asc, this accepts it because the field and direction are validated independently. The report list then requests that ordering, while the Sort control has no matching option and falls back to displaying Priority first, so recipients can see results sorted differently from the UI label. Validate against the actual emitted sort option keys instead of the full field/direction Cartesian product.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4b6dd04. parseFilterSearchParams now validates the whole field:direction key against the actual INBOX_SORT_OPTIONS (VALID_SORT_KEYS) rather than validating field and direction independently, so an unsupported combo like priority:desc or updated_at:asc falls back to the default sort instead of driving an ordering the Sort control can't display. Added a test case for it.
| values.sortField !== parsed.sortField || | ||
| values.sortDirection !== parsed.sortDirection | ||
| if (changed) { | ||
| actions.setFilters(parsed) |
There was a problem hiding this comment.
When the inbox is already mounted with a non-empty search box, following a shared URL like /inbox?source=github only dispatches setFilters(parsed) for the URL-backed fields and leaves searchQuery intact; if the URL-backed fields already match, the changed guard skips dispatch entirely. reportListLogic still sends search: searchQuery.trim() with the list request, so the recipient sees a further narrowed list that is not represented in the shared URL. Clear search on URL hydration or include it in the URL state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4b6dd04. Rather than clearing search on hydration (which would clobber an in-app search on ordinary navigation, since urlToAction also fires when opening a report), I made searchQuery part of the URL state: it's now included in the search param, hydrated via setFilters, and covered by the changed guard. So a shared link reproduces the search too, and following /inbox?source=github resets a recipient's stale search. It's still not persisted to localStorage, matching desktop.
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Contained frontend feature (URL-shareable inbox filters) by an owning-team author; the P1-level import-cycle and malformed-UUID security concerns were fixed with tests, and no in-flight (👀) review or maintainer hold exists.
- Author wrote 0% of the modified lines and has 42 merged PRs in these paths (familiarity MODERATE).
- chatgpt-codex-connector[bot] reviewed the current head.
- Two unresolved Codex P2 comments remain: unsupported sort field/direction combos (e.g. priority:desc) are accepted into state without matching a UI sort option, and a stale in-session search query isn't cleared when hydrating from a shared-filter URL, so results can silently differ from what the URL/link implies. Neither is a crash/data-loss/security issue, but worth a follow-up fix or reply to Codex.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 184L, 4F substantive, 271L/5F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (271L, 5F, single-area, feat) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ 1231bb9 · reviewed head 71c8f69 |
Fix import ordering in inboxSceneLogic and wrap a long object literal in the filters test so oxfmt --check passes. Generated-By: PostHog Code Task-Id: f4904064-96b4-4bcb-b45e-6abf5d5f38f2
New commits pushed (delta classified non_trivial_delta) — stamphog approval dismissed; re-review running automatically.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| DEFAULT_SORT_FIELD as InboxSortField, | ||
| { persist: true }, | ||
| { | ||
| setSort: (_, { field }) => field, | ||
| setFilters: (_, { filters }) => filters.sortField, | ||
| }, | ||
| ], | ||
| sortDirection: [ | ||
| 'asc' as InboxSortDirection, | ||
| DEFAULT_SORT_DIRECTION as InboxSortDirection, | ||
| { persist: true }, | ||
| { | ||
| setSort: (_, { direction }) => direction, | ||
| setFilters: (_, { filters }) => filters.sortDirection, | ||
| }, | ||
| ], |
There was a problem hiding this comment.
The clearFilters action does not reset sortField and sortDirection to their defaults, only sourceProductFilter and priorityFilter. This creates inconsistent behavior where clearing filters leaves the sort in a non-default state.
When a user clicks "Clear Filters", they expect all filters to reset, but the sort will remain unchanged. This is especially problematic because:
- The URL will still contain sort params after "clearing" (line 284 includes
clearFiltersinactionToUrl) - The visual state won't match user expectations
- Shared URLs after "clearing" will still have custom sort applied
Fix by adding clearFilters handlers to both reducers:
sortField: [
DEFAULT_SORT_FIELD as InboxSortField,
{ persist: true },
{
setSort: (_, { field }) => field,
setFilters: (_, { filters }) => filters.sortField,
clearFilters: () => DEFAULT_SORT_FIELD,
},
],
sortDirection: [
DEFAULT_SORT_DIRECTION as InboxSortDirection,
{ persist: true },
{
setSort: (_, { direction }) => direction,
setFilters: (_, { filters }) => filters.sortDirection,
clearFilters: () => DEFAULT_SORT_DIRECTION,
},
],| DEFAULT_SORT_FIELD as InboxSortField, | |
| { persist: true }, | |
| { | |
| setSort: (_, { field }) => field, | |
| setFilters: (_, { filters }) => filters.sortField, | |
| }, | |
| ], | |
| sortDirection: [ | |
| 'asc' as InboxSortDirection, | |
| DEFAULT_SORT_DIRECTION as InboxSortDirection, | |
| { persist: true }, | |
| { | |
| setSort: (_, { direction }) => direction, | |
| setFilters: (_, { filters }) => filters.sortDirection, | |
| }, | |
| ], | |
| DEFAULT_SORT_FIELD as InboxSortField, | |
| { persist: true }, | |
| { | |
| setSort: (_, { field }) => field, | |
| setFilters: (_, { filters }) => filters.sortField, | |
| clearFilters: () => DEFAULT_SORT_FIELD, | |
| }, | |
| ], | |
| sortDirection: [ | |
| DEFAULT_SORT_DIRECTION as InboxSortDirection, | |
| { persist: true }, | |
| { | |
| setSort: (_, { direction }) => direction, | |
| setFilters: (_, { filters }) => filters.sortDirection, | |
| clearFilters: () => DEFAULT_SORT_DIRECTION, | |
| }, | |
| ], | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
There was a problem hiding this comment.
Thanks — I'm going to leave this one as-is. clearFilters intentionally resets only source/priority/search and not scope/sort; that's a pre-existing product decision documented on the hasActiveFilters selector ("Scope and sort are excluded … clearFilters leaves them untouched"). The URL after clearing still accurately mirrors the real state (sort preserved), so there's no new inconsistency introduced by this PR, and changing the Clear-filters semantics is outside this PR's scope. Happy to do it as a separate change if the team wants that behavior.
- Validate the sort param against the actual offered sort options instead of the field/direction Cartesian product, so an unsupported combo like priority:desc no longer sends an ordering the Sort control can't display. - Mirror the search query to the URL as well, so a shared link reproduces (and hydration resets) the search box instead of leaving a recipient's existing search silently narrowing the shared view. Generated-By: PostHog Code Task-Id: f4904064-96b4-4bcb-b45e-6abf5d5f38f2
2 participants
Problem
Inbox users had no way to share the exact view they were looking at. The author (reviewer scope), source, priority and sort filters lived only in local storage, so a link to
/inboxalways dropped the recipient into their own persisted filters. If you spotted something worth handing off ("look at the error-tracking reports assigned to me, priority first") you had to describe the filters in words.Changes
Filter state now round-trips through the URL query string:
scope— the author/reviewer scope (for-you,entire-project,teammate:<uuid>)source— comma-separated source productspriority— comma-separated prioritiessort—field:directionThe logic (
inboxFiltersLogic) gainedactionToUrl/urlToAction. Any filter change rewrites the current URL (withreplace: true, so toggles don't pile up in history). Defaults are omitted, keeping shared links clean. On load a link is authoritative for the filters it carries and resets the rest to defaults, so it reproduces the sender's view faithfully; a bare/inboxkeeps the persisted state and reflects it back into the URL so the current view is immediately shareable. Unknown or malformed param values are dropped rather than trusted.Local storage persistence is unchanged. The logic is now connected into
inboxSceneLogicso its URL sync applies on a cold deep-link before the filter bar renders.How did you test this code?
Added unit tests on the pure encode/decode functions in
inboxFiltersLogic.test.ts: a round-trip throughfilterSearchParams→parseFilterSearchParams, that default filters produce an empty param set (clean links), and that unknown source/priority/scope/sort values fall back to defaults. These guard the two ways shared links break silently — a default leaking into the URL, or an invalid param leaking into the filter state. No existing test covered URL syncing (it didn't exist before).I (Claude, the PostHog Slack app) could not run the frontend typecheck or jest locally — dependencies aren't installed in this environment — so CI is the first place these run.
Automatic notifications
Docs update
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Andy Maguire asked for the inbox author and source filters to become shareable URL params. I chose to sync the full filter/sort set (scope, source, priority, sort) rather than just the two named ones, so a link reproduces the whole view. Key design decision: make the link authoritative on load but fall back to persisted state on a bare URL, which keeps existing local-storage persistence intact while enabling sharing. Invoked the
/writing-testsskill before adding tests and kept them at the cheapest rung (pure functions) rather than a full kea-router harness test.Created with PostHog from a Slack thread