Scope owner inputs to admin businesses; keep membership names readable - #4296
Open
gilgardosh wants to merge 2 commits into
Open
Scope owner inputs to admin businesses; keep membership names readable#4296gilgardosh wants to merge 2 commits into
gilgardosh wants to merge 2 commits into
Conversation
Every "Owner" input listed all businesses or financial entities, including counterparties that can never be an owner. They now read allAdminBusinesses, as the charges filter already did. Where there is a single option the input is disabled and shows it instead of pretending to offer a choice; single-select owner fields also pre-select that value so the submitted filter matches what is displayed. The Green Invoice sync modal loses its hard-coded owner UUID. Also fixes the user-menu business switcher rendering out-of-scope memberships as bare UUIDs: membership names resolve through financial_entities, which RLS narrows to the requested X-Business-Scope, so after the first scoped request the names of the user's other businesses — the very list needed to leave that scope — became unreadable. A permissive SELECT policy lets a user always read the financial_entities row of a business they are a member of; API-key requests, which carry no user id, are unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AfgxehTempoyjd5hewfdYm
Contributor
🚀 Snapshot Release (
|
| Package | Version | Info |
|---|---|---|
@accounter/client |
0.1.0-alpha-20260826143211-737d372de7c376288a44cdbce576ea6a53346625 |
npm ↗︎ unpkg ↗︎ |
@accounter/green-invoice-graphql |
0.8.7-alpha-20260826143211-737d372de7c376288a44cdbce576ea6a53346625 |
npm ↗︎ unpkg ↗︎ |
@accounter/hashavshevet-mesh |
0.2.13-alpha-20260826143211-737d372de7c376288a44cdbce576ea6a53346625 |
npm ↗︎ unpkg ↗︎ |
@accounter/israeli-vat-scraper |
0.1.13-alpha-20260826143211-737d372de7c376288a44cdbce576ea6a53346625 |
npm ↗︎ unpkg ↗︎ |
@accounter/modern-poalim-scraper |
0.11.0-alpha-20260826143211-737d372de7c376288a44cdbce576ea6a53346625 |
npm ↗︎ unpkg ↗︎ |
@accounter/payper-mesh |
0.2.13-alpha-20260826143211-737d372de7c376288a44cdbce576ea6a53346625 |
npm ↗︎ unpkg ↗︎ |
@accounter/scraper-app |
0.0.3-alpha-20260826143211-737d372de7c376288a44cdbce576ea6a53346625 |
npm ↗︎ unpkg ↗︎ |
@accounter/server |
0.2.0-alpha-20260826143211-737d372de7c376288a44cdbce576ea6a53346625 |
npm ↗︎ unpkg ↗︎ |
@accounter/shaam-uniform-format-generator |
0.2.7-alpha-20260826143211-737d372de7c376288a44cdbce576ea6a53346625 |
npm ↗︎ unpkg ↗︎ |
@accounter/shaam6111-generator |
0.1.9-alpha-20260826143211-737d372de7c376288a44cdbce576ea6a53346625 |
npm ↗︎ unpkg ↗︎ |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The Documents Owners multi-select can display the sole admin business while react-hook-form still submits a different/empty ownerIDs value because the form state isn’t synchronized when the field is disabled.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR tightens “Owner”/business selection across the client so users can only act as owners for businesses they actually administer, and it fixes business-switcher readability under scoped RLS by allowing membership-business name resolution regardless of the current X-Business-Scope.
Changes:
- Client: switch multiple “Owner” pickers from all businesses/financial entities to admin businesses; add “sole admin business” handling to lock and (in most cases) preselect when only one option exists.
- Server/DB: add an RLS
FOR SELECTpolicy onfinancial_entitiesto allow reading names for businesses the user is a member of (even when scoped), preventing membership names from degrading to UUIDs. - UX/API safety: remove hard-coded Green Invoice owner default and make the owner field required.
File summaries
| File | Description |
|---|---|
| packages/server/src/modules/common/resolvers/user-context.resolver.ts | Adds resolver comment clarifying membership name loads rely on the new RLS policy. |
| packages/migrations/src/run-pg-migrations.ts | Registers the new migration in the migration runner. |
| packages/migrations/src/actions/2026-08-26T10-00-00.rls-membership-financial-entities.ts | Introduces membership_business_visibility RLS policy to read financial_entities for membership businesses. |
| packages/client/src/hooks/use-get-admin-businesses.ts | Adds soleAdminBusinessId to support “single option means locked input” behavior. |
| packages/client/src/components/screens/reports/shaam-6111-report/shaam-6111-filters.tsx | Switches owner/business picker to admin businesses; auto-selects and locks when sole option. |
| packages/client/src/components/screens/reports/depreciation-report/depreciation-report-filters.tsx | Switches Owner selector to admin businesses; auto-selects/locks for sole option. |
| packages/client/src/components/screens/reports/balance-report/balance-report-filters.tsx | Uses soleAdminBusinessId to preselect/lock owner when only one admin business exists. |
| packages/client/src/components/screens/documents/all-documents/documents-filters.tsx | Changes Owners filter to use admin businesses (instead of all financial entities). |
| packages/client/src/components/reports/vat-monthly-report/vat-monthly-report-filters.tsx | Uses soleAdminBusinessId to preselect/lock issuer. |
| packages/client/src/components/reports/validations/validate-reports-filter.tsx | Switches “report issuer” to admin businesses; preselect/lock when sole option. |
| packages/client/src/components/reports/trial-balance-report/trial-balance-report-filters.tsx | Switches Owners multi-select to admin businesses and locks when sole option. |
| packages/client/src/components/reports/dynamic-report/toolbar.tsx | Adds ownerDisabled support to render owner select read-only when sole option. |
| packages/client/src/components/reports/dynamic-report/index.tsx | Replaces hard-coded owner list with useGetAdminBusinesses; locks/report-filtering uses sole owner when applicable. |
| packages/client/src/components/common/modals/sync-green-invoice-documents-modal.tsx | Removes hard-coded owner UUID default; requires owner; preselect/lock when sole option. |
| packages/client/src/components/charges/charges-filters/sections/entities-section.tsx | Changes Owners section to show (and lock) even for single-owner tenants. |
| packages/client/src/components/charges/charges-filters/negatable-multi-select-field.tsx | Adds disabled prop passthrough for MultiSelectField. |
| packages/client/src/components/business-ledger/business-ledger-filters.tsx | Switches Owners multi-select to admin businesses and locks when sole option. |
| .changeset/owner-selects-admin-businesses.md | Adds a changeset documenting the owner scoping + membership-name/RLS fix. |
Review details
- Files reviewed: 18/18 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A disabled field never fires onChange, so the locked owner shown in the documents, business-ledger and trial-balance filters was display-only: the submitted filter kept whatever ownerIds/ownerIDs the form was seeded with, which need not be the sole admin business (the default derives from the active read scope, not from businesses_admin). Pre-select it via setValue so the input and the submitted filter agree. Addresses review feedback on packages/client/src/components/screens/documents/all-documents/documents-filters.tsx. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AfgxehTempoyjd5hewfdYm
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two related fixes around "which businesses am I acting as?".
1. Owner inputs offer only businesses the user owns
Every "Owner" input listed all businesses or all financial entities — including counterparties that can never be an owner. They now read
allAdminBusinesses, which is what the charges filter already did:useGetBusinessesuseGetAdminBusinessesuseGetBusinessesuseGetAdminBusinessesuseGetFinancialEntitiesuseGetAdminBusinessesuseGetFinancialEntitiesuseGetAdminBusinesses[{ id: adminBusinessId, name: username }]useGetAdminBusinessesuseGetBusinessesuseGetAdminBusinessesuseGetBusinessesuseGetAdminBusinessesWhere there is only one option, the input is disabled and shows it rather than pretending to offer a choice. Single-select owner fields (VAT monthly, balance report, depreciation, PCN874, Shaam-6111, Green Invoice sync) also pre-select that value, so the submitted filter matches what is displayed. The charges filter's Owners field switches from being hidden on a single owner to being shown and locked, for consistency.
useGetAdminBusinessesgainssoleAdminBusinessId, which is the single option's id ornull.The Green Invoice sync modal also loses its hard-coded owner UUID default (
6a20aa69-…) in favour of the sole admin business, and the field is nowrequired.2. Business switcher no longer degrades to bare UUIDs
Query.userContext.membershipsresolves each business name throughfinancial_entities, which RLS narrows to the request'sX-Business-Scope. So on the first (unscoped) fetch the user menu showed names, and on every fetch afterwards the memberships outside the chosen scope came back nameless — the very list a user needs in order to leave a narrow scope became unreadable.New migration
2026-08-26T10-00-00.rls-membership-financial-entities: a permissiveFOR SELECTpolicy onfinancial_entitiesletting a user always read the row of a business they are a member of. It ORs withtenant_isolation, so it only ever adds visibility, and only for the caller's own memberships.get_current_user_id()is NULL for API-key requests, which leaves the subquery empty and those requests unchanged.Testing
yarn lint— 0 errorsyarn workspace @accounter/client build(tsc && vite build) — cleanyarn test— 3776 passed; the only failing suite ispackages/migrations/src/__tests__/rls-all-tables.test.ts, which exits on missing Postgres env vars in this environment and is unrelated to the diffyarn migration:check-conflicts— passedGenerated by Claude Code