feat(customer-analytics): add custom property filters to the accounts list - #69415
feat(customer-analytics): add custom property filters to the accounts list#69415arthurdedeus wants to merge 6 commits into
Conversation
… list Generated-By: PostHog Code Task-Id: 3faf9d95-024e-4878-bd8b-57e2e9e46724
🤖 CI report
|
| File | Size | Δ vs base |
|---|---|---|
render-query/src/render-query/render-query.js |
24.08 MiB | 🔺 +4.7 KiB (+0.0%) |
exporter/_parent/products/customer_analytics/frontend/CustomerAnalyticsScene.js |
123.5 KiB | 🔺 +3.9 KiB (+3.2%) |
posthog-app/_parent/products/customer_analytics/frontend/CustomerAnalyticsScene.js |
124.1 KiB | 🔺 +3.9 KiB (+3.2%) |
toolbar/src/toolbar/toolbar.js |
8.20 MiB | 🔺 +1.1 KiB (+0.0%) |
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.20 MiB · 22 files | no change | ███░░░░░░░ 27.9% of 4.29 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.05 MiB · 2,947 files | 🔺 +4.2 KiB (+0.1%) | █████████░ 87.0% 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 |
| 668 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 277.1 KiB | ../node_modules/.pnpm/posthog-js@1.398.2/node_modules/posthog-js/dist/rrweb.js |
| 266.9 KiB | ../node_modules/.pnpm/@posthog+icons@0.37.4_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 224.6 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 213.6 KiB | ../node_modules/.pnpm/posthog-js@1.398.2/node_modules/posthog-js/dist/module.js |
| 164.0 KiB | src/queries/validators.js |
| 154.0 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.1 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.5 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.1_@tiptap+pm@3.20.1/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 — 🔺 +148.1 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1454.05 MiB · 🔺 +148.1 KiB (+0.0%)
ℹ️ MCP UI apps size — 31 app(s), 15641.8 KB JS
Built size of each MCP UI app (main.js + styles.css).
| App | JS | CSS |
|---|---|---|
| debug | 579.0 KB | 173.6 KB |
| action | 426.8 KB | 173.6 KB |
| action-list | 543.7 KB | 173.6 KB |
| cohort | 425.8 KB | 173.6 KB |
| cohort-list | 542.7 KB | 173.6 KB |
| email-template | 425.7 KB | 173.6 KB |
| error-details | 442.6 KB | 173.6 KB |
| error-issue | 426.5 KB | 173.6 KB |
| error-issue-list | 543.6 KB | 173.6 KB |
| experiment | 540.9 KB | 173.6 KB |
| experiment-list | 544.5 KB | 173.6 KB |
| experiment-results | 542.7 KB | 173.6 KB |
| feature-flag | 546.4 KB | 173.6 KB |
| feature-flag-list | 550.0 KB | 173.6 KB |
| feature-flag-testing | 429.6 KB | 173.6 KB |
| insight-actors | 541.6 KB | 173.6 KB |
| invite-email-preview | 425.1 KB | 173.6 KB |
| llm-costs | 539.0 KB | 173.6 KB |
| session-recording | 427.6 KB | 173.6 KB |
| session-summary | 432.9 KB | 173.6 KB |
| survey | 427.4 KB | 173.6 KB |
| survey-global-stats | 541.8 KB | 173.6 KB |
| survey-list | 544.4 KB | 173.6 KB |
| survey-stats | 541.8 KB | 173.6 KB |
| trace-span | 426.2 KB | 173.6 KB |
| trace-span-list | 543.6 KB | 173.6 KB |
| workflow | 426.2 KB | 173.6 KB |
| workflow-list | 543.1 KB | 173.6 KB |
| query-results | 724.5 KB | 173.6 KB |
| render-ui | 615.6 KB | 173.6 KB |
| visual-review-snapshots | 430.6 KB | 173.6 KB |
⚠️ Playwright — 10 failed, 4 flaky
🎭 Playwright report
❌ 10 failed tests:
- create trends insight via API and snapshot (chromium)
- See recordings action (chromium)
- password-protected insight sharing (chromium)
- CSS loads correctly on shared insight page (chromium)
- CSS fallback works when hashed CSS returns error (chromium)
- renders successfully in a logged-out browser context (chromium)
- renders successfully in a logged-out browser context (chromium)
- renders successfully in a logged-out browser context (chromium)
- plays a multi-source recording, pauses, seeks, scrubs, and reaches the end (chromium)
- creates, launches, edits and deletes new survey (chromium)
- Configure step ordering and session aggregation gating (chromium)
- creates a Postgres direct source and queries it successfully (chromium)
- See SQL Editor (chromium)
- Toolbar loads (chromium)
These issues are not necessarily caused by your changes.
Annoyed by this section? Help fix flakies and failures and it will go green!
⚠️ MCP snapshots — 9 updated (9 modified, 0 added, 0 deleted)
Snapshots: MCP unit test snapshots updated
Changes: 9 snapshots (9 modified, 0 added, 0 deleted)
What this means:
- Snapshots have been automatically updated to match current output
Next steps:
- Review the changes to ensure they're intentional
- If unexpected, investigate what caused the output to change
|
⏭️ Skipped snapshot commit because branch advanced to The new commit will trigger its own snapshot update workflow. If you expected this workflow to succeed: This can happen due to concurrent commits. To get a fresh workflow run, either:
|
|
|
Generated-By: PostHog Code Task-Id: 2ccd132a-eb71-47c4-b775-321ff679297e
Generated-By: PostHog Code Task-Id: 2ccd132a-eb71-47c4-b775-321ff679297e
![hex-security-app[bot]](https://avatars.githubusercontent.com/in/2665233?s=60&v=4){kind=small}
|
marked as ready for the bot reviews |
|
Reviews (1): Last reviewed commit: "fix(property-definitions): Mark unfetcha..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| column = "value_num" if definition.data_type == DataType.NUMERIC else "value_str" | ||
| queryset = CustomPropertyValue.objects.for_team(team_id).filter(definition_id=definition.id, is_deleted=False) | ||
| if needle and column == "value_str": | ||
| queryset = queryset.filter(value_str__icontains=needle) | ||
| values = ( | ||
| queryset.exclude(**{f"{column}__isnull": True}) | ||
| .values_list(column, flat=True) | ||
| .distinct() | ||
| .order_by(column)[:VALUE_SUGGESTIONS_LIMIT] | ||
| ) | ||
| if column == "value_num": | ||
| formatted = [_format_numeric_suggestion(value) for value in values] | ||
| return [value for value in formatted if needle in value] if needle else formatted | ||
| return list(values) |
There was a problem hiding this comment.
For numeric properties, the VALUE_SUGGESTIONS_LIMIT slice is applied to the database query before the Python-side needle filter runs. If a team has more than 50 distinct numeric values, a search term that matches only values ranked beyond the 50th will return nothing — even though matching values exist. For example, with values 1–100 and search "9", the DB returns [1…50], Python filters to ["9", "19", "29", "39", "49"], but "59", "69", …, "99" are silently dropped.
The string path avoids this by pushing the icontains filter into the queryset before slicing. A simple fix is to fetch more rows from the DB (e.g., VALUE_SUGGESTIONS_LIMIT * 10) and then apply the limit after the Python filter.
# Conflicts: # products/customer_analytics/frontend/components/Accounts/AGENTS.md # products/customer_analytics/frontend/components/Accounts/accountsColumnConfigLogic.ts # products/customer_analytics/frontend/components/Accounts/accountsLogic.ts
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
| key = request.GET.get("key") | ||
| if not key: | ||
| return Response({"results": [], "refreshing": False}) | ||
| suggestions = api.list_custom_property_value_suggestions(self.team_id, key, request.GET.get("value")) |
There was a problem hiding this comment.
Medium: Account value suggestions bypass account access
An authenticated user who can access this endpoint can call it with a custom-property definition id and enumerate distinct values across all accounts in the team, including accounts they are not allowed to read. Pass self.user_access_control through the facade and restrict the CustomPropertyValue query to readable account ids before returning distinct suggestions.
PR overviewThis pull request adds custom property filtering support to the customer analytics accounts list, including backend view logic for suggesting account property values. There is one open security issue: authenticated users can request custom-property value suggestions that are not limited to accounts they are permitted to read, allowing enumeration of values across the broader team. No issues have been fixed yet, so the PR still needs access-control enforcement in the value-suggestion path before it is in a safe state. Open issues (1)
Fixed/addressed: 0 · PR risk: 7/10 |
| key = request.GET.get("key") | ||
| if not key: | ||
| return Response({"results": [], "refreshing": False}) | ||
| suggestions = api.list_custom_property_value_suggestions(self.team_id, key, request.GET.get("value")) |
There was a problem hiding this comment.
The new merge commit did not address the earlier issue: values still calls list_custom_property_value_suggestions directly on a caller-supplied definition id instead of authorizing the definition through get_custom_property_definition(..., user_access_control=...). A user who can hit this endpoint can still enumerate active values for a definition they are not allowed to view, leaking customer property data across the definition RBAC boundary.
Prompt To Fix With AI
Before returning suggestions, look up the target definition through the same RBAC-aware path used by `retrieve` (e.g. `api.get_custom_property_definition(self.team_id, key, user_access_control=self.user_access_control)`) and return 404 when that lookup fails. Only then call the suggestion logic. Add an access-control regression test proving a caller without read access to that definition cannot use `/values/` to enumerate its values.Severity: medium | Confidence: 89% | React with 👍 if useful or 👎 if not
| ]), | ||
| type: zod | ||
| .literal('workflow_variable') | ||
| .literal('account_custom_property') |
There was a problem hiding this comment.
The new generated MCP schemas now accept type: 'account_custom_property' filters on generic endpoints, error-tracking rules, experiments, and logs alerts. Those surfaces operate on events/logs/error issues rather than customer-analytics accounts, so this expands their accepted input beyond the backend’s intended trust boundary and can let MCP clients submit account-only filter shapes to tools that were never designed or reviewed to process account data. That creates a cross-surface data-access footgun for MCP consumers and any downstream executor that treats the generated schemas as authoritative authorization/validation.
Prompt To Fix With AI
Regenerate or hand-fix the MCP Zod schemas so `account_custom_property` is only admitted on customer-analytics surfaces that actually support account filters. Remove it from generic endpoint-run, error-tracking, experiments, and logs tool schemas unless the corresponding server-side APIs explicitly support and authorize account custom-property filtering. Add a regression test that these unrelated MCP tool schemas reject `type: 'account_custom_property'`.Severity: medium | Confidence: 81% | React with 👍 if useful or 👎 if not
|
👋 Visual changes detected for this PR. Review and approve in PostHog Visual Review If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix. |
|
Split into three PRs by layer for reviewability:
All dev-server fixes from this branch (boolean/date filters, propertyDefinitionsModel unfetchable types) are carried over. |
1 participant
Problem
Teams can define custom account properties (populated from warehouse sources or workflows), but there is no way to filter the accounts list by them.
Changes
PropertyFilterscomponent (same look and feel as the persons list)AccountCustomPropertiestaxonomic group +account_custom_propertyfilter type across the taxonomy plumbing (legacy filter and rebuild menu)valuesaction on the definitions viewset)custom_propertieslazy join, ANDed into the existingfilterExpressionso list rows and overview tiles stay consistent#view=URL hash, keyed by definition id so they survive renamesHow did you test this code?
Unit tests: jest for the expression builder (escaping, per-type casts, incomplete filters degrade to no-op), view-state round-trip, and the existing Accounts/TaxonomicFilter/PropertyFilters suites; pytest for the value-suggestions logic (select labels, soft-delete exclusion, team isolation), the endpoint envelope, and a query-runner test proving a WHERE-only reference expands the custom-properties lazy join (no existing test covered that path).
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Automatic notifications
Docs update
No
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Authored by Claude Code (PostHog Code). Skills invoked: /modifying-taxonomic-filter (group mirrored across the legacy logic and the rebuild's
buildTaxonomicGroups), /improving-drf-endpoints, /writing-tests.Notable decisions: filters compile client-side into
filterExpressionfragments instead of adding apropertiesfield toAccountsQuery— the lazy-join expansion from a WHERE reference already works and tile/account-id filters use the same hook; filter keys are definition ids (not names) so saved views survive renames, with the name carried on the filter'slabelfor display; the operator dropdown is narrowed to exactly the set the expression builder can compile.Created with PostHog Code