Skip to content

Commit fed3a51

Browse files
fix(app-shell): a ⌘K record hit opens under the app that lists its object (#28964)
<!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} --> _Requested by **Steedos** · [project thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9UbbWpUA7fw8QiGG9YiDDSgEUC)_ Fixes #28890 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:⌘K 搜到的记录,现在在「列出该对象的应用」里打开:当前应用列出它就留在当前应用;否则去第一个列出它的可打开应用;都没有才留在当前应用。 - **为什么改**:没有对象导航的应用(如 cloud 的「账户」应用)会搜全部对象,搜到的环境记录却按「账户」应用拼链接,打开后左栏和应用切换器都不对。 - **风险与回滚**:只改记录命中的链接,当前应用列出该对象时行为不变;回滚即 revert 本 PR。 - **先看**:`ui/packages/app-shell/src/chrome/CommandPalette.tsx` 的 `recordHitAppSegment` 与 `appListsObject`;其次新测试文件。 - **两分钟验证**:`cd ui && pnpm exec vitest run packages/app-shell/src/chrome/CommandPalette.recordHitApp-28890.test.tsx`,预期 4 个用例全绿。 - **需要你做的**:无,席位落地。 ## What changes `CommandPalette.tsx` built every record hit's link from the palette's own app (`baseUrl`). An app with no object navigation searches every object (the scope at `CommandPalette.tsx` `searchableObjectNames`), so its hits named objects it does not list, and each one opened under that app with the wrong sidebar and switcher. A record hit now opens under: 1. the palette's own app, when it lists the hit's object (unchanged behaviour); 2. otherwise the first app the user can open that lists it: `filterActiveApps` (active, not hidden), in metadata order, addressed by `appAddressSegment`; 3. otherwise the palette's own app. "Lists" means a `type: 'object'` entry naming the object at any depth of the app's `navigation` or any `areas[].navigation`, among the entries the member is shown there: `navEntriesShownToMember` under the palette's gates, walked with `navigationNames`. That is the way `activityObjectRoster` reads every app's navigation. No new resolver module. On `main` at `9a9823d9` no console helper resolves an app from an object: `resolveHostAppSegment` (the card's named example) takes no object and answers "the app the user is in, else the first active app", and every other record-link producer uses the current app or `resolveHostAppSegment`. The two private functions added here compose the existing helpers (`filterActiveApps`, `appAddressSegment`, `navEntriesShownToMember`, `navigationNames`); only the "does this app list this object" predicate is new, and it stays private to the palette. ## Tests `ui/packages/app-shell/src/chrome/CommandPalette.recordHitApp-28890.test.tsx` mounts the real palette and `CommandPaletteProvider`, chooses the record hit, and reads the address off the router: - from a component-only app, the hit opens under the first app that lists the object (skipping an app that lists nothing and a hidden app that lists it), addressed by its package id; - an app that lists the object only in an `areas[].navigation` counts as listing it; - CONTROL: from an app that lists the object, the hit stays there, even with an earlier app that also lists it; - CONTROL: when no openable app lists the object, the hit stays under the palette's app. Ablation (`scripts/ablation-replace.mjs`, mutation landed and restored to the HEAD blob): with the hit's base forced back to `baseUrl`, the two branch cases go red (`Received: "/apps/account/sys_environment_28890/record/E1"`) and the two controls stay green. ## Acceptance notes - Acceptance 1 is measured with a local reproduction (a component-only app and an object another app lists), not on the cloud control plane; the cloud repository was not needed. - Out of scope, per the ruling: the search scope (an app with no object navigation searches every object) is left as it is. It is what lets such an app find these records at all, and this PR only changes where they open. - A hit whose object is listed by several other apps opens under the first in metadata order, never a "best" one. ## Verification Local readings, `objectstack-ai/objectstack`. Derived gates on `af0d5914` (the fix and changeset, before the main merge); tests, type-check and the key gates re-run on the final head `938ba76b` (main merged at `a3640933`, plus a type-only commit). <details><summary>Derived gates: 34 derived, 34 run, 0 NOT-MEASURED, all exit 0</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 34 families for the 3 changed paths (199 changed lines, under 3000). Every one exited 0, including `node scripts/check-sdui-manifest.mjs` and its `--self-test`, the changeset gates (run with this body as the event), `pnpm check:cross-package-test-inputs`, `pnpm check:test-source-alias`, `pnpm check:type-check-coverage` / `-debt`, and `pnpm --dir ui exec vitest run scripts/` (169 files passed, 2 skipped). `dispatch-gates --ran` reconciled: 34 run, 0 NOT-MEASURED, 0 UNRUN. On `938ba76b`: `check-sdui-manifest`, `check-changeset-no-major --base origin/main`, `check-empty-changeset --base origin/main`, `check:nul-bytes`, `check:type-check-debt`, `check:type-check-coverage`, `check:undeclared-dep-imports`: all exit 0. </details> <details><summary>Tests and type-check</summary> - New file: 4/4 passed. - Every test file that names `CommandPalette` (`git grep -l`, 71 files: 66 under `ui/packages`, 5 under `ui/apps/console`): 609 tests passed, on `af0d5914`. All 23 files in `ui/packages/app-shell/src/chrome/` (144 tests) passed again on `938ba76b`. - `turbo run type-check --filter=@object-ui/app-shell` (`tsc --noEmit` and `tsc -p tsconfig.test.json`): exit 0; both re-run on `938ba76b`, exit 0. - `eslint` on the two touched files: 0 errors, and no warning the file did not already have on `main`. - The whole app-shell suite is left to CI's Test UI. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01QsR2jTYffwZcQvQX8pCSso)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01QsR2jTYffwZcQvQX8pCSso --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 65a8d4f commit fed3a51

