Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .changeset/29750-native-push-route-hosted.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
'@object-ui/app-shell': patch
'@object-ui/console': patch
---

fix(console): a push route from the mobile shell opens the record, hosted under an app the way the inbox hosts it

Clause-②: no
Metadata-protocol: none

A push delivery carries the notification's link as `route`: app-relative (`/opportunity/123`) or already a Console route (`/apps/crm/opportunity/123`). The Console's listener for the mobile shell's navigate event moved its router to that route as written, so `/opportunity/123` matched no route and the person landed on Home instead of the record.

What changes:

- The listener resolves the route with the same resolver the inbox bell and Home use for that notification. An app-relative route is hosted under the app the person is in or last had open; an `/apps/...` route passes through unchanged.
- With no app open yet (a tap that starts the app), the route goes to the router as given, and the Console's unmatched-route page hosts it under the default app once the app list has loaded: the app the person last had open while it is still active, else the first active app. The same page handles an app-relative path that arrives as the page's first URL, which the shell sends when it re-creates its web view at a held route or when no listener answered.
- A path whose first segment names a Console route served outside an app (`/home`, `/dev`, `/studio`, ...) still redirects to `/`, as before. A path such as `/foo/bar` that names no Console route now opens `/apps/APP/foo/bar` (APP being that default app) instead of Home.
- `@object-ui/app-shell` exports `resolveNotificationTarget`, its `NotificationTarget` type and `CONSOLE_ROOT_SEGMENTS` from the package root.

What to do: nothing.
36 changes: 31 additions & 5 deletions ui/apps/console/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
* with extra `<Route>` children.
*/

import { lazy, Suspense, useEffect, useMemo } from 'react';
import { BrowserRouter, Routes, Route, Navigate, Outlet, useLocation, useNavigate } from 'react-router-dom';
import { lazy, Suspense, useCallback, useEffect, useMemo } from 'react';
import { BrowserRouter, Routes, Route, Navigate, Outlet, matchPath, useLocation, useNavigate } from 'react-router-dom';
import { useNativeHostRouting } from '@object-ui/mobile';
import { AuthProvider, useAuth, createAuthenticatedFetch } from '@object-ui/auth';
import { UploadProvider, createObjectStackUploadAdapter } from '@object-ui/providers';
Expand All @@ -37,7 +37,7 @@ import {
DefaultInvitationsPage,
DefaultSettingsPage,
DefaultAcceptInvitationPage,
RedirectWithSplash,
useMetadata,
} from '@object-ui/app-shell';

