Skip to content

Commit d6ba70b

Browse files
authored
Merge branch 'main' into fix/import-match-only-key
2 parents 4cb26df + c41174a commit d6ba70b

48 files changed

Lines changed: 1493 additions & 313 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
"@object-ui/plugin-grid": patch
3+
"@object-ui/types": patch
4+
"@object-ui/app-shell": patch
5+
---
6+
7+
fix(grid): drop the `bulkEnabled` derivation — the spec key is a tombstone
8+
9+
Follow-up to objectui#3002 / #3031. That change folded two sources into the
10+
selection bar: a view's `bulkActions` names resolved against
11+
`objectDef.actions`, and object actions declaring `ActionSchema.bulkEnabled`.
12+
The second source is dead.
13+
14+
`@objectstack/spec` 17.0.0 retired `action.bulkEnabled` in the #3896 audit
15+
close-out (framework#4054, landed while #3031 was in flight — the spec source
16+
still carried the key when its design was settled). It is now a `retiredKey()`
17+
tombstone, so it is not merely ignored: `defineStack` **hard-rejects** a config
18+
that sets it, and the backend refuses to boot. Browser verification against a
19+
real showcase backend is what surfaced this — the derivation branch could never
20+
run, and #3031's changeset pointed authors at a key that breaks their app.
21+
22+
The tombstone's own prescription is the path that survives:
23+
24+
> the multi-select toolbar is driven by the LIST VIEW's `bulkActions` /
25+
> `bulkActionDefs`, never by this flag … declare the action in the view's
26+
> `bulkActions` instead.
27+
28+
So `resolveBulkActions` now folds exactly two vocabularies — inline-authored
29+
`bulkActionDefs`, and `bulkActions` names promoted to their declared object
30+
action — which is what #3031's other half already did and what the end-to-end
31+
run exercised: naming `showcase_mark_done` in the view's `bulkActions` issued
32+
one `POST /api/v1/actions/showcase_task/showcase_mark_done` per selected
33+
record (10/10 → `done: true, progress: 100` server-side). Everything downstream
34+
of the fold is unchanged: promoted defs still carry the action's label, icon,
35+
`visible`, confirm text and params; still run through `BulkActionDialog`
36+
(params → confirm → progress → result); still dispatch per record with
37+
`_rowRecord` attached; still attribute failures per record.
38+
39+
A stale `bulkEnabled: true` on an object action is now inert rather than a
40+
second path into the bar. Note tsc cannot catch this class of drift here — the
41+
fold reads a loosely-typed `NamedActionDef` with an index signature, so the
42+
retired key never surfaces as `never`.
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
"@object-ui/plugin-grid": patch
3+
---
4+
5+
fix(grid): a bulk delete / by-name action clears the row checkboxes, not just the toolbar — objectui#3056
6+
7+
After a successful bulk delete (or a bulk action dispatched to a
8+
consumer-registered runner handler), the selection toolbar vanished but every
9+
row stayed visibly ticked. The user was left on a page of selected rows with no
10+
toolbar to act on them, and no way back except unticking each row or reloading.
11+
12+
`ObjectGrid` carries two selection sources that must move together:
13+
`selectedRows` (ours — drives the toolbar) and the data-table's internal
14+
`selectedRowIds` (drives the checkboxes, cleared only when the host bumps
15+
`selectionResetKey`). `handleBulkDialogClose` reset both; `dispatchBulkAction`
16+
reset only the first, on both of its branches.
17+
18+
Both now go through one `resetSelection()` helper — including the dialog path,
19+
so the invariant is structural rather than three call sites remembering to
20+
agree. Failure semantics are untouched: a by-name action that reports
21+
`success: false` still keeps the toolbar AND the checkboxes so the user can fix
22+
the cause and retry the same rows.
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
"@object-ui/app-shell": patch
3+
"@object-ui/plugin-list": patch
4+
"@object-ui/react": patch
5+
"@object-ui/i18n": patch
6+
---
7+
8+
fix(console): three real-user console failures — 403 blamed on the network, ⌘K search capped at 8 objects, nav gating fields inert
9+
10+
1. **List error panel classifies the failure** (`plugin-list`, `i18n`): a 403/401 from the data source used to render the same "check your connection" copy as a genuine outage, sending users to debug their network while the server was correctly denying access. The panel now classifies by `httpStatus`/`status`/`statusCode`, the `PERMISSION_DENIED`/`UNAUTHORIZED` error codes, or an `HTTP <status>` message prefix, and renders dedicated permission-denied / sign-in-required copy (all nine locales).
11+
12+
2. **⌘K / full-page search scope is no longer truncated** (`react`): `maxObjectsQueried` caps the per-object fanout fallback, not the search scope — it used to slice the candidate pool itself, so the `objects` whitelist sent to the platform's `/api/v1/search` only ever named the first 8 nav objects. Which sidebar group came first decided which records were findable; everything later in the nav was unsearchable no matter what the user typed.
13+
14+
3. **Nav gating fields finally gate** (`app-shell`): `evaluateVisibility` only evaluated `${…}` template strings, so the `{ dialect: 'cel', source }` envelopes the spec normalizes every authored `visible` predicate into fell through to a blanket "visible" — a constant-false predicate still rendered for everyone. It now delegates to `ExpressionEvaluator.evaluateCondition`, which routes CEL envelopes to the canonical `@objectstack/formula` engine. And the sidebars' `requiredPermissions` check treats a bare name as an ADR-0066 system capability (union of the user's permission-set `systemPermissions` from `/me/permissions`) — the same subset rule the server applies to `AppSchema.requiredPermissions` — instead of misreading it as `can(<name>, 'read')`, which had degraded `requiredPermissions` into a hide-from-everyone switch (admins included). The `object:action` form and the legacy object-read fallback keep working.

