Skip to content

Commit 7788eed

Browse files
fix(app-shell): a member refused the Setup app gets the access-denied screen on the Setup create form too (#29237)
<!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} --> _Requested by **Steedos** · [project thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9UbbLD68mjjNpw7dAnZ7BBKoKi)_ Fixes #29102 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:没有 Setup 应用权限的普通成员打开 Setup 下的新建表单(`/apps/setup/sys_user/new` 这类别名地址)时,现在看到与同一对象列表页相同的「无权访问此应用」页面;规范地址 `/apps/com.objectstack.setup/OBJECT/new` 也是同一页面。 - 为什么改:上一张卡只拦了列表路由,新建表单仍会套着默认应用的顶栏和侧栏显示,地址栏写着 setup,像进了管理后台。 - 风险与回滚:只改界面判定,服务端对每次写入的权限判定不变;管理员不受影响(不发起探测)。审批收件箱链接到的记录页 `/apps/setup/OBJECT/record/ID` 按 objectui#12706 的未决决定保持原样。回滚即还原本 PR。 - 先看:`ui/packages/app-shell/src/console/AppContent.tsx`(`OBJECT_COLLECTION_ROUTE_PATHS` 新增的 `:objectName/new` 一行)。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/app-shell/src/console/__tests__/AppContent.setupObjectListVerdict-28988.test.tsx`,预期 16 个用例全绿。 - 需要你做的:无,席位落地。 ## What changes - The route set that waits on the Setup app's access verdict under the `setup` alias now includes the create form, `:objectName/new`, beside the list routes #28988 added (`:objectName`, `:objectName/view/:viewId`, `:objectName/data`). A `denied` verdict draws `app-access-denied` and the form never mounts. - The set and its matcher are renamed for what they now hold, the object routes that name no record: `OBJECT_LIST_ROUTE_PATHS` becomes `OBJECT_COLLECTION_ROUTE_PATHS`, `isObjectListAppRoute` becomes `isObjectCollectionAppRoute`, and `isSetupObjectListRoute` becomes `isSetupObjectCollectionRoute`. Matching is unchanged: by route rank against `APP_ROUTE_METADATA_READS` plus the host's routes, so a static or host route under the Setup segment is never read as one. - Record routes (`OBJECT/record/ID`, its `/edit` and `/page/NAME`, and the `OBJECT/ID` shorthand) stay outside the gate, as objectui#12706 leaves them. No metadata key or option is added. ## Why `/apps/setup` is a pseudo-route, so an unmatched `setup` segment falls back to the launcher's default app unless `appSurfaceAwaitsVerdict` names the sub-route. After #28988 it named the list routes only, so a refused member on `/apps/setup/OBJECT/new` got the create form inside the default app's chrome. The create form names no record, so no approvals-inbox link lands on it. The canonical `/apps/com.objectstack.setup/OBJECT/new` is an ordinary unmatched app and already drew the refusal; it is pinned here so both spellings stay the same. ## Tests - `AppContent.setupObjectListVerdict-28988.test.tsx` gains a stubbed `RecordFormPage` (counted like the other page stubs) and four cases: - A refused caller gets `app-access-denied`, no `ConsoleLayout` and no page mounted, on `/apps/setup/sys_user/new`, `/apps/setup/showcase_task/new` and `/apps/com.objectstack.setup/sys_user/new`. The one by-name probe is the app the URL names, and the URL is left as typed. - Control: an admitted caller gets the create form (`mode="create"`) under Setup, unprobed. - The existing control that a refused caller still opens the record page the approvals inbox links to (`/apps/setup/sys_user/record/usr_000001`) passes unchanged, as do the objectui#12706 file's inbox controls. - Red on `main`: with `AppContent.tsx` at `c44c8ac4` and the new tests unchanged, the two alias cases fail (Tests 2 failed, 14 passed). The package-id case and the controls pass there. ## Acceptance notes - The canonical spelling needed no code change; the test pins it so the two spellings cannot drift apart again. - Record routes under the Setup segment are untouched, per the claim and objectui#12706. ## Verification <details><summary>Local runs</summary> Head `83289601` (fix `ead56761` plus a merge of `main` at `49aba82b`; base `c44c8ac4`). The suites below marked "merged head" ran on `83289601`; the rest ran on `ead56761`, and the merge brought nothing into `ui/packages/app-shell/src/console/AppContent.tsx` or its tests. - Prerequisite build under the verify lock: `turbo run build --filter='@object-ui/app-shell^...'` plus formula, sdui-parser, lint and client, 64 of 64 tasks. - The edited file with the #27507, objectui#12706 and #28578 Setup verdict files: Test Files 4 passed, Tests 119 passed. - Red on `main` (`AppContent.tsx` at `c44c8ac4`, restored to the HEAD blob afterwards, `git diff HEAD` empty): Tests 2 failed, 14 passed. - Every `packages/app-shell` test file that names `AppContent` (`git grep -l`, 93 files), on `ead56761` and again on the merged head after rebuilding the closure: Test Files 93 passed, Tests 777 passed, both times. A narrowing of app-shell's suite; CI's Test UI shards run the rest. - `turbo run type-check --filter=@object-ui/app-shell` (its script also runs `tsc -p tsconfig.test.json`, so the test is compiled): 65 of 65 tasks. - `node scripts/pm/dispatch-gates.mjs --commands`: 34 derived commands, each exit 0, among them `check-sdui-manifest.mjs`, `check-changeset-no-major.mjs --base origin/main`, `check:type-check-debt`, `check:published-files`, `check:nul-bytes` and `pnpm --dir ui exec vitest run scripts/` (Test Files 169 passed, 2 skipped; Tests 4900 passed, 2 skipped). `--ran` reconciliation: 34 derived, 34 run, 0 NOT-MEASURED. The derivation on the merged head is identical, and the PR-body and changeset gates were run again there against this body. `pnpm check:new-line-citations`: 0 new citations. - Not measured: a browser run of the built Console as a real member. The test drives the real adapter's probe over the server's 403 envelope. </details> ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (`packages/spec` not touched.) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` is CI's run. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: not applicable. - [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 (the 维护者速读 section is the Chinese summary the repo asks for). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01QsR2jU9jKHFCJuQuuKRtpX --- _Generated by [Claude Code](https://claude.ai/code/session_01QsR2jU9jKHFCJuQuuKRtpX)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 39951d2 commit 7788eed