import { AppContent, ConsolePermissionsRequest, ConsolePermissionsScope } from './AppContent';
Expand All @@ -47,6 +47,8 @@ import { RootLandingRedirect } from './components/RootLandingRedirect';
import { ProtectedRoute } from './components/ProtectedRoute';
import { studioRoutes } from './components/StudioRoute';
import { SetupRoute } from './components/SetupRoute';
import { UnmatchedRoute } from './components/UnmatchedRoute';
import { nativeShellRouteTarget } from './nativeShellRoute';
import { FormPage } from './components/FormPage';
import { InternalFormRoute } from './components/InternalFormRoute';
import { MetadataHmrReloader } from './components/MetadataHmrReloader';
Expand Down Expand Up @@ -148,13 +150,33 @@ const BASENAME = resolveBasename();
* the page. `data-os-native-shell` on the root element names the platform, so
* a stylesheet can tell shell mode apart. Without the bridge this renders
* nothing and installs nothing.
*
* The shell's route is a push route, the notification's `actionUrl`, and is
* never navigated verbatim (objectstack#29750): it is hosted the way the inbox
* hosts that link (`nativeShellRouteTarget`). Inside an app it goes under the
* app the URL names. Anywhere else (Home, a cold start) the route goes to the
* router as given, and the catch-all's `UnmatchedRoute` hosts it once the app
* list is loaded: under the app the person last had open while it is still
* active, else the first active app. The shell's published `currentAppName` is
* left to that guard because it is an app NAME, and only the app list turns a
* name into the address its route uses.
*/
function NativeShellRouting() {
const location = useLocation();
const navigate = useNavigate();
// Outside every `MetadataProvider`, so the empty list; see `nativeShellRouteTarget`.
const { apps } = useMetadata();
const hostHint = matchPath('/apps/:appName/*', location.pathname)?.params.appName;
const navigateShellRoute = useCallback(
(route: string) => {
const path = nativeShellRouteTarget(route, hostHint, apps);
if (path !== null) navigate(path);
},
[apps, hostHint, navigate],
);
const host = useNativeHostRouting({
route: `${location.pathname}${location.search}${location.hash}`,
navigate,
navigate: navigateShellRoute,
});
const platform = host?.platform;
useEffect(() => {
Expand Down Expand Up @@ -486,7 +508,11 @@ export function App() {
* `index` redirect above is deliberately NOT changed — it fires
* under an already-painted organization layout, where covering the
* screen with a splash would be the regression. */}
<Route path="*" element={<RedirectWithSplash to="/" replace />} />
{/* objectstack#29750 — an app-relative path (a push route such as
* `/opportunity/123`, arriving as the first URL or handed over by
* `NativeShellRouting`) is hosted under an app the way the inbox
* hosts it; a stale Console link still redirects to `/`. */}
<Route path="*" element={<UnmatchedRoute />} />
</Routes>
</ConsoleShell>
</UploadProvider>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,11 @@ vi.mock('@object-ui/auth', async (importOriginal) => ({

// ── Everything else App.tsx imports but this card does not touch ─────────
vi.mock('../components/ProtectedRoute', () => ({ ProtectedRoute: passthrough }));
vi.mock('../components/RootLandingRedirect', () => ({ RootLandingRedirect: stub('root-landing') }));
// objectstack#29750: the catch-all reads `isAppListConclusive` from here.
vi.mock('../components/RootLandingRedirect', async (importOriginal) => ({
...(await importOriginal<Record<string, unknown>>()),
RootLandingRedirect: stub('root-landing'),
}));
vi.mock('../components/SetupRoute', () => ({ SetupRoute: stub('setup-route') }));
vi.mock('../components/MetadataHmrReloader', () => ({ MetadataHmrReloader: () => null }));
vi.mock('../AppContent', () => ({
Expand Down
178 changes: 174 additions & 4 deletions ui/apps/console/src/__tests__/App.nativeShellMode-29039.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,42 @@ import {
} from '../../../../packages/mobile/src/nativeHost';
import { resetNativeHostWatchForTests } from '../../../../packages/mobile/src/useNativeHost';

const { passthrough, stub } = vi.hoisted(() => {
const { passthrough, stub, shellState } = vi.hoisted(() => {
const passthrough = ({ children }: { children?: ReactNode }) => <>{children}</>;
const stub = (testid: string) => () => <div data-testid={testid} />;
return { passthrough, stub };
// objectstack#29750: what the shell's navigation context and the metadata
// provider answer. The defaults are what this file's other cases read
// without either provider: no current app, an empty app list, not loading.
const shellState = {
currentAppName: undefined as string | undefined,
apps: [] as Array<Record<string, unknown>>,
loading: false,
appStatus: undefined as 'idle' | 'loading' | 'ready' | 'error' | undefined,
};
return { passthrough, stub, shellState };
});

vi.mock('@object-ui/app-shell', async (importOriginal) => ({
...(await importOriginal<Record<string, unknown>>()),
ConsoleShell: passthrough,
ConsoleToaster: () => null,
LoadingScreen: stub('loading-screen'),
RedirectWithSplash: stub('redirect-with-splash'),
// Records where it would go, so the catch-all's answer is readable.
RedirectWithSplash: ({ to }: { to: string }) => <div data-testid="redirect-with-splash" data-to={to} />,
LoadingFallback: stub('loading-fallback'),
useNavigationContext: () => ({
context: 'app',
setContext: () => {},
currentAppName: shellState.currentAppName,
setCurrentAppName: () => {},
recordTitle: undefined,
setRecordTitle: () => {},
}),
useMetadata: () => ({
apps: shellState.apps,
loading: shellState.loading,
getTypeStatus: (type: string) => (type === 'app' ? shellState.appStatus : undefined),
}),
RequireAiSurface: passthrough,
SystemRedirect: () => null,
DefaultHomeLayout: passthrough,
Expand Down Expand Up @@ -76,7 +100,11 @@ vi.mock('../AppContent', () => ({
ConsolePermissionsRequest: () => null,
}));
vi.mock('../components/ProtectedRoute', () => ({ ProtectedRoute: passthrough }));
vi.mock('../components/RootLandingRedirect', () => ({ RootLandingRedirect: stub('root-landing') }));
// objectstack#29750: the catch-all reads `isAppListConclusive` from here.
vi.mock('../components/RootLandingRedirect', async (importOriginal) => ({
...(await importOriginal<Record<string, unknown>>()),
RootLandingRedirect: stub('root-landing'),
}));
vi.mock('../components/SetupRoute', () => ({ SetupRoute: stub('setup-route') }));
vi.mock('../components/FormPage', () => ({ FormPage: stub('form-page') }));
vi.mock('../components/InternalFormRoute', () => ({ InternalFormRoute: stub('internal-form-route') }));
Expand Down Expand Up @@ -134,6 +162,10 @@ function dispatchNavigate(route: unknown): CustomEvent {
beforeEach(() => {
resetNativeHostEventsForTests();
resetNativeHostWatchForTests();
shellState.currentAppName = undefined;
shellState.apps = [];
shellState.loading = false;
shellState.appStatus = undefined;
window.history.pushState({}, '', '/login');
});

Expand Down Expand Up @@ -188,3 +220,141 @@ describe('control: the Console in a browser behaves as today', () => {
expect(window.location.pathname).toBe('/login');
});
});

/**
* objectstack#29750 — the shell's route is a push route, the notification's
* `actionUrl`, and the push contract (`packages/spec/src/api/push-delivery.zod.ts`)
* says it is resolved "as the inbox does, never verbatim": app-relative paths
* are hosted under `/apps/APP/`, an `/apps/…` route passes through. Navigated
* verbatim, `/opportunity/123` matched no route and the catch-all sent the
* person to Home.
*/
describe('a push route from the shell is hosted the way the inbox hosts it', () => {
it('an app-relative route is hosted under the app in the URL', async () => {
installBridge();
window.history.pushState({}, '', '/apps/crm/opportunity');
render(<App />);
await screen.findByTestId('app-content');
const event = dispatchNavigate('/opportunity/123?tab=activity#notes');
expect(event.defaultPrevented).toBe(true);
expect(`${window.location.pathname}${window.location.search}${window.location.hash}`)
.toBe('/apps/crm/opportunity/123?tab=activity#notes');
});

it('away from an app, the catch-all hosts it under the app the person last had open', async () => {
// `currentAppName` is an app NAME; the catch-all turns it into the app's
// route segment with the app list, which the listener does not have.
installBridge();
shellState.currentAppName = 'sales';
shellState.apps = [{ name: 'crm', active: true }, { name: 'sales', active: true, _packageId: 'com.acme.sales' }];
shellState.appStatus = 'ready';
window.history.pushState({}, '', '/home');
render(<App />);
await screen.findByTestId('home-page');
const event = dispatchNavigate('/opportunity/123');
expect(event.defaultPrevented).toBe(true);
expect(window.location.pathname).toBe('/opportunity/123');
expect((await screen.findByTestId('redirect-with-splash')).getAttribute('data-to'))
.toBe('/apps/com.acme.sales/opportunity/123');
});

it('a Console route under /apps passes through unchanged', async () => {
installBridge();
window.history.pushState({}, '', '/apps/crm/opportunity');
render(<App />);
await screen.findByTestId('app-content');
const event = dispatchNavigate('/apps/x/y/1');
expect(event.defaultPrevented).toBe(true);
expect(window.location.pathname).toBe('/apps/x/y/1');
});

it('with no app open, the catch-all hosts the handed-over route under the default app', async () => {
installBridge();
shellState.apps = [{ name: 'crm', active: true }];
shellState.appStatus = 'ready';
render(<App />);
await screen.findByTestId('login-page');
const event = dispatchNavigate('/opportunity/123');
expect(event.defaultPrevented).toBe(true);
expect(window.location.pathname).toBe('/opportunity/123');
expect((await screen.findByTestId('redirect-with-splash')).getAttribute('data-to'))
.toBe('/apps/crm/opportunity/123');
});
});

/**
* objectstack#29750 — the same push route can arrive as the page's FIRST URL:
* the mobile shell starts a re-created WebView at a held route, and falls back
* to `location.assign` of it when no navigate listener answered. The
* catch-all hosts an app-relative path like the inbox; a stale Console link
* keeps the old redirect to `/`.
*/
describe('the catch-all hosts an app-relative first URL under an app', () => {
function catchAllTarget(): Promise<string | null> {
return screen.findByTestId('redirect-with-splash').then((el) => el.getAttribute('data-to'));
}

it('hosts it under the first active app, query and hash kept', async () => {
shellState.apps = [{ name: 'gone', active: false }, { name: 'crm', active: true }];
shellState.appStatus = 'ready';
window.history.pushState({}, '', '/opportunity/123?tab=activity#notes');
render(<App />);
expect(await catchAllTarget()).toBe('/apps/crm/opportunity/123?tab=activity#notes');
});

it('hosts it under the app the person last had open while that app is still active', async () => {
shellState.apps = [{ name: 'crm', active: true }, { name: 'sales', active: true }];
shellState.appStatus = 'ready';
shellState.currentAppName = 'sales';
window.history.pushState({}, '', '/opportunity/123');
render(<App />);
expect(await catchAllTarget()).toBe('/apps/sales/opportunity/123');
});

it('waits for the app list before it answers', async () => {
shellState.loading = true;
window.history.pushState({}, '', '/opportunity/123');
render(<App />);
expect(await screen.findByTestId('loading-fallback')).toBeInTheDocument();
expect(screen.queryByTestId('redirect-with-splash')).toBeNull();
});

it('waits while a session-cached app list is still being re-read', async () => {
// Measured live: the provider stops `loading` on the cached list while
// the app type's own status still reads `loading`.
shellState.apps = [{ name: 'crm', active: true }];
shellState.appStatus = 'loading';
window.history.pushState({}, '', '/opportunity/123');
render(<App />);
expect(await screen.findByTestId('loading-fallback')).toBeInTheDocument();
expect(screen.queryByTestId('redirect-with-splash')).toBeNull();
});

it('an app list that failed to load is no answer: the old redirect to / runs', async () => {
shellState.appStatus = 'error';
window.history.pushState({}, '', '/opportunity/123');
render(<App />);
expect(await catchAllTarget()).toBe('/');
});

it('control: a stale Console link still redirects to /', async () => {
shellState.apps = [{ name: 'crm', active: true }];
shellState.appStatus = 'ready';
for (const path of ['/dev/nope', '/home/nope', '/apps']) {
window.history.pushState({}, '', path);
render(<App />);
expect(await catchAllTarget()).toBe('/');
cleanup();
}
});

it('control: a real top-level Console route is never rewritten', async () => {
shellState.apps = [{ name: 'crm', active: true }];
shellState.appStatus = 'ready';
window.history.pushState({}, '', '/home');
render(<App />);
expect(await screen.findByTestId('home-page')).toBeInTheDocument();
expect(screen.queryByTestId('redirect-with-splash')).toBeNull();
expect(window.location.pathname).toBe('/home');
});
});
61 changes: 61 additions & 0 deletions ui/apps/console/src/__tests__/nativeShellRoute-29750.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectstack#29750 — where a route the native shell hands the Console lands,
* at the unit level. The wiring into the real router is pinned in
* `App.nativeShellMode-29039.test.tsx`; this file holds the two answers that
* no listener input can reach (the resolver's `null` and `external`), because
* `isInAppRoute` refuses a blank route and any scheme or `//host` before the
* Console ever sees it, and the unmatched-path predicate's table.
*/

import { describe, expect, it } from 'vitest';
import { isAppRelativeUnmatchedPath, nativeShellRouteTarget } from '../nativeShellRoute';

describe('nativeShellRouteTarget', () => {
it('hosts an app-relative route under the hinted app', () => {
expect(nativeShellRouteTarget('/opportunity/123', 'crm', [])).toBe('/apps/crm/opportunity/123');
});

it('passes a Console route under /apps through', () => {
expect(nativeShellRouteTarget('/apps/x/y/1', 'crm', [])).toBe('/apps/x/y/1');
});

it('re-checks the hint against an app list in reach, as the inbox does', () => {
const apps = [{ name: 'crm', active: true }, { name: 'old', active: false }];
expect(nativeShellRouteTarget('/opportunity/123', 'old', apps)).toBe('/apps/crm/opportunity/123');
});

it('with no hint, hands the route to the router unchanged', () => {
expect(nativeShellRouteTarget('/opportunity/123', undefined, [])).toBe('/opportunity/123');
});

it('a blank route resolves to nothing: no navigation', () => {
expect(nativeShellRouteTarget('', 'crm', [])).toBeNull();
expect(nativeShellRouteTarget(' ', 'crm', [])).toBeNull();
});

it('an off-console route resolves to nothing for the router: no navigation', () => {
expect(nativeShellRouteTarget('https://evil.example/x', 'crm', [])).toBeNull();
expect(nativeShellRouteTarget('//evil.example/x', 'crm', [])).toBeNull();
});
});

describe('isAppRelativeUnmatchedPath', () => {
it('a path whose first segment names no Console route is app-relative', () => {
expect(isAppRelativeUnmatchedPath('/opportunity/123')).toBe(true);
expect(isAppRelativeUnmatchedPath('/showcase_task')).toBe(true);
});

it('a Console route served outside an app, the landing and /apps are not', () => {
for (const path of ['/', '/home/x', '/dev/nope', '/studio/x/y', '/forms/x/y', '/apps', '/apps/']) {
expect(isAppRelativeUnmatchedPath(path), path).toBe(false);
}
});
});
Loading
Loading