packages/app-shell/src/layout/AppSidebar.tsx

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -237,17 +237,20 @@ export function AppSidebar({ activeAppName, onAppChange }: { activeAppName: stri
237237
[evaluator],
238238
);
239239

240-
// Permission check from Console permissions context
241-
const { can } = usePermissions();
240+
// Permission check from Console permissions context. Mirrors
241+
// UnifiedSidebar: `object:action` → object CRUD gate; a bare name is an
242+
// ADR-0066 system capability (union of permission-set `systemPermissions`),
243+
// with the legacy "can read <object>" reading kept as fallback.
244+
const { can, hasCapabilities } = usePermissions();
242245
const checkPerm = React.useCallback(
243246
(permissions: string[]) => permissions.every((perm: string) => {
244247
const parts = perm.split(':');
245-
const [object, action] = parts.length >= 2
246-
? [parts[0], parts[1]]
247-
: [perm, 'read'];
248-
return can(object, action as any);
248+
if (parts.length >= 2) {
249+
return can(parts[0], parts[1] as any);
250+
}
251+
return hasCapabilities([perm]) || can(perm, 'read');
249252
}),
250-
[can],
253+
[can, hasCapabilities],
251254
);
252255

253256
// Runtime capability checker — gates nav entries with `requiresObject` /

packages/app-shell/src/layout/UnifiedSidebar.tsx

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -311,17 +311,28 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) {
311311
[evaluator],
312312
);
313313

314-
// Permission check
315-
const { can } = usePermissions();
314+
// Permission check for nav `requiredPermissions` entries.
315+
//
316+
// Two authored forms:
317+
// - `object:action` → object CRUD gate.
318+
// - bare name → an ADR-0066 system capability, checked against the union of
319+
// the user's permission-set `systemPermissions` (from /me/permissions) —
320+
// the SAME subset rule the server applies to `AppSchema.requiredPermissions`.
321+
// This used to be misread as `can(<name>, 'read')` only, so a nav item
322+
// requiring a capability was hidden even from users whose permission set
323+
// granted it (admins included) — `requiredPermissions` degenerated into a
324+
// "hide from everyone" switch. The object-read fallback stays for nav
325+
// items that gate on a plain object name.
326+
const { can, hasCapabilities } = usePermissions();
316327
const checkPerm = React.useCallback(
317328
(permissions: string[]) => permissions.every((perm: string) => {
318329
const parts = perm.split(':');
319-
const [object, action] = parts.length >= 2
320-
? [parts[0], parts[1]]
321-
: [perm, 'read'];
322-
return can(object, action as any);
330+
if (parts.length >= 2) {
331+
return can(parts[0], parts[1] as any);
332+
}
333+
return hasCapabilities([perm]) || can(perm, 'read');
323334
}),
324-
[can],
335+
[can, hasCapabilities],
325336
);
326337

