Skip to content

Commit c8097a9

Browse files
fix(app-shell): list a report in the index and ⌘K only when the navigation shows the member its entry (#27060)
<!-- claude-projects-attribution: {"github_login":"os-warren"} --> _Requested by **Warren** · [project thread](https://claude.ai/code/project/chan_01U5RdnAAmkSQmZbqh6FtqZ8?thread=cmsg_01U5RdnAAmkSQmZbqh6FtqZ8SfRz7pGa2P99NAztqiusfy)_ Fixes #26892 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:应用的「全部报表」页和 ⌘K 面板,现在只列出侧边栏会给当前成员看的导航项所指向的报表;导航没有指到的报表照旧列出。⌘K 面板的导航项(对象、仪表盘、页面、报表)也改用侧边栏同一套成员门控。 - **为什么改**:HotCRM 里销售和客服在「全部报表」里看到导航对他们隐藏的报表(如商机报表),点开只有「没有数据」,误以为没数据。 - **风险与回滚**:风险低到中:只在 `app-shell` 前端过滤,复用侧边栏现成的门控,不新增请求;⌘K 面板原本只看叶子项的 `visible`,现在也看 `requiredPermissions`、能力门控和分组的 `visible`,所以有些成员在面板里看到的项会变少(与侧边栏一致)。回滚即还原本 PR。 - **先看**:`views/appReportIndex.ts` 的 `navigationShowsReport`;`chrome/CommandPalette.tsx` 的 `navItems` 与新的 `AppCommandPaletteBody`;新测试 `AppReportIndex.navGates-26892.test.tsx`。 - **两分钟验证**:`pnpm --dir ui exec vitest run packages/app-shell/src/views/__tests__/AppReportIndex.navGates-26892.test.tsx`,预期 16 个用例全绿。 - **需要你做的**:无,席位落地。 ## What changes **The rule kept.** A report that one or more entries of the app's navigation name is listed by the report index (`/apps/APP/reports`) and by the palette's "reports beyond the navigation" only when the member is shown **at least one** of those entries, under the sidebar's own gates: `navEntriesShownToMember` over `useNavMemberGates()`. An entry hidden by its `visible`, its `requiredPermissions`, a runtime-capability gate (`requiresObject` / `requiresService`), the object-read prune, a group those gates hide, or an API `dataSource` read that has not answered (pending counts as hidden, as in the sidebar) does not count. A report **no** entry names keeps today's read rule alone, so the index stays its only door (the objectui#13020 reason for the index). "At least one, not every": a member shown any entry naming the report can already open it from the sidebar, so hiding it from the index would only remove a second door to something they can reach. Against the card's three personas: the service agent's four opportunity reports are named by entries the navigation hides from them (`visible: OPPORTUNITY_AUDIENCE`, per the card), so they leave the index and the palette; the sales rep's three case reports leave too when HotCRM's navigation names them behind an audience gate the rep fails, which the card's "sidebar shows 7" reading implies; the marketing user's index already dropped the case reports through the dataset read rule, which is unchanged. HotCRM's source is not in this session's reach: these are readings of the card against the rule, pinned here on fixtures of the same shapes, not a run against the HotCRM seed. A HotCRM report named by **no** entry keeps listing, by design. Commits, in order: 1. `views/appReportIndex.ts`: new exported `navigationShowsReport(app, gates)`; `useAppReportEntries(app, enabled, gates)` takes the member's gates and filters by it before `mayOpenReport`. `AppReportIndexView` passes `useNavMemberGates()`; the palette passes its gates. 2. `chrome/CommandPalette.tsx` (surface widened by the seat's order amendment): the palette's `navItems` were `flattenNavigation(nav)` filtered by each leaf's own `visible` only, so an entry hidden by `requiredPermissions`, a capability gate or its group's `visible` was still offered through the palette's navigation door, a nav-named report included. They are now `flattenNavigation(navEntriesShownToMember(nav, gates))`. **This also gates the palette's object, dashboard and page entries, and the record-search scope (`searchableObjectNames`, read from `navItems`), the way the sidebar gates them.** The palette's own `useNavVisibility(activeApp)` reader is gone: it now has exactly the one binding reader `useNavMemberGates()` builds. Two doc sentences in `hooks/useNavMemberGates.ts` that named the palette as calling `useNavVisibility` directly are corrected; commit 5 adds an optional parameter to `useNavMemberGates`. 3. The gates are built in a new `AppCommandPaletteBody` wrapper and handed down, `null` in the `studio` scope: that scope has no app and reads nothing until it opens (pinned by `CommandPalette.studioScope-11863`), and building the gates in the shared body read the object list while closed. `useAppReportEntries` answers no entry without gates. 4. Test-only: the new suite's adapter double is typed instead of `any`. 5. `useNavMemberGates` takes an optional app, the frame's by default; the palette passes the app it lists, so Home (expression scope binds no app, palette lists the host app) gates the host app's bindings and package scope. The one existing-export change: an added optional parameter, no behaviour change for any existing caller. No server change, no new request, no spec change. ## Tests New `views/__tests__/AppReportIndex.navGates-26892.test.tsx` (18 cases), through the real `MetadataProvider`, `MePermissionsProvider`, `ExpressionProvider` and `I18nProvider`. The package owns Alpha, Beta and Gamma (named by entries) and Delta (named by none), all on a readable dataset, so the navigation is the only gate in play: - index: Gamma's entry `visible: false`, or `requiredPermissions` the member lacks, or inside a `visible: false` group, or bound to an API read still pending ⇒ Alpha, Beta, Delta; controls: the permission granted, `visible: true`, a second shown entry beside a hidden one, and a navigation naming no report ⇒ all four; - palette, both doors: the same `visible: false`, pending, `requiredPermissions` and hidden-group cases ⇒ no Gamma; the granted control offers it once; an object entry with `requiredPermissions` the member lacks is not offered; - one read per binding: with a sidebar stand-in (`useNavMemberGates` + `navEntriesShownToMember`, the sidebar's walk) mounted beside the open palette, an entry's API `dataSource` URL is requested exactly once and both show the entry; - Home shape (h): the frame's expression scope binds no app (`{}`) while the palette lists the host app with a report entry bound to an API read: the entry is shown once the read answers (`total` 1) and requested exactly once; control: `total` 2 stays hidden. Changed: `AppReportIndex-13020.test.tsx`'s control "reads the report and dataset lists, never the object registry". Its adapter double now records a package-scoped `getItems` with its scope, because the index's gates read the app's object scope (`object?packageId=…`), the sidebar's own read in the console; the unscoped `object` registry read is still pinned absent, and the scoped read is now pinned present. ## Acceptance notes - The palette's wider gating (commit 2) is a behaviour change for every palette entry type, not only reports: a member now sees in ⌘K exactly the navigation entries the sidebar shows them. Approved as a bounded in-place fix by the seat (order amendment on the card), in its own commit. - The index and the palette read the gates' reads (the app's object scope, each entry's API binding) through the same readers as the sidebar; in the console the sidebar is mounted on every app page, so these are not new requests. - Like the rest of this module, the navigation read is the app's `navigation` (with module contributions merged in); `areas[].navigation` is not walked, so a report named only inside an area is treated as named by no entry, as `appShippedReports` already treats it. Observation only, not measured against a producer. ## Verification All runs after merging `origin/main` (`cb8988ee`) into the branch; head `ad2bc8e9` (commit 5 added on `e8cfd011`). Dependency closure `pnpm --workspace-concurrency=2 --filter '@object-ui/app-shell^...' build` under the verify lock. <details><summary>Tests, type-check, lint</summary> - `pnpm --dir ui exec vitest run --maxWorkers=2` over the 140 app-shell test files that import the palette, the report index, `ConsoleLayout`, `AppContent`, `HomeLayout`, `sharedUserFeeds` or `useNavMemberGates` (the new suite included), on `ad2bc8e9`: Test Files 140 passed (140), Tests 1050 passed (1050), lock VERDICT command-exit 0. Narrowed from the package's 1804 files to those importers; the full `Test UI` run is CI's. - `pnpm --filter @object-ui/app-shell type-check` (both `tsc --noEmit` and `tsconfig.test.json`): exit 0. - `pnpm --dir ui exec eslint` on the six changed files: exit 0, 0 errors; warning counts per modified file equal their `origin/main` counts, the new suite has none. - `pnpm --dir ui run check:new-line-citations`: 0 new citations, exit 0. </details> <details><summary>Ablations (scripts/ablation-replace.mjs, restored to the HEAD blob each time)</summary> 1. `appReportIndex.ts`: `shownByNavigation(report.name as string) && mayOpenReport(` replaced by `mayOpenReport(` plus a marker (the index gate dropped): 8 cases red (index `visible: false`, `requiredPermissions`, hidden group, pending; palette `visible: false`, pending, `requiredPermissions`, hidden group, which all come back through the "beyond the navigation" door), 8 controls green. Restored: blob `41159b265975` equals HEAD, `git diff HEAD` empty. A first attempt was refused by the tool before running (its replacement text already occurred in the file), so it measured nothing. 2. `CommandPalette.tsx`: `flattenNavigation(navEntriesShownToMember(activeApp?.navigation || [], gates))` replaced by `flattenNavigation(activeApp?.navigation || [])` (the palette's navigation gate dropped): 5 palette cases red (`visible: false`, pending, `requiredPermissions`, hidden group, the gated object entry), 11 green. Restored: blob `2e5450c4f0d7` equals HEAD, `git diff HEAD` empty. 3. `CommandPalette.tsx`: `useNavMemberGates(props.activeApp ?? undefined)` replaced by `useNavMemberGates()` (the palette gating the frame's app): both Home-shape (h) cases red, then restored to the HEAD blob, on `ad2bc8e9`. Ablations 1 and 2 ran on head `c5ed8f31` (before the test-typing commit and the merge of `origin/main`, neither of which touches the two ablated files). </details> <details><summary>Derived gates</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` on `ad2bc8e9` derived 24 commands; each ran with its exit captured before any pipe (`pnpm --dir ui exec vitest run scripts/` under the verify lock: Test Files 176 passed, 2 skipped). `--ran` reconciliation: 24 derived, 24 run, 0 NOT-MEASURED, 0 UNRUN. Model tier: no path-derived mandate. - `node scripts/check-closing-keyword-parity.mjs` exit 0 - `node scripts/check-closing-keyword-parity.mjs --self-test` exit 0 - `node scripts/check-comment-mask-corpus.mjs` exit 0 - `node scripts/check-sdui-manifest.mjs` exit 0 - `node scripts/check-sdui-manifest.mjs --self-test` exit 0 - `pnpm --dir ui exec vitest run scripts/` exit 0 - `pnpm --filter @objectstack/spec run check:duration-unit-keys` exit 0 - `pnpm check:cross-package-test-inputs` exit 0 - `pnpm check:dispatcher-error-vocabulary` exit 0 - `pnpm check:driver-memory-census` exit 0 - `pnpm check:engine-double-contract` exit 0 - `pnpm check:gitlink-declared` exit 0 - `pnpm check:nul-bytes` exit 0 - `pnpm check:published-files` exit 0 - `pnpm check:query-options-erasure` exit 0 - `pnpm check:refd-timer-probe` exit 0 - `pnpm check:skill-identifier-liveness` exit 0 - `pnpm check:test-source-alias` exit 0 - `pnpm check:tier-file-adoption` exit 0 - `pnpm check:type-check-coverage` exit 0 - `pnpm check:type-check-debt` exit 0 - `pnpm check:undeclared-dep-imports` exit 0 - `pnpm check:watch-hint-literal` exit 0 - `pnpm check:where-matcher` exit 0 </details> ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (not touched) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; eslint on the changed files: 0 errors, no warning added. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. Nothing published changes (`@object-ui/app-shell` is `private: true`): `skip-changeset`; both declaration lines are filled in above. - [x] 6. `packages/spec` not touched. - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English. - [x] The *Before Submitting* list in CONTRIBUTING.md is done. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JQwxbZNdnTTr8WJsM98Mxh --- _Generated by [Claude Code](https://claude.ai/code/session_01JQwxbZNdnTTr8WJsM98Mxh)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent e3470ed commit c8097a9

6 files changed

Lines changed: 544 additions & 38 deletions

File tree

‎ui/packages/app-shell/src/chrome/CommandPalette.tsx‎

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ import {
5252
import { useRecordSearch } from '@object-ui/react';
5353
import { usePermissions } from '@object-ui/permissions';
5454
import { useTheme } from './ThemeProvider.js';
55-
import { useNavVisibility } from '../hooks/useNavMemberGates.js';
55+
import { navEntriesShownToMember, useNavMemberGates, type NavMemberGates } from '../hooks/useNavMemberGates.js';
5656
import { useObjectTranslation } from '@object-ui/i18n';
5757
import { resolveKeyedI18nLabel, getRecordDisplayName, appRouteSegment } from '../utils/index.js';
5858
import { getIcon } from '../utils/getIcon.js';
@@ -144,23 +144,36 @@ export function CommandPalette(props: CommandPaletteProps) {
144144
setOpen(false);
145145
}}
146146
>
147-
<CommandPaletteBody {...props} />
147+
{props.scope === 'studio' ? <CommandPaletteBody {...props} gates={null} /> : <AppCommandPaletteBody {...props} />}
148148
</ErrorBoundary>
149149
);
150150
}
151151