3 files changed

Lines changed: 73 additions & 17 deletions

File tree

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
Clause-②: no
6+
Metadata-protocol: none
7+
8+
A member the Setup app refuses who opens an object's create form under the Setup segment, `/apps/setup/OBJECT/new`, now gets the same "You don't have access to this app" screen as the object list beside it, instead of the create form inside the default app's header and sidebar. The package-id spelling `/apps/com.objectstack.setup/OBJECT/new` draws the same screen. Admins see the form as before, and the record pages the approvals inbox links to (`/apps/setup/OBJECT/record/ID`) are unchanged.

‎ui/packages/app-shell/src/console/AppContent.tsx‎

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -202,24 +202,31 @@ function preloadAppRouteChunk(subPath: string, hostPaths: readonly string[] = []
202202
}
203203

204204
/**
205-
* objectstack#28988 — the route paths that draw an object's LIST: the default
206-
* list, a saved list view and the bare data surface. Record-scoped paths
207-
* (`:objectName/record/...`, the `:objectName/:maybeRecordId` shorthand) and
208-
* the create form are deliberately absent; see `appSurfaceAwaitsVerdict`.
205+
* The object-scoped route paths that name no record: the default list, a saved
206+
* list view, the bare data surface (objectstack#28988) and the create form
207+
* (objectstack#29102). Record-scoped paths (`:objectName/record/...`, the
208+
* `:objectName/:maybeRecordId` shorthand) are deliberately absent; see
209+
* `appSurfaceAwaitsVerdict`.
209210
*/
210-
const OBJECT_LIST_ROUTE_PATHS: ReadonlySet<string> = new Set([':objectName', ':objectName/view/:viewId', ':objectName/data']);
211+
const OBJECT_COLLECTION_ROUTE_PATHS: ReadonlySet<string> = new Set([
212+
':objectName',
213+
':objectName/view/:viewId',
214+
':objectName/data',
215+
':objectName/new',
216+
]);
211217