327338
// Runtime capability gate: hide nav items targeting objects/services
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
import { describe, it, expect } from 'vitest';
10+
import { ExpressionEvaluator } from '@object-ui/core';
11+
import { evaluateVisibility } from './ExpressionProvider';
12+
13+
/**
14+
* Regression: nav/area `visible` predicates arrive from the server as
15+
* `{ dialect: 'cel', source }` envelopes (the spec's ExpressionInputSchema
16+
* normalizes every authored string into that shape). evaluateVisibility used
17+
* to fall through to a blanket `return true` for anything that wasn't a
18+
* `${…}` template string — so a constant-false CEL predicate still rendered
19+
* the menu item for everyone, and "hide this nav item by role" was
20+
* unimplementable from app metadata.
21+
*/
22+
23+
function makeEvaluator(user: Record<string, unknown>) {
24+
const context = { current_user: user, user, ctx: { user }, os: { user }, app: {}, data: {}, features: {} };
25+
return new ExpressionEvaluator(context as any);
26+
}
27+
28+
describe('evaluateVisibility', () => {
29+
const worker = makeEvaluator({ id: 'u1', positions: ['worker'] });
30+
const orgAdmin = makeEvaluator({ id: 'u2', positions: ['org_admin'] });
31+
32+
it('keeps literal handling: booleans and "true"/"false" strings', () => {
33+
expect(evaluateVisibility(undefined, worker)).toBe(true);
34+
expect(evaluateVisibility(true, worker)).toBe(true);
35+
expect(evaluateVisibility('true', worker)).toBe(true);
36+
expect(evaluateVisibility(false, worker)).toBe(false);
37+
expect(evaluateVisibility('false', worker)).toBe(false);
38+
});
39+
40+
it('evaluates a CEL envelope against current_user (spec P`…` form)', () => {
41+
const visible = { dialect: 'cel', source: "'org_admin' in current_user.positions" };
42+
expect(evaluateVisibility(visible, orgAdmin)).toBe(true);
43+
expect(evaluateVisibility(visible, worker)).toBe(false);
44+
});
45+
46+
it('still evaluates ${…} template expressions', () => {
47+
const evaluator = makeEvaluator({ role: 'admin' });
48+
expect(evaluateVisibility("${user.role === 'admin'}", evaluator)).toBe(true);
49+
expect(evaluateVisibility("${user.role === 'guest'}", evaluator)).toBe(false);
50+
});
51+
52+
it('fails open (visible) on an unevaluable predicate', () => {
53+
expect(evaluateVisibility({ dialect: 'cel', source: 'not ] valid (' }, worker)).toBe(true);
54+
});
55+
});

packages/app-shell/src/providers/ExpressionProvider.tsx

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -99,25 +99,32 @@ export function useExpressionContext(): ExpressionContextValue {
9999
* - boolean: true/false
100100
* - string "true"/"false"
101101
* - template expression: "${user.role === 'admin'}"
102+
* - `{ dialect: 'cel', source }` envelopes — the shape the spec's
103+
* `ExpressionInputSchema` normalizes every authored `visible` string into,
104+
* which is what nav/area items carry after the server serves the app schema
105+
* - bare expression strings (evaluated as one boolean expression)
102106
*
103-
* Returns true if the item should be visible.
107+
* Everything non-literal is delegated to `evaluator.evaluateCondition`, which
108+
* routes CEL envelopes to the canonical `@objectstack/formula` engine. The
109+
* envelope and bare-string shapes used to fall through to a blanket
110+
* `return true`, so a constant-false nav `visible` predicate (e.g.
111+
* ``P`'org_admin' in current_user.positions` ``) still rendered for everyone —
112+
* the app author had no working way to hide a menu item by role.
113+
*
114+
* Returns true if the item should be visible (fail-open on evaluation errors,
115+
* matching the evaluator's own default).
104116
*/
105117
export function evaluateVisibility(
106-
expression: string | boolean | undefined,
118+
expression: string | boolean | { dialect?: string; source?: string } | undefined,
107119
evaluator: ExpressionEvaluator,
108120
): boolean {
109121
if (expression === undefined || expression === null) return true;
110122
if (expression === true || expression === 'true') return true;
111123
if (expression === false || expression === 'false') return false;
112124

113-
if (typeof expression === 'string' && expression.includes('${')) {
114-
try {
115-
const result = evaluator.evaluateCondition(expression);
116-
return result;
117-
} catch {
118-
return true; // Default to visible on error
119-
}
125+
try {
126+
return evaluator.evaluateCondition(expression);
127+
} catch {
128+
return true; // Default to visible on error
120129
}
121-
122-
return true;
123130
}

packages/app-shell/src/views/ObjectView.tsx

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,13 +1422,12 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an
14221422
: []),
14231423
/**
14241424
* Selection-bar actions. Unlike `rowActionDefs` above, these are
1425-
* NOT derived here: `ObjectGrid` is the single convergence point of
1426-
* all three list callers (this view, plugin-view's ObjectView and
1427-
* plugin-list's ListView), so it folds `objectDef.actions` — the
1428-
* spec's `bulkEnabled: true` declaration, plus any legacy
1429-
* `bulkActions` name that resolves to a declared action — into the
1430-
* def list itself (`resolveBulkActions`, objectui#3002). What we
1431-
* pass through is the view author's own inline declaration.
1425+
* NOT resolved here: `ObjectGrid` is the single convergence point
1426+
* of all three list callers (this view, plugin-view's ObjectView
1427+
* and plugin-list's ListView), so it resolves each `bulkActions`
1428+
* name against `objectDef.actions` and promotes it into the def
1429+
* list itself (`resolveBulkActions`, objectui#3002). What we pass
1430+
* through is the view author's own declaration.
14321431
*/
14331432
bulkActions: viewDef.bulkActions ?? listSchema.bulkActions,
14341433
bulkActionDefs: (viewDef as any).bulkActionDefs ?? (listSchema as any).bulkActionDefs,

0 commit comments

Comments
 (0)