diff --git a/.changeset/13097-landing-waits-reads.md b/.changeset/13097-landing-waits-reads.md new file mode 100644 index 0000000000..a95f4e7dce --- /dev/null +++ b/.changeset/13097-landing-waits-reads.md @@ -0,0 +1,14 @@ +--- +'@object-ui/app-shell': patch +--- + +An app's landing waits for the API reads of its navigation entries (objectui#13097). + +A navigation entry that declares an API `dataSource` is hidden while its read is pending (objectui#13029). The app root picks its landing from the entries the member is shown, so before this fix a pending entry was already gone when it chose. Two things went wrong: + +- An app whose entries were all bound showed "Nothing in this app is available to you yet" until the reads answered. +- An app whose first entry was bound and whose second was plain navigated to the second entry on first render. The bound entry could never be the landing. + +Now, while a pending read ahead of the first decided entry could still change the landing, the app root shows a skeleton. It does not navigate and does not show the no-access state. When the reads answer, it lands on the first entry the member is shown. A failed read hides its entry, and the landing moves on to the next one. An app with no bound entry lands on its first render, as before. No new requests are made: the landing reuses the reads the sidebar already makes. + +Nothing is added to the package entry. diff --git a/packages/app-shell/src/console/AppContent.tsx b/packages/app-shell/src/console/AppContent.tsx index 787a250edc..8d92f3e211 100644 --- a/packages/app-shell/src/console/AppContent.tsx +++ b/packages/app-shell/src/console/AppContent.tsx @@ -56,7 +56,11 @@ import { RouteFader } from '../chrome/RouteFader.js'; import { NavigationSyncEffect } from '../hooks/useNavigationSync.js'; import { AppNoAccessEmptyState } from './AppNoAccessEmptyState.js'; import { appServesNoNavigation } from './appServesNoNavigation.js'; -import { useNavMemberGates, navEntriesShownToMember } from '../hooks/useNavMemberGates.js'; +import { + useNavMemberGates, + navEntriesShownToMember, + navEntriesShownOncePendingReadsShow, +} from '../hooks/useNavMemberGates.js'; import { resolveSetupAppPath } from './ConsoleShell.js'; import { APP_ROUTE_METADATA_READS, collectRoutePaths, metadataReadsForAppRoute, type LazyMetadataType } from './appRouteMetadataReads.js'; import { readAppObjectScope } from './appObjectScope.js'; @@ -1604,6 +1608,18 @@ function resolveLandingRoute(navigation: NavigationItem[], ctx?: NavTemplateCont * `service_agent` landed on a dashboard built on an object they may not read, * with no active sidebar entry. * + * An entry whose API `dataSource` read is still pending is hidden by those + * gates (objectui#13029), but it is not decided yet (objectui#13097). So the + * landing is asked twice: over the trees as the member is shown them now, and + * over the trees as they would be shown if every pending entry were shown once + * it answers. When the two disagree, on the landing route or on whether the + * app shows this member anything, a pending read ahead of the first decided + * entry could still change the answer: the root holds on a skeleton, neither + * navigating nor saying "nothing here", and lands when the reads answer. A + * failed read hides its entry, and the landing moves on. An app with no bound + * entry, or whose first decided entry comes before every pending one, gets the + * same answer twice and lands on first render, as before. + * * It is a component, not an expression in `AppContent`'s render, because the * `visible` evaluator is the `ExpressionProvider` mounted INSIDE that render; * read from `AppContent` itself it would be an outer one, not the sidebar's. @@ -1620,6 +1636,28 @@ function AppRootLanding({ const gates = useNavMemberGates(); const navigation = navEntriesShownToMember(app?.navigation || [], gates); const landing = resolveLandingRoute(navigation, ctx); + const areas = (app?.areas || []) as Array<{ navigation?: NavigationItem[] } | null | undefined>; + const shownApp = { + navigation, + areas: areas.map((area) => ({ navigation: navEntriesShownToMember(area?.navigation || [], gates) })), + }; + // objectui#13097 — the same questions, as if every pending read showed its + // entry. A different answer means a pending read is still deciding it. + const undecided = navEntriesShownOncePendingReadsShow(app?.navigation || [], gates); + const undecidedApp = { + navigation: undecided, + areas: areas.map((area) => ({ navigation: navEntriesShownOncePendingReadsShow(area?.navigation || [], gates) })), + }; + if ( + resolveLandingRoute(undecided, ctx) !== landing || + (!landing && appServesNoNavigation(shownApp) !== appServesNoNavigation(undecidedApp)) + ) { + return ( +
+ +
+ ); + } if (landing) return ; // objectui#12079 — an app that shows this member NO navigation (every entry // gated away from them, or none served at all) is an app they have no access @@ -1629,12 +1667,6 @@ function AppRootLanding({ // asked of the trees as this member is shown them (objectui#12426), so a // member who can see no entry gets this answer, never a page the sidebar // hides. - const shownApp = { - navigation, - areas: (app?.areas || []).map((area: any) => ({ - navigation: navEntriesShownToMember(area?.navigation || [], gates), - })), - }; if (appServesNoNavigation(shownApp)) { return ; } diff --git a/packages/app-shell/src/console/__tests__/AppContent.landingWaitsReads-13097.test.tsx b/packages/app-shell/src/console/__tests__/AppContent.landingWaitsReads-13097.test.tsx new file mode 100644 index 0000000000..50e2f77ae8 --- /dev/null +++ b/packages/app-shell/src/console/__tests__/AppContent.landingWaitsReads-13097.test.tsx @@ -0,0 +1,309 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#13097 — the app root's landing waits on the API reads of the nav + * entries it would choose between. + * + * objectui#13029 hides a navigation entry whose `dataSource` read is pending or + * failed. `AppRootLanding` picks the first entry the member is shown + * (objectui#12426), so before this card a pending entry was already gone when + * it chose: an app whose entries are all bound said "no access" until the reads + * answered, and an app whose first entry is bound landed (with `replace`) on + * the next plain entry before the read could show the bound one. + * + * Direction (A), pinned here through the real `AppContent`, the real + * `ExpressionProvider` it mounts, the real `MePermissionsProvider` above it and + * a stubbed `apiFetch` on `SchemaRendererContext` that each case holds pending + * until it answers: + * (a) all bound, pending: no `AppNoAccessEmptyState` and no navigation; once + * the reads answer and show the entries, it lands on the first one; + * (b) first bound and pending, second plain: no navigation while pending; + * `ready` and shown lands on the bound entry, a failed read on the plain; + * (c) all bound, every read failed: the no-access state, as before; + * (d) CONTROL: no bound entry lands on its first render: no read, and the + * hold never draws. + * + * Each case uses its own URL: `readElementApi` shares a pending request per URL + * for the life of the module. + * + * NOTE ON SCOPE: routing only. `ConsoleLayout` and the lazily imported pages + * are stubbed, as in `AppContent.landingMemberGates-12426.test.tsx`. + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import React from 'react'; +import { render, screen, waitFor, act } from '@testing-library/react'; +import { MemoryRouter, Routes, Route, useLocation } from 'react-router-dom'; +import { MePermissionsProvider, type MePermissionsResponse } from '@object-ui/permissions'; +import { SchemaRendererContext, type ApiFetch } from '@object-ui/react'; + +// AGENTS.md §测试纪律 — the lazy page modules are stubbed so no unbounded +// `import()` races a bounded `findBy` window. +vi.mock('../../views/metadata-admin', () => ({ + MetadataDirectoryPage: () =>
, + MetadataResourceListPage: () =>
, + MetadataResourceEditPage: () =>
, + MetadataResourceHistoryPage: () =>
, + MetadataDiagnosticsPage: () =>
, +})); + +vi.mock('@object-ui/plugin-designer', async (importOriginal) => ({ + ...(await importOriginal()), + CreateAppPage: () =>
, + EditAppPage: () =>
, + DashboardDesignPage: () =>
, +})); + +vi.mock('../../layout/ConsoleLayout', () => ({ + ConsoleLayout: ({ activeAppName, children }: { activeAppName?: string; children?: React.ReactNode }) => ( +
+ {children} +
+ ), +})); +vi.mock('../../chrome/CommandPalette', () => ({ CommandPalette: () => null })); +vi.mock('../../chrome/KeyboardShortcutsDialog', () => ({ KeyboardShortcutsDialog: () => null })); +vi.mock('../../chrome/OnboardingWalkthrough', () => ({ OnboardingWalkthrough: () => null })); +vi.mock('../../views/ObjectView', () => ({ + ObjectView: () =>
, +})); + +vi.mock('@object-ui/i18n', async (importOriginal) => ({ + ...(await importOriginal>()), + useObjectTranslation: () => ({ + t: (key: string, options?: Record) => + String(options?.defaultValue ?? key).replace(/\{\{(\w+)\}\}/g, (_m, name: string) => String(options?.[name] ?? '')), + }), + useObjectLabel: () => ({ + objectLabel: ({ label }: { label?: string }) => label, + objectPluralLabel: ({ label }: { label?: string }) => label, + appLabel: ({ name, label }: { name: string; label?: string }) => label ?? name, + }), +})); + +// One auth value for every render, as the real provider memoises it: +// `AppContent` re-reads the auth config whenever `getAuthConfig`'s identity +// changes, so a fresh function per render re-renders it without end once +// anything re-renders it. +const authValue = { + user: { id: 'u_1', name: 'Member', email: 'member@example.com', role: 'member' }, + getAuthConfig: async () => ({ features: {} }), + activeOrganization: { id: 'org_1', name: 'Org' }, +}; +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => authValue, + useWorkspaceAdminStatus: () => ({ isAdmin: false, isResolved: true }), +})); + +const dataSourceStub = { + onConnectionStateChange: () => () => {}, + getConnectionState: () => 'connected', +}; +vi.mock('../../providers/AdapterProvider', async (importOriginal) => ({ + ...(await importOriginal>()), + useAdapter: () => dataSourceStub, +})); + +/** The app list as the server hands it to this session. */ +let metadataApps: unknown[] = []; +vi.mock('../../providers/MetadataProvider', async (importOriginal) => ({ + ...(await importOriginal>()), + useMetadata: () => ({ + apps: metadataApps, + objects: [], + loading: false, + ensureType: undefined, + error: null, + refresh: vi.fn(async () => {}), + }), +})); + +const actionRunnerStub = { registerHandler: vi.fn(), getContext: () => ({}) }; +vi.mock('@object-ui/react', async (importOriginal) => ({ + ...(await importOriginal>()), + useActionRunner: () => ({ execute: vi.fn(), runner: actionRunnerStub }), + useGlobalUndo: () => {}, + useMutationInvalidationBridge: () => {}, +})); + +import { AppContent } from '../AppContent'; + +function LocationProbe() { + const location = useLocation(); + return
{location.pathname}
; +} + +const EVERYTHING_READABLE = { + authenticated: true, + userId: 'u_1', + tenantId: null, + roles: [], + permissionSets: [], + objects: { '*': { allowRead: true } }, + fields: {}, +} as MePermissionsResponse; + +let urlSeq = 0; +/** A URL no earlier case asked for. */ +function freshUrl(): string { + urlSeq += 1; + return `/api/v1/landing-probe/${urlSeq}`; +} + +/** An `apiFetch` whose answers the test releases, one per URL. */ +function heldApiFetch() { + const releases = new Map void>(); + const apiFetch = vi.fn( + (url) => + new Promise((resolve) => { + releases.set(String(url), (status, body) => + resolve({ ok: status >= 200 && status < 300, status, json: async () => body } as unknown as Response), + ); + }), + ); + /** Answer `url`, then let the settled read reach the state it sets. */ + const answer = async (url: string, status: number, body?: unknown) => { + await waitFor(() => expect(releases.has(url)).toBe(true)); + await act(async () => { + releases.get(url)!(status, body); + await new Promise((resolve) => setTimeout(resolve, 20)); + }); + }; + return { apiFetch, answer }; +} + +/** An object entry bound to `url`: shown when the bound `total` is 1. */ +function boundEntry(id: string, objectName: string, url: string) { + return { + id, + type: 'object', + objectName, + label: objectName, + visible: 'data.total == 1', + dataSource: { provider: 'api', read: { url }, path: 'summary' }, + }; +} + +const plainEntry = (id: string, objectName: string) => ({ id, type: 'object', objectName, label: objectName }); + +function renderConsoleAt(initialUrl: string, apiFetch: ApiFetch) { + return render( + + + + + + } /> + + + + , + ); +} + +const pathname = () => screen.getByTestId('pathname').textContent; +const SHOWN = { summary: { total: 1 } }; + +describe('objectui#13097 — the app landing waits on its nav entries\' API reads', () => { + beforeEach(() => { + metadataApps = []; + }); + + it('(a) all bound, pending: no no-access state and no navigation; answered and shown, it lands on the first', async () => { + const [envUrl, siteUrl] = [freshUrl(), freshUrl()]; + metadataApps = [ + { name: 'crm', label: 'CRM', navigation: [boundEntry('nav_env', 'crm_env', envUrl), boundEntry('nav_site', 'crm_site', siteUrl)] }, + ]; + const { apiFetch, answer } = heldApiFetch(); + renderConsoleAt('/apps/crm', apiFetch); + + await waitFor(() => expect(apiFetch).toHaveBeenCalledTimes(2)); + expect(screen.getByTestId('app-landing-pending')).toBeInTheDocument(); + expect(screen.queryByTestId('app-no-access-empty-state')).not.toBeInTheDocument(); + expect(pathname()).toBe('/apps/crm'); + + await answer(siteUrl, 200, SHOWN); + // The second entry is shown now, but the first is still pending ahead of it. + expect(screen.queryByTestId('app-no-access-empty-state')).not.toBeInTheDocument(); + expect(pathname()).toBe('/apps/crm'); + + await answer(envUrl, 200, SHOWN); + expect(await screen.findByTestId('object-view')).toBeInTheDocument(); + expect(pathname()).toBe('/apps/crm/crm_env'); + }); + + it('(b) first bound and pending, second plain: no navigation while pending; ready and shown lands on the bound entry', async () => { + const envUrl = freshUrl(); + metadataApps = [ + { name: 'crm', label: 'CRM', navigation: [boundEntry('nav_env', 'crm_env', envUrl), plainEntry('nav_cases', 'crm_case')] }, + ]; + const { apiFetch, answer } = heldApiFetch(); + renderConsoleAt('/apps/crm', apiFetch); + + await waitFor(() => expect(apiFetch).toHaveBeenCalledWith(envUrl, expect.objectContaining({ method: 'GET' }))); + expect(screen.getByTestId('app-landing-pending')).toBeInTheDocument(); + expect(pathname()).toBe('/apps/crm'); + + await answer(envUrl, 200, SHOWN); + expect(await screen.findByTestId('object-view')).toBeInTheDocument(); + expect(pathname()).toBe('/apps/crm/crm_env'); + }); + + it('(b) first bound and pending, second plain: a failed read lands on the plain entry', async () => { + const envUrl = freshUrl(); + metadataApps = [ + { name: 'crm', label: 'CRM', navigation: [boundEntry('nav_env', 'crm_env', envUrl), plainEntry('nav_cases', 'crm_case')] }, + ]; + const { apiFetch, answer } = heldApiFetch(); + renderConsoleAt('/apps/crm', apiFetch); + + await waitFor(() => expect(apiFetch).toHaveBeenCalled()); + expect(pathname()).toBe('/apps/crm'); + + await answer(envUrl, 500); + expect(await screen.findByTestId('object-view')).toBeInTheDocument(); + expect(pathname()).toBe('/apps/crm/crm_case'); + }); + + it('(c) all bound, every read failed: the no-access state, as before', async () => { + const [envUrl, siteUrl] = [freshUrl(), freshUrl()]; + metadataApps = [ + { name: 'crm', label: 'CRM', navigation: [boundEntry('nav_env', 'crm_env', envUrl), boundEntry('nav_site', 'crm_site', siteUrl)] }, + ]; + const { apiFetch, answer } = heldApiFetch(); + renderConsoleAt('/apps/crm', apiFetch); + + await answer(envUrl, 500); + await answer(siteUrl, 500); + expect(await screen.findByTestId('app-no-access-empty-state')).toBeInTheDocument(); + expect(pathname()).toBe('/apps/crm'); + expect(screen.queryByTestId('app-landing-pending')).not.toBeInTheDocument(); + }); + + it('(d) CONTROL: an app with no bound entry lands on its first render, with no read and no hold', async () => { + metadataApps = [{ name: 'crm', label: 'CRM', navigation: [plainEntry('nav_cases', 'crm_case'), plainEntry('nav_accounts', 'crm_account')] }]; + const { apiFetch } = heldApiFetch(); + // Every node the document ever receives, so a hold that came and went is + // seen as well as one that stayed. + let held = false; + const observer = new MutationObserver((records) => { + for (const record of records) { + record.addedNodes.forEach((node) => { + if (node instanceof Element && (node.matches('[data-testid="app-landing-pending"]') || node.querySelector('[data-testid="app-landing-pending"]'))) { + held = true; + } + }); + } + }); + observer.observe(document.body, { childList: true, subtree: true }); + renderConsoleAt('/apps/crm', apiFetch); + + expect(await screen.findByTestId('object-view')).toBeInTheDocument(); + observer.disconnect(); + expect(pathname()).toBe('/apps/crm/crm_case'); + expect(held).toBe(false); + expect(apiFetch).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/app-shell/src/hooks/useNavMemberGates.ts b/packages/app-shell/src/hooks/useNavMemberGates.ts index 693474f2af..848802460c 100644 --- a/packages/app-shell/src/hooks/useNavMemberGates.ts +++ b/packages/app-shell/src/hooks/useNavMemberGates.ts @@ -235,13 +235,26 @@ export function navEntryVisible( export function useNavVisibility( app: unknown, ): (expression: NavigationItem['visible'], entry?: NavigationItem) => boolean { + return useNavVisibilityAndReads(app).evaluate; +} + +/** + * {@link useNavVisibility}'s evaluator together with the read state it is built + * on, so a consumer that must tell "hidden" from "not answered yet" asks the + * same reads instead of issuing its own (objectui#13097). + */ +function useNavVisibilityAndReads(app: unknown): { + evaluate: (expression: NavigationItem['visible'], entry?: NavigationItem) => boolean; + readEntry: (entry: NavigationItem) => NavEntryDataRead; +} { const { evaluator } = useExpressionContext(); const readEntry = useNavEntryData(app); - return useCallback( + const evaluate = useCallback( (expression: NavigationItem['visible'], entry?: NavigationItem) => navEntryVisible(expression, entry, evaluator, readEntry), [evaluator, readEntry], ); + return { evaluate, readEntry }; } /** The member's gates, as the sidebar applies them. */ @@ -256,14 +269,24 @@ export interface NavMemberGates { can: ObjectReadCheck; } +/** + * The member's gates, with where each entry's API binding stands + * (objectui#13097): the read state the `visible` evaluator is built on, so the + * app root's landing can tell an entry that is hidden from one whose read has + * not answered yet, without a read of its own. + */ +export interface NavMemberGatesWithReads extends NavMemberGates { + readEntry: (entry: NavigationItem) => NavEntryDataRead; +} + /** * The member's gates, built from the providers the console mounts: the * expression evaluator, `usePermissions()`, and the registered object list. */ -export function useNavMemberGates(): NavMemberGates { +export function useNavMemberGates(): NavMemberGatesWithReads { const { app } = useExpressionContext(); // objectui#13029 — the entries of the app this frame renders may bind data. - const evalVis = useNavVisibility(app); + const { evaluate: evalVis, readEntry } = useNavVisibilityAndReads(app); // Permission check for nav `requiredPermissions` entries. // @@ -346,7 +369,7 @@ export function useNavMemberGates(): NavMemberGates { [registeredObjectNames, missingObjectNames, readObjectScope], ); - return { evaluateVisibility: evalVis, checkPermission: checkPerm, checkCapability: checkCap, can }; + return { evaluateVisibility: evalVis, checkPermission: checkPerm, checkCapability: checkCap, can, readEntry }; } /** @@ -376,3 +399,26 @@ export function navEntriesShownToMember(items: NavigationItem[], gates: NavMembe ); return shown(withoutUnreadableObjectEntries(items, gates.can)); } + +/** + * `items` as this member would be shown them if every entry whose API read is + * still pending were shown once it answers (objectui#13097): the same gates as + * {@link navEntriesShownToMember}, except that a pending entry's `visible` is + * not decided yet and keeps it. Every other gate (the object-read prune, + * `requiredPermissions`, the runtime-capability gates, a plain or answered + * `visible`) applies as it does there. + * + * Comparing the two answers tells the app root's landing whether a pending read + * could still change what it chooses: if it cannot, it lands now. + */ +export function navEntriesShownOncePendingReadsShow( + items: NavigationItem[], + gates: NavMemberGatesWithReads, +): NavigationItem[] { + return navEntriesShownToMember(items, { + ...gates, + evaluateVisibility: (expression, entry) => + (entry !== undefined && gates.readEntry(entry).status === 'pending') || + gates.evaluateVisibility(expression, entry), + }); +}