152-
function CommandPaletteBody(props: CommandPaletteProps) {
152+
/**
153+
* The app scope's palette, with the member's navigation gates, the sidebar's
154+
* (objectstack#26892), for the app it lists: inside an app that is the
155+
* frame's app; on Home, whose expression scope binds none, it is the host app.
156+
* Built here, not in the body: the `studio` scope has no app, so it asks for
157+
* no gate and reads nothing until it opens.
158+
*/
159+
function AppCommandPaletteBody(props: AppCommandPaletteProps) {
160+
const gates = useNavMemberGates(props.activeApp ?? undefined);
161+
return <CommandPaletteBody {...props} gates={gates} />;
162+
}
163+
164+
/**
165+
* `gates`: the member's navigation gates in the app scope, `null` in the
166+
* `studio` scope, which has no navigation and lists no app report.
167+
*/
168+
function CommandPaletteBody(props: CommandPaletteProps & { gates: NavMemberGates | null }) {
169+
const { gates } = props;
153170
const inStudio = props.scope === 'studio';
154171
const { apps, activeApp, objects, onAppChange, dataSource } = inStudio ? NO_APP : props;
155172
const { open, setOpen } = useCommandPalette();
156173
const [inputValue, setInputValue] = useState('');
157174
const navigate = useNavigate();
158175
const { appName } = useParams();
159176
const { setTheme } = useTheme();
160-
// The shell's one navigation `visible` evaluator, the sidebar's and
161-
// `nav:menu`'s: an entry that declares an API `dataSource` reads its bound
162-
// value as `data`, and is hidden until that read answers (objectui#13029).
163-
const evalVis = useNavVisibility(activeApp);
164177
const { t, language } = useObjectTranslation();
165178
// A nav entry's text, the way the sidebar and `nav:menu` name it: an entry
166179
// with NO `label` shows its target's current label, else its target's machine
@@ -204,14 +217,17 @@ function CommandPaletteBody(props: CommandPaletteProps) {
204217
// its edits; OK, or a clean page, goes at once.
205218
const leaveTo = (to: string) => runCommand(() => confirmUnsavedLeave(() => navigate(to)));
206219

207-
// Extract navigation items from active app, filtering by the spec's one
208-
// navigation gate, `visible`. The `?? item.visibleOn` fallback this read
209-
// once carried was a consumer-side alias of a key the spec never declared on
210-
// a navigation entry (`NavigationItem.visible` is the gate) — dropped with the
211-
// node-level dialects.
212-
const navItems = flattenNavigation(activeApp?.navigation || []).filter(
213-
(item) => evalVis(item.visible, item)
214-
);
220+
// The active app's navigation entries as the sidebar shows them to this
221+
// member (objectstack#26892): `navEntriesShownToMember` under the sidebar's
222+
// gates, the ones `UnifiedSidebar` draws with (the object-read prune, and per
223+
// entry its `visible`, through the shell's one evaluator, so an entry that
224+
// declares an API `dataSource` is hidden until its read answers,
225+
// objectui#13029; its `requiredPermissions`; its runtime-capability gates),
226+
// then flattened. Filtering the flattened leaves by their own `visible`
227+
// alone, as this once did, offered an entry the sidebar hides by
228+
// `requiredPermissions`, by a runtime-capability gate, or by its group's
229+
// `visible`, and opened it empty.
230+
const navItems = gates ? flattenNavigation(navEntriesShownToMember(activeApp?.navigation || [], gates)) : [];
215231

216232
// The entries the palette shows for the query: a navigation entry or app by
217233
// its label and its machine name, a built-in command by its value. They are
@@ -232,7 +248,7 @@ function CommandPaletteBody(props: CommandPaletteProps) {
232248
const navReportNames = new Set(
233249
navItems.filter((item) => item.type === 'report').map((item) => item.reportName as string),
234250
);
235-
const appReports = useAppReportEntries(activeApp, open && !inStudio).entries.filter(
251+
const appReports = useAppReportEntries(activeApp, open && !inStudio, gates).entries.filter(
236252
(entry) => !navReportNames.has(entry.name) && matches(entry.label, entry.name),
237253
);
238254
const switchableApps = apps.filter((a) => a.active !== false);

‎ui/packages/app-shell/src/hooks/useNavMemberGates.ts‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,9 @@
5858
* its `visible` reads the value the binding selects as `data`
5959
* (`data.total == 1`). Only `visible` reads it: not the label, not the target,
6060
* and not the entry's `children`. {@link useNavVisibility} is the one evaluator
61-
* every navigation surface of the shell hands its guard: the sidebar and the
62-
* landing through {@link useNavMemberGates}, the `nav:menu` block and the
63-
* command palette directly. It reads each binding an app's navigation declares
61+
* every navigation surface of the shell hands its guard: the sidebar, the
62+
* landing and the command palette through {@link useNavMemberGates}, the
63+
* `nav:menu` block directly. It reads each binding an app's navigation declares
6464
* through `@object-ui/react`'s element reader (`readElementApi`, the request a
6565
* page element's binding makes, with the host's `apiFetch`), so there is no
6666
* second fetch; and in the open the read is asked in (`ElementApiOpenScope`,
@@ -254,8 +254,8 @@ export function navEntryVisible(
254254
/**
255255
* The `visible` evaluator for `app`'s navigation: {@link navEntryVisible} over
256256
* the shell's expression evaluator and the bindings `app` declares. The sidebar
257-
* (through {@link useNavMemberGates}), the `nav:menu` block and the command
258-
* palette each build theirs here, so they cannot disagree.
257+
* and the command palette (through {@link useNavMemberGates}) and the `nav:menu`
258+
* block each build theirs here, so they cannot disagree.
259259
*/
260260
export function useNavVisibility(
261261
app: unknown,
@@ -308,9 +308,15 @@ export interface NavMemberGatesWithReads extends NavMemberGates {
308308
/**
309309
* The member's gates, built from the providers the console mounts: the
310310
* expression evaluator, `usePermissions()`, and the registered object list.
311+
*
312+
* `forApp` names the app whose navigation is gated when it is not the frame's
313+
* own (objectstack#26892): Home binds no app in its expression scope yet hands
314+
* the palette its host app, whose entries' API bindings and package object
315+
* scope are that app's. Omitted, the frame's app is gated, as before.
311316
*/
312-
export function useNavMemberGates(): NavMemberGatesWithReads {
313-
const { app } = useExpressionContext();
317+
export function useNavMemberGates(forApp?: Record<string, unknown>): NavMemberGatesWithReads {
318+
const { app: frameApp } = useExpressionContext();
319+
const app: Record<string, unknown> = forApp ?? frameApp;
314320
// objectui#13029 — the entries of the app this frame renders may bind data.
315321
const { evaluate: evalVis, readEntry } = useNavVisibilityAndReads(app);
316322

‎ui/packages/app-shell/src/views/AppReportIndexView.tsx‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,21 +12,25 @@
1212
* to its own route, `report/NAME`. Which reports, and under which read rule,
1313
* is `useAppReportEntries`'s answer (`appReportIndex.ts`); the ⌘K palette
1414
* reads the same one. The sidebar links here from every app, so a report the
15-
* app's navigation does not name still has a UI path.
15+
* app's navigation does not name still has a UI path. A report it names is
16+
* listed only when the sidebar's gates show the member an entry naming it
17+
* (objectstack#26892).
1618
*/
1719

1820
import { Link, useParams } from 'react-router-dom';
1921
import { BarChart3, ChevronRight, Loader2 } from 'lucide-react';
2022
import { Empty, EmptyTitle, EmptyDescription } from '@object-ui/components';
2123
import { useObjectTranslation } from '@object-ui/i18n';
2224
import { useExpressionContext } from '../providers/ExpressionProvider.js';
25+
import { useNavMemberGates } from '../hooks/useNavMemberGates.js';
2326
import { useAppReportEntries } from './appReportIndex.js';
2427

2528
export function AppReportIndexView() {
2629
const { t } = useObjectTranslation();
2730
const { appName } = useParams<{ appName: string }>();
2831
const { app } = useExpressionContext();
29-
const { entries, loading } = useAppReportEntries(app, true);
32+
const gates = useNavMemberGates();
33+
const { entries, loading } = useAppReportEntries(app, true, gates);
3034
const title = t('console.reportIndex.title', { defaultValue: 'Reports' });
3135

3236
if (loading) {

‎ui/packages/app-shell/src/views/__tests__/AppReportIndex-13020.test.tsx‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,11 @@ function adapterFor(reports: unknown[], asked: string[]) {
108108
},
109109
getClient: () => ({
110110
meta: {
111-
getItems: (type: string) => {
112-
asked.push(type);
111+
// A package-scoped read is recorded with its scope: the navigation
112+
// gates read the app's object scope, as the sidebar does
113+
// (objectstack#26892), which is not the object registry.
114+
getItems: (type: string, params?: { packageId?: string }) => {
115+
asked.push(params?.packageId ? `${type}?packageId=${params.packageId}` : type);
113116
return Promise.resolve({ type, items: items[type] ?? [] });
114117
},
115118
getItem: () => Promise.resolve({ item: null }),
@@ -202,6 +205,8 @@ describe('the app report index (objectui#13020)', () => {
202205
expect(asked).toContain('report');
203206
expect(asked).toContain('dataset');
204207
expect(asked).not.toContain('object');
208+
// The sidebar's own read, through its gates: the app's package scope.
209+
expect(asked).toContain(`object?packageId=${PKG}`);
205210
});
206211

207212
it('CONTROL: an app that ships no report shows the empty state', async () => {

0 commit comments

Comments
 (0)