diff --git a/.changeset/6493-bind-field-visibility-evaluators.md b/.changeset/6493-bind-field-visibility-evaluators.md new file mode 100644 index 000000000..aa20d7da0 --- /dev/null +++ b/.changeset/6493-bind-field-visibility-evaluators.md @@ -0,0 +1,49 @@ +--- +'@object-ui/app-shell': minor +--- + +⚠️ **Behaviour change: `current_user` and `features` gates on an object field's +`visible` that have been doing nothing on the record form page and in the +console's record modal now TAKE EFFECT.** Read this before upgrading if any of +your object metadata gates a field on the session user or on a deployment flag. + +objectui#6010 and objectui#6110 bound the host predicate scope on the form +renderer and on the console form routes. `evaluateVisibility` was still being +reached with a THIRD and FOURTH evaluator that neither of those touched: +`RecordFormPage` and `AppContent` each built a private +`new ExpressionEvaluator({ user, app, data })` for the field-visibility filter, +beside — not from — the `ExpressionProvider` each of them mounts. Those bags +bound `user`, but not the canonical `current_user` nor the ADR-0068 `ctx.user` / +`os.user` spellings of that same object, and not `features` at all. So one +authored predicate meant two different things depending on which evaluator +reached it: `current_user` resolved on a nav item and was unbound on a field. +Both sites now build their scope with the same `buildExpressionScope` the +provider uses, which is the only declaration of what an app-shell predicate may +name. + +**Why nobody noticed, and why the fix is felt as a change.** A field `visible` +predicate fails OPEN: a field on screen is what you get when the predicate says +TRUE, when the root was never bound so the predicate faulted, *and* when the +predicate has a typo. Those worlds are indistinguishable, so an app that +authored a `current_user` gate saw the field render and had no way to tell the +rule was inert. After this change the predicate is evaluated for real, and +**fields that have always been visible will disappear for the users the rule +excludes** — and a `features` gate whose flag is off will hide its field once +`/api/v1/auth/config` resolves. + +`AppContent`'s bag also hand-rolled its user as `{ name, email, role }`, without +`positions`. It now uses the same `buildExpressionUser` normaliser every other +console surface publishes, so `'sales' in current_user.positions` — the gate the +server enforces on write — reaches the same verdict client-side instead of +faulting open. + +**Before upgrading**, audit any `visible` predicate in your object metadata that +names `current_user` (or `user` / `ctx.user` / `os.user`) or `features`, and +confirm each says what you actually want evaluated. Measured on the metadata +shipped in this repo and in the framework at the time of the change: **nothing +in it authors such a gate**, so no shipped surface changes behaviour today — +the audit is for your own object metadata, which this cannot see. + +**The error path is deliberately unchanged.** A predicate that throws still +fails open, exactly as objectui#6443 / objectui#6487 left it. This change is +about which roots are BOUND, not about what happens when evaluation fails. diff --git a/packages/app-shell/src/console/AppContent.tsx b/packages/app-shell/src/console/AppContent.tsx index 1188be960..b3f7e3660 100644 --- a/packages/app-shell/src/console/AppContent.tsx +++ b/packages/app-shell/src/console/AppContent.tsx @@ -22,14 +22,17 @@ import { useMetadata } from '../providers/MetadataProvider.js'; import { useAdapter } from '../providers/AdapterProvider.js'; import { usePreviewDrafts } from '../preview/PreviewModeContext.js'; import { PreviewDraftEmptyState } from '../preview/PreviewDraftEmptyState.js'; -import { ExpressionProvider, evaluateVisibility } from '../providers/ExpressionProvider.js'; +import { + ExpressionProvider, + createExpressionEvaluator, + evaluateVisibility, +} from '../providers/ExpressionProvider.js'; import { useTrackRouteAsRecent } from '../hooks/useTrackRouteAsRecent.js'; import { resolveRecordFormTarget, resolveFormViewLayout, resolveNavigateCreateUrl, resolveNavigateEditUrl, resolvePostCreateTarget } from '../utils/recordFormNavigation.js'; import { deriveRecordSurface, deriveRecordFlowSurface } from '@object-ui/plugin-view'; import { RECORD_FORM_PARAM, RECORD_FORM_OBJECT_PARAM, RECORD_FORM_LINK_PARAM } from '../urlParams.js'; import { matchAppBySegment } from '../utils/appRoute.js'; import { resolveHref, type NavTemplateContext } from '@object-ui/layout'; -import { ExpressionEvaluator } from '@object-ui/core'; // Components (eagerly loaded — always needed) import { ConsoleLayout } from '../layout/ConsoleLayout.js'; @@ -668,13 +671,25 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps = navigate(`/apps/${newAppName}`); }; + // Evaluator for the ModalForm's field-visibility gates below, over the SAME + // bag the `ExpressionProvider` this component mounts publishes — one builder, + // called with the same inputs (objectui#6493). This one sits ABOVE that + // provider in its own tree, so it cannot read it back through the hook; what + // it can do is stop hand-writing a second, narrower bag. + // + // Two roots the private bag dropped, both of which fail OPEN when named: + // `current_user` (and the `ctx.user` / `os.user` spellings of the same + // object) and `features`. Its `user` was hand-rolled too, without + // `positions` — so `'sales' in current_user.positions`, the gate the server + // enforces on write, faulted here rather than hiding the field. const expressionEvaluator = useMemo( - () => new ExpressionEvaluator({ - user: user ? { name: user.name, email: user.email, role: user.role ?? 'user' } : {}, + () => createExpressionEvaluator({ + user: buildExpressionUser(user), app: activeApp || {}, data: editingRecord || {}, + features, }), - [user, activeApp, editingRecord], + [user, activeApp, editingRecord, features], ); // objectui#5619 — `isWorkspaceAdminResolved` belongs in this readiness gate diff --git a/packages/app-shell/src/providers/ExpressionProvider.predicateScope.test.ts b/packages/app-shell/src/providers/ExpressionProvider.predicateScope.test.ts new file mode 100644 index 000000000..6e5faa5b6 --- /dev/null +++ b/packages/app-shell/src/providers/ExpressionProvider.predicateScope.test.ts @@ -0,0 +1,154 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#6493 — the app-shell predicate scope is built ONCE, and the two + * imperative evaluators bind it too. + * + * `evaluateVisibility` was reached with three different evaluators. Only + * `ExpressionProvider`'s carried the full bag; `RecordFormPage` and + * `AppContent` each built a private `new ExpressionEvaluator({ user, app, + * data })` for the SAME kind of gate — an object field's `visible` — and those + * bags named neither `current_user` (nor its `ctx.user` / `os.user` spellings) + * nor `features`. + * + * ## Why the old shape could not be caught by rendering alone + * + * A CEL predicate over an unbound root does not throw here: `evaluateCelCondition` + * fails SOFT to `true` when the caller has not asked for `throwOnError`, and + * `evaluateVisibility` is such a caller. So the field rendered — exactly as it + * would for a predicate that legitimately said yes, and exactly as it would for + * a predicate with a typo. The three worlds are indistinguishable on screen, + * which is why the fail-open direction is asserted below as its own case rather + * than assumed: `OLD_BAG` reproduces the pre-fix bag literally, and every + * assertion against it is the RED this change turns green. + * + * ADR-0068 D1 is the rule being conformed to — one user object under four + * spellings, so "a predicate `'org_admin' in current_user.roles` evaluates + * identically in a formula, an RLS policy, and a client `visible` gate". + */ + +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { ExpressionEvaluator } from '@object-ui/core'; +import { + buildExpressionScope, + createExpressionEvaluator, + evaluateVisibility, +} from './ExpressionProvider'; + +/** + * The served shape. `ExpressionInputSchema` normalises every authored `visible` + * string into a `{ dialect, source }` envelope, so this — not a bare string — + * is what actually reaches the evaluator once the server has served the schema. + */ +const POSITION_GATE = { dialect: 'cel', source: "'sales_manager' in current_user.positions" }; +const CTX_ALIAS_GATE = { dialect: 'cel', source: "'sales_manager' in ctx.user.positions" }; +const OS_ALIAS_GATE = { dialect: 'cel', source: "'sales_manager' in os.user.positions" }; +const FEATURE_GATE = { dialect: 'cel', source: 'features.multiOrgEnabled == true' }; + +const MANAGER = { name: 'Ada', email: 'ada@example.com', role: 'user', positions: ['sales_manager'] }; +const CLERK = { name: 'Bo', email: 'bo@example.com', role: 'user', positions: ['sales_clerk'] }; + +/** The bag both ad-hoc sites hand-wrote before this change, reproduced verbatim. */ +const oldBag = (user: Record) => + new ExpressionEvaluator({ user, app: { name: 'crm' }, data: {} }); + +describe('objectui#6493 — buildExpressionScope binds one user object under all four spellings', () => { + it('current_user / user / ctx.user / os.user are the SAME object, not four copies', () => { + const user = { name: 'Ada', positions: ['sales_manager'] }; + const scope = buildExpressionScope({ user }); + + expect(scope.current_user).toBe(user); + expect(scope.user).toBe(user); + expect(scope.ctx.user).toBe(user); + expect(scope.os.user).toBe(user); + }); + + it('binds app, data and features, and defaults every root to an empty object', () => { + const scope = buildExpressionScope(); + expect(scope).toStrictEqual({ + current_user: {}, user: {}, ctx: { user: {} }, os: { user: {} }, app: {}, data: {}, features: {}, + }); + // The identity above holds for the defaults too — the hand-written fallback + // in `useExpressionContext` used to mint three separate empty objects. + expect(scope.current_user).toBe(scope.user); + expect(scope.ctx.user).toBe(scope.user); + expect(scope.os.user).toBe(scope.user); + }); +}); + +describe('objectui#6493 — a current_user gate BITES through the shared scope', () => { + it('hides the field from a user the rule excludes', () => { + expect(evaluateVisibility(POSITION_GATE, createExpressionEvaluator({ user: CLERK }))).toBe(false); + }); + + it('shows the field to a user the rule admits', () => { + expect(evaluateVisibility(POSITION_GATE, createExpressionEvaluator({ user: MANAGER }))).toBe(true); + }); + + it('reaches the same verdict through the ctx.user and os.user spellings', () => { + const clerk = createExpressionEvaluator({ user: CLERK }); + const manager = createExpressionEvaluator({ user: MANAGER }); + + expect(evaluateVisibility(CTX_ALIAS_GATE, clerk)).toBe(false); + expect(evaluateVisibility(OS_ALIAS_GATE, clerk)).toBe(false); + expect(evaluateVisibility(CTX_ALIAS_GATE, manager)).toBe(true); + expect(evaluateVisibility(OS_ALIAS_GATE, manager)).toBe(true); + }); + + it('binds features, so a deployment flag can hide a field', () => { + expect(evaluateVisibility(FEATURE_GATE, createExpressionEvaluator({ features: { multiOrgEnabled: false } }))).toBe(false); + expect(evaluateVisibility(FEATURE_GATE, createExpressionEvaluator({ features: { multiOrgEnabled: true } }))).toBe(true); + }); +}); + +describe('objectui#6493 — the bag this change replaced failed OPEN on every one of those roots', () => { + it('showed the excluded user the field: an unbound current_user faults, and a fault reads as YES', () => { + // The whole defect in one line. Same predicate, same user, same + // `evaluateVisibility` — and the opposite answer from the one above. + expect(evaluateVisibility(POSITION_GATE, oldBag(CLERK))).toBe(true); + }); + + it('did the same for the ctx.user / os.user spellings', () => { + expect(evaluateVisibility(CTX_ALIAS_GATE, oldBag(CLERK))).toBe(true); + expect(evaluateVisibility(OS_ALIAS_GATE, oldBag(CLERK))).toBe(true); + }); + + it('did the same for a features flag that was off', () => { + expect(evaluateVisibility(FEATURE_GATE, oldBag(CLERK))).toBe(true); + }); + + it('bound `user` all along — which is what made the divergence invisible', () => { + // A gate authored against the back-compat spelling worked before AND after. + // An author who tested with `user.positions` had no way to discover that the + // canonical spelling was inert on this surface. + const USER_ALIAS_GATE = { dialect: 'cel', source: "'sales_manager' in user.positions" }; + expect(evaluateVisibility(USER_ALIAS_GATE, oldBag(CLERK))).toBe(false); + expect(evaluateVisibility(USER_ALIAS_GATE, createExpressionEvaluator({ user: CLERK }))).toBe(false); + }); +}); + +describe('objectui#6493 — neither call site hand-writes a predicate bag any more', () => { + // A source guard, because the defect was a COPY of the bag rather than a + // wrong value in it: nothing about a second `new ExpressionEvaluator({...})` + // is visible in a render, and the copy that drifted read as reasonable code + // for as long as it existed. The fence is the producer-side repair. + const sources = { + 'views/RecordFormPage.tsx': new URL('../views/RecordFormPage.tsx', import.meta.url), + 'console/AppContent.tsx': new URL('../console/AppContent.tsx', import.meta.url), + }; + + for (const [label, url] of Object.entries(sources)) { + it(`${label} builds its evaluator through createExpressionEvaluator`, () => { + const src = readFileSync(url, 'utf8'); + expect(src).not.toMatch(/new\s+ExpressionEvaluator\s*\(/); + expect(src).toMatch(/createExpressionEvaluator\s*\(/); + }); + } +}); diff --git a/packages/app-shell/src/providers/ExpressionProvider.tsx b/packages/app-shell/src/providers/ExpressionProvider.tsx index fa5e1578e..7320717f9 100644 --- a/packages/app-shell/src/providers/ExpressionProvider.tsx +++ b/packages/app-shell/src/providers/ExpressionProvider.tsx @@ -40,6 +40,66 @@ export interface ExpressionContextValue { const ExprCtx = createContext(null); +/** The inputs an app-shell surface has when it needs a predicate scope. */ +export interface ExpressionScopeInput { + user?: Record; + app?: Record; + data?: Record; + features?: Record; +} + +/** + * The ONE predicate scope this tier binds — the single declaration of what an + * app-shell expression can name. + * + * ADR-0068 D1: expose the SAME user object under the canonical `current_user` + * plus the back-compat `user` alias, the server-RLS-parity `ctx.user` alias, + * and the server-CEL-parity `os.user` alias (the spec's canonical identity + * scope — `{{os.user.id}}` per @objectstack/spec expression docs), so a + * predicate authored against any one form evaluates identically on client, + * server-formula, and server-RLS (#2358 trap 1). D1 names a client `visible` + * gate as one of the three surfaces that must agree. + * + * ## Why this is a function and not three literals + * + * It was three literals, and they drifted (objectui#6493). `ExpressionProvider` + * built the full bag, while `RecordFormPage` and `AppContent` each built a + * private `new ExpressionEvaluator({ user, app, data })` for the SAME kind of + * gate — an object field's `visible` — beside the provider they never read. + * Those bags bound `user` but not the other three spellings of the same object, + * and not `features` at all, so ONE authored predicate meant two things + * depending on which evaluator reached it: `current_user` resolved on a nav + * item and FAULTED on a field, and a fault fails OPEN (`evaluateVisibility` + * below), which is indistinguishable on screen from a gate that said yes. + * A copy of the bag is how that recurs; a call is not. + * + * `features` is renderer-tier, not contract — the same posture `@objectstack/ + * spec`'s `page.zod.ts` documents for component `visibleWhen` ("the shipping + * renderer additionally mounts `app`, `features`, `os.user` … renderer + * behaviour, NOT contract-guaranteed"). It is bound here because it is what + * THIS tier's own diagnostic advice tells an author they may name. + */ +export function buildExpressionScope({ + user = {}, + app = {}, + data = {}, + features = {}, +}: ExpressionScopeInput = {}): Record { + return { current_user: user, user, ctx: { user }, os: { user }, app, data, features }; +} + +/** + * An `ExpressionEvaluator` over {@link buildExpressionScope}. + * + * Every app-shell site that needs an evaluator imperatively (i.e. one it cannot + * take from `useExpressionContext()`, because it builds the field list ABOVE + * the provider it mounts) calls this instead of `new ExpressionEvaluator(...)` + * with a hand-written bag. + */ +export function createExpressionEvaluator(input: ExpressionScopeInput = {}): ExpressionEvaluator { + return new ExpressionEvaluator(buildExpressionScope(input)); +} + interface ExpressionProviderProps { children: React.ReactNode; user?: Record; @@ -50,24 +110,17 @@ interface ExpressionProviderProps { export function ExpressionProvider({ children, user = {}, app = {}, data = {}, features = {} }: ExpressionProviderProps) { const value = useMemo(() => { - // ADR-0068: expose the SAME user object under the canonical `current_user` - // plus the back-compat `user` alias, the server-RLS-parity `ctx.user` - // alias, and the server-CEL-parity `os.user` alias (the spec's canonical - // identity scope — `{{os.user.id}}` per @objectstack/spec expression docs), - // so a predicate authored against any one form evaluates identically on - // client, server-formula, and server-RLS (#2358 trap 1). - const context = { current_user: user, user, ctx: { user }, os: { user }, app, data, features }; - const evaluator = new ExpressionEvaluator(context); + const evaluator = createExpressionEvaluator({ user, app, data, features }); return { user, app, data, features, evaluator }; }, [user, app, data, features]); // Also feed the predicate scope used by useCondition/useExpression in // @object-ui/react so action visibility predicates (e.g. on toolbar // buttons) can see deployment-level flags like features.multiOrgEnabled. - // Mirror the canonical `current_user`/`user`/`ctx.user`/`os.user` aliases - // here too. + // The SAME bag the evaluator above got — one builder, so the imperative and + // the hook-driven halves of this provider cannot drift apart either. const scope = useMemo( - () => ({ current_user: user, user, ctx: { user }, os: { user }, app, data, features }), + () => buildExpressionScope({ user, app, data, features }), [user, app, data, features], ); @@ -85,10 +138,12 @@ export function ExpressionProvider({ children, user = {}, app = {}, data = {}, f export function useExpressionContext(): ExpressionContextValue { const ctx = useContext(ExprCtx); if (!ctx) { - // Return a safe default so components can be used outside the provider + // Return a safe default so components can be used outside the provider. + // Through the same builder: the hand-written version gave `current_user`, + // `ctx.user` and `os.user` three DIFFERENT empty objects, which ADR-0068 D1 + // spells as aliases "pointing at the same object". const fallback = { user: {}, app: {}, data: {}, features: {} }; - const evalContext = { current_user: {}, ctx: { user: {} }, os: { user: {} }, ...fallback }; - return { ...fallback, evaluator: new ExpressionEvaluator(evalContext) }; + return { ...fallback, evaluator: createExpressionEvaluator(fallback) }; } return ctx; } diff --git a/packages/app-shell/src/views/RecordFormPage.predicateScope.test.tsx b/packages/app-shell/src/views/RecordFormPage.predicateScope.test.tsx new file mode 100644 index 000000000..88e95f214 --- /dev/null +++ b/packages/app-shell/src/views/RecordFormPage.predicateScope.test.tsx @@ -0,0 +1,186 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#6493 — the field-visibility gate on the record form page binds the + * roots this tier declares. + * + * This is the call site, not the seam: the page filters `objectDef.fields` by + * `evaluateVisibility(f.visible, expressionEvaluator)` and hands the surviving + * names to `ObjectForm` as `schema.fields`, which is what these tests read. + * + * The evaluator used to be a private `new ExpressionEvaluator({ user, app, + * data })` built beside — not from — the `ExpressionProvider` this same page + * mounts around the form. `current_user`, `ctx.user`, `os.user` and `features` + * were unbound in it, and an unbound root fails OPEN, so every gate below + * rendered its field for everyone. Each `not.toContain` here was `toContain` + * before the fix. + * + * Note what is NOT being tested: the error path. A CEL predicate over an + * unbound root never reaches `evaluateVisibility`'s `catch` — `evalFieldPredicate` + * fails soft to `true` on its own — so binding the roots and fail-open-on-throw + * are independent, and the latter is deliberately unchanged (#6443 / #6487). + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, cleanup, waitFor } from '@testing-library/react'; +import { MemoryRouter, Routes, Route } from 'react-router-dom'; +import { I18nProvider } from '@object-ui/i18n'; +import { RecordFormPage } from './RecordFormPage'; + +const h = React.createElement; + +const { formSchemas, getAuthConfig, authState } = vi.hoisted(() => ({ + formSchemas: [] as any[], + getAuthConfig: vi.fn(async () => ({ features: {} as Record })), + authState: { + user: null as Record | null, + activeOrganization: null as { name: string } | null, + }, +})); + +vi.mock('sonner', () => ({ + toast: Object.assign(vi.fn(), { + success: vi.fn(), error: vi.fn(), info: vi.fn(), + warning: vi.fn(), loading: vi.fn(), dismiss: vi.fn(), + }), +})); + +vi.mock('@object-ui/auth', () => ({ + useAuth: () => ({ + get user() { return authState.user; }, + getAuthConfig, + get activeOrganization() { return authState.activeOrganization; }, + }), +})); + +vi.mock('@object-ui/plugin-form', () => ({ + ObjectForm: ({ schema }: any) => { + formSchemas.push(schema); + return h('div', { 'data-testid': 'object-form' }); + }, +})); + +const metadataState = { objects: [] as any[], loading: false }; +vi.mock('../providers/MetadataProvider', () => ({ useMetadata: () => metadataState })); +vi.mock('../providers/AdapterProvider', () => ({ useAdapter: () => null })); + +/** The served shape: `ExpressionInputSchema` normalises authored strings into this. */ +const cel = (source: string) => ({ dialect: 'cel', source }); + +const CONTACTS = { + name: 'contacts', + label: 'Contacts', + fields: { + name: { type: 'text' }, + commission: { type: 'currency', visible: cel("'sales_manager' in current_user.positions") }, + escalate_to: { type: 'text', visible: cel("'sales_manager' in ctx.user.positions") }, + owner_note: { type: 'text', visible: cel("'sales_manager' in os.user.positions") }, + org_switcher: { type: 'text', visible: cel('features.multiOrgEnabled == true') }, + }, +}; + +const MANAGER = { id: 'u1', name: 'Ada', email: 'ada@example.com', role: 'user', positions: ['sales_manager'] }; +const CLERK = { id: 'u2', name: 'Bo', email: 'bo@example.com', role: 'user', positions: ['sales_clerk'] }; + +function renderPage() { + return render( + h(I18nProvider, { + config: { defaultLanguage: 'en', detectBrowserLanguage: false }, + children: h( + MemoryRouter, + { initialEntries: ['/apps/crm/contacts/new'] }, + h(Routes, null, h(Route, { + path: '/apps/:appName/:objectName/new', + element: h(RecordFormPage, { mode: 'create' }), + })), + ), + }), + ); +} + +/** + * The field list from the LAST schema handed to `ObjectForm`. `features` is + * fetched, so the first render is always the pre-fetch one — reading anything + * but the last schema would assert against an empty `features` bag and pass for + * the wrong reason. + */ +const lastFields = (): string[] => formSchemas[formSchemas.length - 1].fields; + +beforeEach(() => { + metadataState.objects = [CONTACTS]; + metadataState.loading = false; + authState.user = CLERK; + authState.activeOrganization = null; + getAuthConfig.mockResolvedValue({ features: {} }); + formSchemas.length = 0; +}); +afterEach(cleanup); + +describe('objectui#6493 — a current_user field gate hides the field on the record form page', () => { + it('withholds the gated field from a user the rule excludes', async () => { + authState.user = CLERK; + renderPage(); + + await waitFor(() => expect(formSchemas.length).toBeGreaterThan(0)); + // Ungated fields are unaffected — the filter still lets everything else by. + expect(lastFields()).toContain('name'); + // Was `toContain` before the fix: `current_user` was not in the bag, the + // predicate faulted, and the fault read as "visible". + expect(lastFields()).not.toContain('commission'); + }); + + it('grants it to a user the rule admits', async () => { + authState.user = MANAGER; + renderPage(); + + await waitFor(() => expect(formSchemas.length).toBeGreaterThan(0)); + expect(lastFields()).toContain('commission'); + }); + + it('honours the ctx.user and os.user spellings of the same object (ADR-0068 D1)', async () => { + authState.user = CLERK; + renderPage(); + + await waitFor(() => expect(formSchemas.length).toBeGreaterThan(0)); + expect(lastFields()).not.toContain('escalate_to'); + expect(lastFields()).not.toContain('owner_note'); + + cleanup(); + formSchemas.length = 0; + authState.user = MANAGER; + renderPage(); + + await waitFor(() => expect(formSchemas.length).toBeGreaterThan(0)); + expect(lastFields()).toContain('escalate_to'); + expect(lastFields()).toContain('owner_note'); + }); +}); + +describe('objectui#6493 — a features field gate is live on the record form page', () => { + it('hides the field when the deployment flag is off', async () => { + getAuthConfig.mockResolvedValue({ features: { multiOrgEnabled: false } }); + renderPage(); + + // Wait for the flag to land, not merely for a render: before it resolves + // `features` is `{}` and the predicate faults open, which is the same + // answer the defect gave. + await waitFor(() => expect(getAuthConfig).toHaveBeenCalled()); + await waitFor(() => expect(lastFields()).not.toContain('org_switcher')); + }); + + it('shows the field when the deployment flag is on', async () => { + getAuthConfig.mockResolvedValue({ features: { multiOrgEnabled: true } }); + renderPage(); + + await waitFor(() => expect(getAuthConfig).toHaveBeenCalled()); + await waitFor(() => expect(lastFields()).toContain('org_switcher')); + }); +}); diff --git a/packages/app-shell/src/views/RecordFormPage.tsx b/packages/app-shell/src/views/RecordFormPage.tsx index c1810046a..144412ad8 100644 --- a/packages/app-shell/src/views/RecordFormPage.tsx +++ b/packages/app-shell/src/views/RecordFormPage.tsx @@ -37,11 +37,14 @@ import { toast } from 'sonner'; import { useObjectTranslation, useObjectLabel } from '@object-ui/i18n'; import { useMetadata } from '../providers/MetadataProvider.js'; import { useAdapter } from '../providers/AdapterProvider.js'; -import { ExpressionProvider, evaluateVisibility } from '../providers/ExpressionProvider.js'; +import { + ExpressionProvider, + createExpressionEvaluator, + evaluateVisibility, +} from '../providers/ExpressionProvider.js'; import { SkeletonDetail } from '../skeletons/index.js'; import { ManagedByBadge } from '../components/ManagedByBadge.js'; import { useAuth } from '@object-ui/auth'; -import { ExpressionEvaluator } from '@object-ui/core'; export interface RecordFormPageProps { /** Form mode — `'create'` for the `/new` route, `'edit'` for the `/edit` route. */ @@ -178,18 +181,31 @@ export function RecordFormPage({ mode }: RecordFormPageProps) { [user], ); - // Build expression evaluator for field-visibility expressions, mirroring - // the global ModalForm setup in AppContent. + // Evaluator for the field-visibility expressions below, over the SAME bag + // the `ExpressionProvider` at the bottom of this file publishes to the form's + // descendants — one builder, called with the same four inputs (objectui#6493). + // + // It cannot simply READ that provider: the provider is mounted in this + // component's own returned tree, so `useExpressionContext()` here would + // resolve to whatever provider sits ABOVE this page (today `AppContent`'s, + // with a different `app`; nothing at all if the page is ever mounted + // elsewhere, which would silently unbind `user` too). Building the same scope + // from the same inputs is the fix that needs no new wiring. + // + // The private bag this replaced bound `user` alone, so an authored + // `current_user` / `ctx.user` / `os.user` / `features` gate on a field + // faulted here and failed OPEN while resolving normally on a nav item. const expressionEvaluator = useMemo( () => - new ExpressionEvaluator({ + createExpressionEvaluator({ // expressionUser already handles the anonymous fallback, so we can // pass it through unconditionally. user: expressionUser, app: { name: appName }, data: {}, + features, }), - [expressionUser, appName], + [expressionUser, appName, features], ); // Resolve the field list using the same visibility-aware logic as the