fix(legal-documents): guard against loading before organization resolves - #76415
fix(legal-documents): guard against loading before organization resolves#76415posthog[bot] wants to merge 1 commit into
Conversation
The `/legal` scene loader and delete listener trusted `currentOrganizationId`, which falls back to the `'@current'` sentinel while the organization is still loading. That sentinel can't be resolved by the backend and 404s, which the loader didn't catch, surfacing an uncaught "Organization not found." error. Guard both call sites on the real `currentOrganization.id` instead, and treat a 404 the same as a 403 (empty list) so a transient unresolvable organization degrades quietly. Added `kea-test-utils` as a devDependency to `products/legal_documents/package.json` so the new logic test type-checks and runs (this product was missing it, unlike sibling products that already have kea logic tests). Generated-By: PostHog Code Task-Id: 77440163-9f91-47a6-8780-9d0e8138c0cf
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
🤖 CI report✅ Bundle size — no changeUncompressed size of every built Total: 65.51 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 |
|---|---|
| 285.6 KiB | ../node_modules/.pnpm/posthog-js@1.409.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 |
| 235.5 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 231.5 KiB | ../node_modules/.pnpm/posthog-js@1.409.5/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 |
| 105.3 KiB | src/lib/api.ts |
| 94.7 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.19 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.19 MiB · 17 files | no change | ████░░░░░░ 38.3% of 5.72 MiB |
| Deferred (lazy) | 2.08 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 |
|---|---|
| 718.3 KiB | dist/toolbar/toolbar-app-FT4UJ3IP.css |
| 551.4 KiB | dist/toolbar/chunk-chunk-AGSGCCBT.js |
| 484.6 KiB | dist/toolbar/chunk-chunk-T44C2V5C.js |
| 133.6 KiB | dist/toolbar/chunk-chunk-KBLX73CM.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-T5KY5WYR.js |
| 71.0 KiB | dist/toolbar/toolbar-app-FKUW2FOM.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-27JL52RE.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-3JVI3ZTF.js |
| 20.9 KiB | dist/toolbar/chunk-chunk-HJ3ZJMTU.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 — 🔺 +456 B (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1381.30 MiB · 🔺 +456 B (+0.0%)
Problem
/legalscene loads its document list keyed oncurrentOrganizationId, but that selector (frontend/src/scenes/organizationLogic.tsx) falls back to the literal string'@current'while the organization is still loading (aTODOthere notes this replaced a throw).legalDocumentsLogic's loader and delete listener only checked that this id was truthy, so a request could fire againstGET /api/organizations/@current/legal_documents/.'@current'to an organization and returns 404Organization not found.(IsOrganizationAdminOrOwner), which the loader's catch only handled for 403 — so the 404 surfaced as an uncaught exception.deleteLegalDocument, but that listener only issues a DELETE. The actual failing call is the list GET dispatched synchronously fromloadLegalDocuments()inside it, so the stack is just inherited from that async call.Impact was low-volume but real: a handful of exceptions from distinct users on
/legal, hitting organization admins/owners trying to generate a DPA or BAA. A page reload works around it, but it's a rough edge on a compliance-adjacent flow.Changes
loadLegalDocumentsand thedeleteLegalDocumentlistener now guard oncurrentOrganization?.idinstead of the string selector, so they no-op while the organization is still loading rather than firing a request against the'@current'sentinel.kea-test-utilsas a devDependency toproducts/legal_documents/package.json(it was missing, unlike sibling products with kea logic tests) and a regression test that reproduces the race by mounting the logic before the organization resolves and asserting the API is never called with the sentinel.The underlying
TODOinorganizationLogic.tsx(restoring the throw once all callers are fixed) is a bigger sweep and out of scope here.How did you test this code?
products/legal_documents/frontend/scenes/legalDocumentsLogic.test.ts, which mounts the logic with no organization in context and assertslegalDocumentsListis never called andlegalDocumentsresolves to[]. Verified it fails against the pre-fix code (calls the API with'@current') and passes with the fix.pnpm --filter=@posthog/frontend typescript:checkandpnpm --filter=@posthog/frontend fix(oxlint + oxfmt) — no new errors introduced by this change.Automatic notifications
Docs update
N/A — internal bug fix, no user-facing workflow change.
🤖 Agent context
Autonomy: Fully autonomous
organizationLogic.tsx,legalDocumentsLogic.ts, and the backend permission class to confirm the race and the exact 404 path./writing-tests(test value gate — confirmed the new test catches a regression no existing test did, by reverting the fix and observing the test fail).legalDocumentsLogiccall sites rather than restoring the throw inorganizationLogic.currentOrganizationId, since that selector has other callers that would need the same audit — flagged as a larger follow-up in the existingTODO.Created with PostHog Desktop from this inbox report.