3 files changed

Lines changed: 201 additions & 1 deletion

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
fix(app-shell): a ⌘K record hit opens under the app that lists its object, not under the app the palette was opened in (#28890)
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
An app whose navigation names no object (a component-only app, such as cloud's Account app) searches every object from the command palette, so its hits name objects it does not list. Each hit opened under that app all the same, and the record page drew its sidebar and app switcher, neither of which reaches the object: an environment found from the Account app opened in the Account app, not in the Cloud app that lists environments.
11+
12+
- **Now.** A record hit opens under the palette's own app when that app lists the hit's object; otherwise under the first app the user can open (active, not hidden, in metadata order) that lists it; and under the palette's own app when none does. "Lists" means a `type: 'object'` entry naming the object, at any depth of the app's `navigation` or of any `areas[].navigation`, among the entries the member is shown there.
13+
- **Unchanged.** When the palette's app lists the object, the hit opens there as before. The palette's search scope is untouched: an app with no object navigation still searches every object.
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
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+
/**
10+
* A ⌘K record hit opens under the app that lists its object
11+
* (objectstack#28890).
12+
*
13+
* An app whose navigation names no object (a component-only app, as cloud's
14+
* Account app is) searches every object, so its hits name objects it does not
15+
* list. Each opened under that app all the same, `/apps/<palette app>/OBJECT/
16+
* record/ID`, and the record page drew a sidebar and a switcher that reach
17+
* nothing of the object. The hit now opens under the palette's own app when
18+
* that app lists the object, else under the first app the user can open that
19+
* lists it, else under the palette's own app.
20+
*
21+
* Pinned against the real `CommandPaletteProvider` (opened through its
22+
* `?palette=1` deep link) over the per-object `find` fanout; the address is
23+
* read off the router after the hit is chosen.
24+
*/
25+
26+
import * as React from 'react';
27+
import { describe, it, expect, vi, afterEach } from 'vitest';
28+
import { render, fireEvent, waitFor, cleanup, screen } from '@testing-library/react';
29+
import { MemoryRouter, useLocation } from 'react-router-dom';
30+
31+
vi.mock('@object-ui/auth', async (importOriginal) => ({
32+
...(await importOriginal<Record<string, unknown>>()),
33+
useAuth: () => ({ user: { id: 'u1' }, activeOrganization: null }),
34+
}));
35+
36+
import { CommandPalette } from './CommandPalette';
37+
import { CommandPaletteProvider } from '../context/CommandPaletteProvider';
38+
39+
const OBJECT = 'sys_environment_28890';
40+
const REC = 'E1';
41+
const RECORD = { id: REC, name: 'Release Check 1001' };
42+
const OBJECTS = [{ name: OBJECT, label: 'Environment', fields: { name: { label: 'Name', type: 'text' } } }];
43+
44+
/** A component-only app: its navigation names no object. */
45+
const ACCOUNT = {
46+
name: 'account',
47+
label: 'Account',
48+
navigation: [{ id: 'n_profile', type: 'component', componentName: 'account/profile_card', label: 'Profile' }],
49+
};
50+
/** An app that does not list the object either, ahead of the one that does. */
51+
const DOCS = { name: 'docs', label: 'Docs', navigation: [{ id: 'n_docs', type: 'page', pageName: 'docs_home', label: 'Docs' }] };
52+
/** The app that lists the object, inside a group, addressed by its package id. */
53+
const CLOUD = {
54+
name: 'cloud',
55+
_packageId: 'com.objectstack.cloud',
56+
label: 'Cloud',
57+
navigation: [
58+
{
59+
id: 'g_infra',
60+
type: 'group',
61+
label: 'Infrastructure',
62+
children: [{ id: 'n_env', type: 'object', objectName: OBJECT, label: 'Environments' }],
63+
},
64+
],
65+
};
66+
/** Lists the object in an area's navigation, not in its own. */
67+
const OPS = {
68+
name: 'ops',
69+
label: 'Ops',
70+
navigation: [],
71+
areas: [{ id: 'a_main', label: 'Main', navigation: [{ id: 'n_env', type: 'object', objectName: OBJECT, label: 'Environments' }] }],
72+
};
73+
/** Lists the object but is hidden from the launcher: no place to send anyone. */
74+
const HIDDEN = { ...CLOUD, name: 'hidden_cloud', _packageId: undefined, hidden: true };
75+
76+
function Probe() {
77+
const location = useLocation();
78+
return <span data-testid="location" data-path={location.pathname} />;
79+
}
80+
81+
function mount(apps: Array<Record<string, unknown>>, activeApp: Record<string, unknown>) {
82+
const ds = { find: vi.fn(async (name: string) => ({ data: name === OBJECT ? [{ ...RECORD }] : [] })) };
83+
const segment = (activeApp._packageId as string | undefined) ?? (activeApp.name as string);
84+
render(
85+
<MemoryRouter initialEntries={[`/apps/${segment}?palette=1`]}>
86+
<CommandPaletteProvider>
87+
<Probe />
88+
<CommandPalette apps={apps} activeApp={activeApp} objects={OBJECTS} onAppChange={() => {}} dataSource={ds} />
89+
</CommandPaletteProvider>
90+
</MemoryRouter>,
91+
);
92+
const input = document.querySelector('[cmdk-input]');
93+
if (!input) throw new Error('the palette did not open');
94+
fireEvent.change(input, { target: { value: 'release' } });
95+
}
96+
97+
/** Choose the record hit once it is on screen (the hook debounces 250ms); answer the address it opened. */
98+
async function openHit(): Promise<string | null> {
99+
const item = await waitFor(
100+
() => {
101+
const found = document.querySelector('[cmdk-item][data-value^="record "]');
102+
if (!found) throw new Error('the record hit is not on screen yet');
103+
return found;
104+
},
105+
{ timeout: 4000 },
106+
);
107+
fireEvent.click(item);
108+
return screen.getByTestId('location').getAttribute('data-path');
109+
}
110+
111+
afterEach(() => cleanup());
112+
113+
describe('objectstack#28890 — a ⌘K record hit opens under the app that lists its object', () => {
114+
it('from an app that does not list the object, the hit opens under the first app that does', async () => {
115+
mount([ACCOUNT, DOCS, HIDDEN, CLOUD, OPS], ACCOUNT);
116+
expect(await openHit()).toBe(`/apps/com.objectstack.cloud/${OBJECT}/record/${REC}`);
117+
});
118+
119+
it('an app that lists the object only in an area counts as listing it', async () => {
120+
mount([ACCOUNT, OPS, CLOUD], ACCOUNT);
121+
expect(await openHit()).toBe(`/apps/ops/${OBJECT}/record/${REC}`);
122+
});
123+
124+
it('CONTROL: from an app that lists the object, the hit stays under that app', async () => {
125+
mount([ACCOUNT, OPS, CLOUD], CLOUD);
126+
expect(await openHit()).toBe(`/apps/com.objectstack.cloud/${OBJECT}/record/${REC}`);
127+
});
128+
129+
it('CONTROL: when no app the user can open lists the object, the hit stays under the palette app', async () => {
130+
mount([ACCOUNT, DOCS, HIDDEN], ACCOUNT);
131+
expect(await openHit()).toBe(`/apps/account/${OBJECT}/record/${REC}`);
132+
});
133+
});

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

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ import { useTheme } from './ThemeProvider.js';
5555
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';
58+
import { appAddressSegment, filterActiveApps } from '../utils/appRoute.js';
59+
import { navigationNames } from '../console/appObjectScope.js';
5860
import { getIcon } from '../utils/getIcon.js';
5961
import { useRecentItems } from '../context/RecentItemsProvider.js';
6062
import { useRecentItemLabel } from '../hooks/useRecentItemLabel.js';
@@ -115,6 +117,53 @@ const NO_APP: Omit<AppCommandPaletteProps, 'scope'> = {
115117
dataSource: undefined,
116118
};
117119

120+
/** The keys of an app the record-hit resolution reads: its route keys and navigation trees. */
121+
type PaletteApp = { navigation?: unknown; areas?: unknown } & Record<string, unknown>;
122+
123+
/**
124+
* Whether `app` lists `objectName` (objectstack#28890): a `type: 'object'`
125+
* entry naming it, at any depth of its `navigation` or of any
126+
* `areas[].navigation`, among the entries the member is shown there
127+
* (`navEntriesShownToMember` under the palette's gates, as
128+
* `activityObjectRoster` reads every app's navigation). `gates` is `null`
129+
* only in the `studio` scope, which searches no record.
130+
*/
131+
function appListsObject(app: PaletteApp | null | undefined, objectName: string, gates: NavMemberGates | null): boolean {
132+
if (!app) return false;
133+
const areas: unknown[] = Array.isArray(app.areas) ? app.areas : [];
134+
const trees = [app.navigation, ...areas.map((area) => (area as { navigation?: unknown } | null)?.navigation)];
135+
const pick = (item: Record<string, unknown>) => (item.type === 'object' ? item.objectName : undefined);
136+
return trees.some(
137+
(tree) =>
138+
Array.isArray(tree) &&
139+
navigationNames(gates ? navEntriesShownToMember(tree as NavigationItem[], gates) : tree, pick).includes(objectName),
140+
);
141+
}
142+
143+
/**
144+
* The app a record hit opens under (objectstack#28890), as the segment of its
145+
* `/apps/APP` link: the palette's own app when it lists the hit's object;
146+
* otherwise the first app the user can open (`filterActiveApps`, metadata
147+
* order) that lists it, addressed by `appAddressSegment`; and the palette's
148+
* own app again when none does, so `undefined` means "stay here".
149+
*
150+
* An app with no object navigation searches every object (the scope below),
151+
* so its hits named objects it does not list, and each opened under it: the
152+
* record page drew that app's sidebar and switcher, neither of which reaches
153+
* the object (cloud's Account app, a component-only app, opened an
154+
* environment there instead of under the Cloud app that lists it).
155+
*/
156+
function recordHitAppSegment(
157+
apps: readonly PaletteApp[],
158+
activeApp: PaletteApp | null | undefined,
159+
objectName: string,
160+
gates: NavMemberGates | null,
161+
): string | undefined {
162+
if (appListsObject(activeApp, objectName, gates)) return undefined;
163+
const host = filterActiveApps(apps).find((app) => appListsObject(app, objectName, gates));
164+
return host ? appAddressSegment(apps, host) : undefined;
165+
}
166+
118167
/**
119168
* The palette, behind its own error boundary (objectui#13387). It is mounted in
120169
* the console's frame, beside the routes, so an error thrown inside it (a
@@ -196,6 +245,11 @@ function CommandPaletteBody(props: CommandPaletteProps & { gates: NavMemberGates
196245
}, [open]);
197246

198247
const baseUrl = `/apps/${appName || appRouteSegment(activeApp)}`;
248+
// A record hit's own base: the app that lists its object (objectstack#28890).
249+
const recordHitBaseUrl = (objectName: string): string => {
250+
const segment = recordHitAppSegment(apps, activeApp, objectName, gates);
251+
return segment ? `/apps/${segment}` : baseUrl;
252+
};
199253
const { user, activeOrganization } = useAuth();
200254
const templateContext = useMemo(
201255
() => ({ currentUserId: user?.id ?? null, currentOrgId: activeOrganization?.id ?? null }),
@@ -483,7 +537,7 @@ function CommandPaletteBody(props: CommandPaletteProps & { gates: NavMemberGates
483537
// Stable across queries: cmdk filters nothing here
484538
// (`shouldFilter` false), it only selects by the value.
485539
value={`record ${hit.display} ${hit.objectLabel} ${hit.objectName} ${hit.recordId}`}
486-
onSelect={() => leaveTo(`${baseUrl}/${hit.objectName}/record/${hit.recordId}`)}
540+
onSelect={() => leaveTo(`${recordHitBaseUrl(hit.objectName)}/${hit.objectName}/record/${hit.recordId}`)}
487541
>
488542
<Icon className="mr-2 h-4 w-4" />
489543
<span className="truncate">{hit.display}</span>

0 commit comments

Comments
 (0)