212218
/**
213-
* Whether the route at `subPath` (the path below `/apps/:appName`) draws an
214-
* object's list. Matched the way `<Routes>` ranks paths, beside every path
215-
* `AppContent` and the host declare, so a static route or a host route that
216-
* shadows `:objectName` (`search`, `reports`, `system/...`) is not one.
219+
* Whether the route at `subPath` (the path below `/apps/:appName`) is one of
220+
* `OBJECT_COLLECTION_ROUTE_PATHS`. Matched the way `<Routes>` ranks paths,
221+
* beside every path `AppContent` and the host declare, so a static route or a
222+
* host route that shadows `:objectName` (`search`, `reports`, `system/...`) is
223+
* not one.
217224
*/
218-
function isObjectListAppRoute(subPath: string, hostPaths: readonly string[] = []): boolean {
225+
function isObjectCollectionAppRoute(subPath: string, hostPaths: readonly string[] = []): boolean {
219226
const routes = [...APP_ROUTE_METADATA_READS.map(({ path }) => ({ path })), ...hostPaths.map((path) => ({ path }))];
220227
const matches = matchRoutes(routes, `/${subPath.replace(/^\/+/, '')}`);
221228
const path = matches?.[matches.length - 1].route.path;
222-
return path !== undefined && OBJECT_LIST_ROUTE_PATHS.has(path);
229+
return path !== undefined && OBJECT_COLLECTION_ROUTE_PATHS.has(path);
223230
}
224231

225232
interface AppContentProps {
@@ -579,16 +586,21 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps =
579586
// while the canonical `/apps/com.objectstack.setup/OBJECT`, an ordinary
580587
// unmatched app, already drew the refusal. The record routes the approvals
581588
// inbox links to stay outside this gate, as above; the list paths are
582-
// matched by route rank (`isObjectListAppRoute`), so they never reach one.
589+
// matched by route rank (`isObjectCollectionAppRoute`), so they never reach
590+
// one.
591+
//
592+
// objectstack#29102 — and the create form beside them (`/apps/setup/OBJECT/
593+
// new`), which a member reached the same way. It names no record, so no
594+
// inbox link lands on it.
583595
const isSetupRootRoute = location.pathname === '/apps/setup' || location.pathname === '/apps/setup/';
584596
const [appSubRoot, appSubPage] = appSubPath.split('/');
585597
const isSettingsOrMarketplaceRoute =
586598
appSubRoot === 'system' && (appSubPage === 'settings' || appSubPage === 'marketplace');
587-
const isSetupObjectListRoute = isSetupRoute && isObjectListAppRoute(appSubPath, hostRoutePaths);
599+
const isSetupObjectCollectionRoute = isSetupRoute && isObjectCollectionAppRoute(appSubPath, hostRoutePaths);
588600
const appSurfaceAwaitsVerdict =
589601
!!appName &&
590602
!matchedApp &&
591-
(isMetadataRoute || isSetupRootRoute || isSettingsOrMarketplaceRoute || isSetupObjectListRoute);
603+
(isMetadataRoute || isSetupRootRoute || isSettingsOrMarketplaceRoute || isSetupObjectCollectionRoute);
592604

593605
// Post-publish readiness: when a requested app isn't in the loaded metadata,
594606
// re-check ONCE (the registry can lag a beat behind a publish) before

‎ui/packages/app-shell/src/console/__tests__/AppContent.setupObjectListVerdict-28988.test.tsx‎

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@
2222
*
2323
* The alias-spelled list routes (`OBJECT`, `OBJECT/view/ID`, `OBJECT/data`)
2424
* wait on the verdict for the app the URL names and draw `app-access-denied`
25-
* on `denied`, nothing of the list mounted. An admitted caller and a server
25+
* on `denied`, nothing of the list mounted. So does the create form beside
26+
* them, `OBJECT/new` (objectstack#29102), in both spellings; an admitted
27+
* caller still gets the form. An admitted caller and a server
2628
* without the app render as before. The record routes the approvals inbox
2729
* links to stay reachable (objectui#12706 leaves them open), as does the inbox
2830
* itself; `AppContent.setupItemRoutesVerdict-12706.test.tsx` pins both too.
@@ -34,7 +36,7 @@
3436
* real probe reading the real envelope.
3537
*
3638
* NOTE ON SCOPE: this file measures WHICH SURFACE renders. `ConsoleLayout` and
37-
* the list, data and record pages are stubbed; each stub counts its mounts, so
39+
* the list, data, record and create-form pages are stubbed; each stub counts its mounts, so
3840
* "nothing of the list mounted" is a count.
3941
*/
4042

@@ -70,6 +72,12 @@ vi.mock('../../views/ObjectDataPage', () => ({
7072
return counted('object-data-page', objectName);
7173
},
7274
}));
75+
vi.mock('../../views/RecordFormPage', () => ({
76+
RecordFormPage: ({ mode }: { mode?: string }) => {
77+
const { objectName } = useParams<{ objectName?: string }>();
78+
return counted('record-form-page', `${objectName}/${mode ?? ''}`);
79+
},
80+
}));
7381
vi.mock('../../views/RecordDetailView', () => ({
7482
RecordDetailView: () => {
7583
const { objectName, recordId } = useParams<{ objectName?: string; recordId?: string }>();
@@ -246,7 +254,7 @@ function renderConsoleAt(initialUrl: string) {
246254
const pathname = () => screen.getByTestId('pathname').textContent;
247255

248256
/** Every page stub this file counts; a refused caller mounts none of them. */
249-
const PAGE_STUBS = ['object-view', 'object-data-page', 'record-detail-view', 'approvals-inbox-page'];
257+
const PAGE_STUBS = ['object-view', 'object-data-page', 'record-form-page', 'record-detail-view', 'approvals-inbox-page'];
250258
const pagesMounted = () => PAGE_STUBS.filter((id) => (mounts[id] ?? 0) > 0);
251259

252260
describe('AppContent — alias-spelled Setup object lists wait on the app\'s access verdict (objectstack#28988)', () => {
@@ -298,6 +306,34 @@ describe('AppContent — alias-spelled Setup object lists wait on the app\'s acc
298306
expect(metaItemRequests[0]).toContain('/api/v1/meta/app/com.objectstack.setup');
299307
});
300308

309+
// objectstack#29102 — the create form names no record, so no inbox link
310+
// lands on it; it waits on the verdict like the lists beside it.
311+
it.each([
312+
['the alias', '/apps/setup/sys_user/new', 'setup'],
313+
['an app object under the alias', '/apps/setup/showcase_task/new', 'setup'],
314+
['the package id', '/apps/com.objectstack.setup/sys_user/new', 'com.objectstack.setup'],
315+
])('a refused caller gets app-access-denied on the create form spelled with %s, never the form', async (_label, url, app) => {
316+
renderConsoleAt(url);
317+
318+
expect(await screen.findByTestId('app-access-denied')).toBeInTheDocument();
319+
expect(screen.queryByTestId('console-layout')).not.toBeInTheDocument();
320+
expect(pagesMounted()).toEqual([]);
321+
expect(metaItemRequests).toHaveLength(1);
322+
expect(metaItemRequests[0]).toContain(`/api/v1/meta/app/${app}`);
323+
expect(pathname()).toBe(url);
324+
});
325+
326+
it('CONTROL — an admitted caller gets the create form under Setup, and nothing is probed', async () => {
327+
metadataApps = [SHOWCASE_APP, SETUP_APP];
328+
329+
renderConsoleAt('/apps/setup/sys_user/new');
330+
331+
expect(await screen.findByTestId('record-form-page')).toHaveTextContent('sys_user/create');
332+
expect(screen.getByTestId('console-layout')).toHaveAttribute('data-active-app', 'setup');
333+
expect(screen.queryByTestId('app-access-denied')).not.toBeInTheDocument();
334+
expect(metaItemRequests).toEqual([]);
335+
});
336+
301337
it('CONTROL — an admitted caller gets the list under Setup, and nothing is probed', async () => {
302338
metadataApps = [SHOWCASE_APP, SETUP_APP];
303339

0 commit comments

Comments
 (0)