diff --git a/.changeset/28036-record-form-app-name-address.md b/.changeset/28036-record-form-app-name-address.md new file mode 100644 index 00000000000..fd9b7be5006 --- /dev/null +++ b/.changeset/28036-record-form-app-name-address.md @@ -0,0 +1,10 @@ +--- +'@object-ui/app-shell': patch +--- + +fix(console): a full-page record form opened on the app's name address stays in the app after Save + +Clause-②: no +Metadata-protocol: none + +A record's full-page form (`/apps/APP/OBJECT/record/ID/edit` or `/apps/APP/OBJECT/new`) opened by a pasted link, a bookmark or a new tab on the app's **name** (rather than its package id) saved, then went Back out of the console: the page the tab showed before came back and the save toast was lost. The console moves a name address to the package-id one on load, and that move made the page believe a console page was behind it. Save and Cancel now go Back only when the console itself navigated to the form; otherwise an edit lands on the record and a new record on the object's list, inside the app, with one toast. A form opened from a list, a view or a record page still returns to that page. diff --git a/ui/e2e/live/record-form-direct-url.spec.ts b/ui/e2e/live/record-form-direct-url.spec.ts index a8fbadf3518..62e967b9592 100644 --- a/ui/e2e/live/record-form-direct-url.spec.ts +++ b/ui/e2e/live/record-form-direct-url.spec.ts @@ -11,8 +11,11 @@ import { expectToast } from './helpers'; * save toast is lost with the unloaded document and the user lands outside the * app. The page must instead go to the record inside the app. A fresh * Playwright page is exactly that entry: its only earlier history entry is - * `about:blank`, not a console page. The URL is the app's one address (its - * package id), the one the console's own links and its address bar carry. + * `about:blank`, not a console page. Both of the app's addresses are opened: + * its package id, the one the console's own links and its address bar carry, + * and its app name, which the console replaces with the package id on load — + * a replace that gives the entry a key with nothing of the console's behind it + * (objectstack-ai/objectstack#28036). * * The record is the test's own, created and deleted through the data API; * the stored value is read back through the same API after the save. @@ -23,56 +26,59 @@ import { expectToast } from './helpers'; */ const APP = 'com.example.showcase'; +const APP_NAME = 'showcase_app'; const OBJECT = 'showcase_task'; const API = process.env.LIVE_API_URL || 'http://localhost:3000'; -test('a full-page edit form opened by URL saves, toasts, and stays inside the app', async ({ page, request }) => { - // A seeded project to hang the task on (`project` is a required master-detail). - const projectsRes = await request.get(`${API}/api/v1/data/showcase_project?$top=1`); - expect(projectsRes.ok(), await projectsRes.text()).toBeTruthy(); - const projectsBody = await projectsRes.json(); - const project = (projectsBody?.records ?? projectsBody?.data ?? [])[0]; - expect(project, 'a seeded showcase_project exists').toBeTruthy(); +for (const [address, opened] of [['package id', APP], ['app name', APP_NAME]] as const) { + test(`a full-page edit form opened by URL on its ${address} saves, toasts, and stays inside the app`, async ({ page, request }) => { + // A seeded project to hang the task on (`project` is a required master-detail). + const projectsRes = await request.get(`${API}/api/v1/data/showcase_project?$top=1`); + expect(projectsRes.ok(), await projectsRes.text()).toBeTruthy(); + const projectsBody = await projectsRes.json(); + const project = (projectsBody?.records ?? projectsBody?.data ?? [])[0]; + expect(project, 'a seeded showcase_project exists').toBeTruthy(); - const stamp = Date.now(); - const createRes = await request.post(`${API}/api/v1/data/${OBJECT}`, { - data: { title: `E2E Direct URL ${stamp}`, project: project.id ?? project._id, status: 'backlog' }, - }); - expect(createRes.ok(), await createRes.text()).toBeTruthy(); - const created = await createRes.json(); - const recordId = created?.id ?? created?.record?.id ?? created?.data?.id; - expect(recordId, 'created task id').toBeTruthy(); + const stamp = Date.now(); + const createRes = await request.post(`${API}/api/v1/data/${OBJECT}`, { + data: { title: `E2E Direct URL ${stamp}`, project: project.id ?? project._id, status: 'backlog' }, + }); + expect(createRes.ok(), await createRes.text()).toBeTruthy(); + const created = await createRes.json(); + const recordId = created?.id ?? created?.record?.id ?? created?.data?.id; + expect(recordId, 'created task id').toBeTruthy(); - try { - const recordPath = `/apps/${APP}/${OBJECT}/record/${encodeURIComponent(recordId)}`; - await page.goto(`${recordPath}/edit`); - const form = page.getByTestId('record-form-page'); - await expect(form).toHaveAttribute('data-mode', 'edit'); + try { + const recordPath = `/apps/${APP}/${OBJECT}/record/${encodeURIComponent(recordId)}`; + await page.goto(`/apps/${opened}/${OBJECT}/record/${encodeURIComponent(recordId)}/edit`); + const form = page.getByTestId('record-form-page'); + await expect(form).toHaveAttribute('data-mode', 'edit'); - const title = form.locator('input[name="title"]'); - await expect(title).toHaveValue(`E2E Direct URL ${stamp}`); - const renamed = `E2E Direct URL renamed ${stamp}`; - await title.fill(renamed); + const title = form.locator('input[name="title"]'); + await expect(title).toHaveValue(`E2E Direct URL ${stamp}`); + const renamed = `E2E Direct URL renamed ${stamp}`; + await title.fill(renamed); - const [saved] = await Promise.all([ - page.waitForResponse( - (r) => ['PATCH', 'PUT'].includes(r.request().method()) && r.url().includes(`/api/v1/data/${OBJECT}/`), - { timeout: 15_000 }, - ), - form.locator('button[type="submit"]').click(), - ]); - expect(saved.ok(), `the save answered ${saved.status()}`).toBe(true); + const [saved] = await Promise.all([ + page.waitForResponse( + (r) => ['PATCH', 'PUT'].includes(r.request().method()) && r.url().includes(`/api/v1/data/${OBJECT}/`), + { timeout: 15_000 }, + ), + form.locator('button[type="submit"]').click(), + ]); + expect(saved.ok(), `the save answered ${saved.status()}`).toBe(true); - // The toast survives because the document did not unload… - await expectToast(page, new RegExp(`Task "${renamed}" updated`)); - // …and the page landed on the record, inside the app. - await expect(page).toHaveURL((url) => url.pathname === recordPath); - await expect(page.getByTestId('record-form-page')).toHaveCount(0); + // The toast survives because the document did not unload… + await expectToast(page, new RegExp(`Task "${renamed}" updated`)); + // …and the page landed on the record, inside the app. + await expect(page).toHaveURL((url) => url.pathname === recordPath); + await expect(page.getByTestId('record-form-page')).toHaveCount(0); - const readRes = await request.get(`${API}/api/v1/data/${OBJECT}/${encodeURIComponent(recordId)}`); - expect(readRes.ok(), `GET ${OBJECT}/${recordId} answered ${readRes.status()}`).toBe(true); - expect((await readRes.json()).record?.title).toBe(renamed); - } finally { - await request.delete(`${API}/api/v1/data/${OBJECT}/${encodeURIComponent(recordId)}`); - } -}); + const readRes = await request.get(`${API}/api/v1/data/${OBJECT}/${encodeURIComponent(recordId)}`); + expect(readRes.ok(), `GET ${OBJECT}/${recordId} answered ${readRes.status()}`).toBe(true); + expect((await readRes.json()).record?.title).toBe(renamed); + } finally { + await request.delete(`${API}/api/v1/data/${OBJECT}/${encodeURIComponent(recordId)}`); + } + }); +} diff --git a/ui/packages/app-shell/src/views/RecordFormPage.aliasAddressReturn-28036.test.tsx b/ui/packages/app-shell/src/views/RecordFormPage.aliasAddressReturn-28036.test.tsx new file mode 100644 index 00000000000..5b0989bce3b --- /dev/null +++ b/ui/packages/app-shell/src/views/RecordFormPage.aliasAddressReturn-28036.test.tsx @@ -0,0 +1,138 @@ +/** + * objectstack-ai/objectstack#28036 — a full-page record form opened on its + * app-NAME address stays in the app after save or cancel. The console moves + * that address to the package-id one with a `replace` navigate on load + * (`AppContent`'s canonical app pathname), which gives the entry a generated + * key with nothing of the console's behind it. Each cell replays that replace + * on a real browser history under `BrowserRouter`. Direction, written before + * the run: keyed on `location.key !== 'default'`, every alias cell RED (Back + * leaves the app); the in-app cell GREEN. + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, cleanup, fireEvent } from '@testing-library/react'; +import { BrowserRouter, Routes, Route, useLocation, useNavigate } from 'react-router-dom'; +import { I18nProvider } from '@object-ui/i18n'; +import { RecordFormPage } from './RecordFormPage'; + +const h = React.createElement; + +const { toastSuccess } = vi.hoisted(() => ({ toastSuccess: vi.fn() })); + +vi.mock('sonner', () => ({ + toast: Object.assign(vi.fn(), { success: toastSuccess, error: vi.fn(), info: vi.fn(), dismiss: vi.fn() }), +})); + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ + user: { id: 'u1', name: 'Ada', email: 'ada@example.com', role: 'admin' }, + getAuthConfig: async () => ({ features: {} }), + activeOrganization: null, + }), +})); + +// Stand-in form: one button per exit, as `ObjectForm` calls `onSuccess(result)` / `onCancel()`. +vi.mock('@object-ui/plugin-form', async (importOriginal) => ({ + ...(await importOriginal>()), + ObjectForm: ({ schema }: { schema: { onSuccess?: (s?: unknown) => void; onCancel?: () => void } }) => + h( + 'div', + null, + h('button', { type: 'button', 'data-testid': 'fire-success', onClick: () => schema.onSuccess?.({ id: 't1', subject: 'Quarterly review' }) }, 'ok'), + h('button', { type: 'button', 'data-testid': 'fire-cancel', onClick: () => schema.onCancel?.() }, 'cancel'), + ), +})); + +const TASK = { name: 'showcase_task', label: 'Task', nameField: 'subject', fields: { subject: { type: 'text' } } }; +const METADATA = { objects: [TASK], loading: false }; +vi.mock('../providers/MetadataProvider', () => ({ useMetadata: () => METADATA })); +vi.mock('../providers/AdapterProvider', () => ({ useAdapter: () => null })); + +const Where = () => h('div', { 'data-testid': 'where' }, useLocation().pathname); +/** + * The console's two navigations: a react-router push to `to`, and the + * replace `AppContent` makes on an app-name address (to its package id). + */ +const Nav = ({ to, canonical }: { to: string; canonical: string }) => { + const navigate = useNavigate(); + return h( + React.Fragment, + null, + h('button', { type: 'button', 'data-testid': 'push', onClick: () => { void navigate(to); } }, 'push'), + h('button', { type: 'button', 'data-testid': 'canonical', onClick: () => { void navigate(canonical, { replace: true }); } }, 'canonical'), + ); +}; + +const ALIAS = '/apps/crm_app/showcase_task'; +const APP = '/apps/com.example.crm/showcase_task'; +const LIST = APP; +const DETAIL = `${APP}/record/t1`; +const VIEW = `${APP}/view/open`; + +beforeEach(() => { + toastSuccess.mockClear(); +}); +afterEach(() => { cleanup(); vi.restoreAllMocks(); }); + +const formOf = (base: string, mode: 'create' | 'edit') => (mode === 'create' ? `${base}/new` : `${base}/record/t1/edit`); + +/** + * Open the form on its app-name address — loaded on it, or pushed from + * `before` — let the console replace it with the package-id address, fire + * `exit`, return where it lands. + */ +async function exitFromAlias(mode: 'create' | 'edit', exit: 'success' | 'cancel', before?: string): Promise { + window.history.replaceState(null, '', before ?? formOf(ALIAS, mode)); + render( + h(I18nProvider, { + config: { defaultLanguage: 'en', detectBrowserLanguage: false }, + children: h( + BrowserRouter, + null, + h(Nav, { to: formOf(ALIAS, mode), canonical: formOf(APP, mode) }), + h( + Routes, + null, + h(Route, { path: '/apps/:appName/:objectName/new', element: h(RecordFormPage, { mode: 'create' }) }), + h(Route, { path: '/apps/:appName/:objectName/record/:recordId/edit', element: h(RecordFormPage, { mode: 'edit' }) }), + h(Route, { path: '*', element: h(Where) }), + ), + ), + }), + ); + if (before) fireEvent.click(screen.getByTestId('push')); + fireEvent.click(screen.getByTestId('canonical')); + // The replace minted a key, so the old in-app test read this entry as having history. + const { key } = window.history.state as { key?: string }; + expect(key).toBeTruthy(); + expect(key).not.toBe('default'); + fireEvent.click(screen.getByTestId(exit === 'success' ? 'fire-success' : 'fire-cancel')); + return (await screen.findByTestId('where')).textContent ?? ''; +} + +describe('RecordFormPage — the app-name address stays in the app (objectstack-ai/objectstack#28036)', () => { + it('edit, app-name URL: saves, toasts once, lands on the record detail', async () => { + expect(await exitFromAlias('edit', 'success')).toBe(DETAIL); + expect(toastSuccess).toHaveBeenCalledTimes(1); + expect(toastSuccess).toHaveBeenCalledWith('Task "Quarterly review" updated'); + }); + + it('create, app-name URL: saves, toasts once, lands on the object list', async () => { + expect(await exitFromAlias('create', 'success')).toBe(LIST); + expect(toastSuccess).toHaveBeenCalledTimes(1); + expect(toastSuccess).toHaveBeenCalledWith('Task "Quarterly review" created'); + }); + + it('edit, app-name URL: cancel lands on the record detail, no toast', async () => { + expect(await exitFromAlias('edit', 'cancel')).toBe(DETAIL); + expect(toastSuccess).not.toHaveBeenCalled(); + }); + + it('edit, pushed from a page of the app: save goes back to that page', async () => { + expect(await exitFromAlias('edit', 'success', VIEW)).toBe(VIEW); + expect(toastSuccess).toHaveBeenCalledTimes(1); + }); +}); diff --git a/ui/packages/app-shell/src/views/RecordFormPage.directUrlReturn-27454.test.tsx b/ui/packages/app-shell/src/views/RecordFormPage.directUrlReturn-27454.test.tsx index 249d2c53ac3..5a8050753a1 100644 --- a/ui/packages/app-shell/src/views/RecordFormPage.directUrlReturn-27454.test.tsx +++ b/ui/packages/app-shell/src/views/RecordFormPage.directUrlReturn-27454.test.tsx @@ -5,13 +5,15 @@ * tab's own history is longer than the console's, so `window.history.length` * is simulated as 3 on every cell. Direction, written before the run: keyed on * `window.history.length > 1`, every direct-URL cell RED; in-app cells GREEN. + * A real browser history under `BrowserRouter`, since the page reads the entry + * index react-router writes there (objectstack-ai/objectstack#28036). */ import '@testing-library/jest-dom/vitest'; import * as React from 'react'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { render, screen, cleanup, fireEvent } from '@testing-library/react'; -import { MemoryRouter, Routes, Route, useLocation } from 'react-router-dom'; +import { BrowserRouter, Routes, Route, useLocation, useNavigate } from 'react-router-dom'; import { I18nProvider } from '@object-ui/i18n'; import { RecordFormPage } from './RecordFormPage'; @@ -50,6 +52,11 @@ vi.mock('../providers/MetadataProvider', () => ({ useMetadata: () => METADATA }) vi.mock('../providers/AdapterProvider', () => ({ useAdapter: () => null })); const Where = () => h('div', { 'data-testid': 'where' }, useLocation().pathname); +/** The console's own in-app navigation: a react-router push to `to`. */ +const Nav = ({ to }: { to: string }) => { + const navigate = useNavigate(); + return h('button', { type: 'button', 'data-testid': 'push', onClick: () => { void navigate(to); } }, 'push'); +}; const LIST = '/apps/crm/showcase_task'; const DETAIL = '/apps/crm/showcase_task/record/t1'; @@ -61,16 +68,17 @@ beforeEach(() => { }); afterEach(() => { cleanup(); vi.restoreAllMocks(); }); -/** Mount the form behind `before` (none = direct URL), fire `exit`, return where it lands. */ -function exitFrom(mode: 'create' | 'edit', exit: 'success' | 'cancel', before?: string): string { +/** Load the document on `before` and push the form (none = direct URL), fire `exit`, return where it lands. */ +async function exitFrom(mode: 'create' | 'edit', exit: 'success' | 'cancel', before?: string): Promise { const form = mode === 'create' ? `${LIST}/new` : `${DETAIL}/edit`; - const entries = before ? [before, form] : [form]; + window.history.replaceState(null, '', before ?? form); render( h(I18nProvider, { config: { defaultLanguage: 'en', detectBrowserLanguage: false }, children: h( - MemoryRouter, - { initialEntries: entries, initialIndex: entries.length - 1 }, + BrowserRouter, + null, + h(Nav, { to: form }), h( Routes, null, @@ -81,32 +89,33 @@ function exitFrom(mode: 'create' | 'edit', exit: 'success' | 'cancel', before?: ), }), ); + if (before) fireEvent.click(screen.getByTestId('push')); fireEvent.click(screen.getByTestId(exit === 'success' ? 'fire-success' : 'fire-cancel')); - return screen.getByTestId('where').textContent ?? ''; + return (await screen.findByTestId('where')).textContent ?? ''; } describe('RecordFormPage — direct-URL entry returns inside the app (objectstack-ai/objectstack#27454)', () => { - it('edit, direct URL: saves, toasts, lands on the record detail', () => { - expect(exitFrom('edit', 'success')).toBe(DETAIL); + it('edit, direct URL: saves, toasts, lands on the record detail', async () => { + expect(await exitFrom('edit', 'success')).toBe(DETAIL); expect(toastSuccess).toHaveBeenCalledWith('Task "Quarterly review" updated'); }); - it('create, direct URL: saves, toasts, lands on the object list', () => { - expect(exitFrom('create', 'success')).toBe(LIST); + it('create, direct URL: saves, toasts, lands on the object list', async () => { + expect(await exitFrom('create', 'success')).toBe(LIST); expect(toastSuccess).toHaveBeenCalledWith('Task "Quarterly review" created'); }); - it('edit, direct URL: cancel lands on the record detail, no toast', () => { - expect(exitFrom('edit', 'cancel')).toBe(DETAIL); + it('edit, direct URL: cancel lands on the record detail, no toast', async () => { + expect(await exitFrom('edit', 'cancel')).toBe(DETAIL); expect(toastSuccess).not.toHaveBeenCalled(); }); - it('edit, in-app entry: save goes back to the page it came from', () => { - expect(exitFrom('edit', 'success', VIEW)).toBe(VIEW); + it('edit, in-app entry: save goes back to the page it came from', async () => { + expect(await exitFrom('edit', 'success', VIEW)).toBe(VIEW); expect(toastSuccess).toHaveBeenCalledTimes(1); }); - it('create, in-app entry: cancel goes back to the page it came from', () => { - expect(exitFrom('create', 'cancel', VIEW)).toBe(VIEW); + it('create, in-app entry: cancel goes back to the page it came from', async () => { + expect(await exitFrom('create', 'cancel', VIEW)).toBe(VIEW); }); }); diff --git a/ui/packages/app-shell/src/views/RecordFormPage.tsx b/ui/packages/app-shell/src/views/RecordFormPage.tsx index dd43d8e638b..4cf71998331 100644 --- a/ui/packages/app-shell/src/views/RecordFormPage.tsx +++ b/ui/packages/app-shell/src/views/RecordFormPage.tsx @@ -29,7 +29,7 @@ */ import { useCallback, useEffect, useMemo, useState } from 'react'; -import { useParams, useNavigate, useLocation, useSearchParams, Link } from 'react-router-dom'; +import { useParams, useNavigate, useSearchParams, Link } from 'react-router-dom'; import { ObjectForm } from '@object-ui/plugin-form'; import { Button, Empty, EmptyTitle, EmptyDescription } from '@object-ui/components'; import { ArrowLeft, Building2, Database } from 'lucide-react'; @@ -71,7 +71,6 @@ export function RecordFormPage({ mode }: RecordFormPageProps) { recordId: string; }>(); const navigate = useNavigate(); - const location = useLocation(); const [searchParams] = useSearchParams(); const dataSource = useAdapter(); const { objects, loading: metadataLoading } = useMetadata(); @@ -121,22 +120,25 @@ export function RecordFormPage({ mode }: RecordFormPageProps) { /** * Navigate back to the most relevant location. - * Back when a page of this console is behind this one (react-router keys - * the entry it started on `'default'`, as `FormPage`'s cancel reads it), so - * users return to the exact list/view they came from. ⛔ Not + * Back only when this console pushed an entry behind this one, so users + * return to the exact list/view they came from. react-router numbers the + * entries it writes in `history.state.idx`: 0 on the entry the document + * loaded on, one more per push, unchanged by a replace. ⛔ Not + * `location.key !== 'default'`: a replace on load (the app-name address + * moved to the package-id one) mints a key with nothing behind it, so Save + * went Back out of the app (objectstack-ai/objectstack#28036). ⛔ Nor * `window.history.length`: it counts the tab's entries from before the - * console loaded, so a pasted URL went Back out of the app and lost the save - * toast (objectstack-ai/objectstack#27454). Otherwise the record detail - * (edit) or list (create), inside the app. + * console loaded (objectstack-ai/objectstack#27454). Otherwise the record + * detail (edit) or list (create), inside the app. */ - const hasInAppHistory = location.key !== 'default'; const goBack = useCallback(() => { - if (hasInAppHistory) { + const entryIndex = (window.history.state as { idx?: unknown } | null)?.idx; + if (typeof entryIndex === 'number' && entryIndex > 0) { navigate(-1); } else { navigate(recordDetailUrl, { replace: true }); } - }, [navigate, recordDetailUrl, hasInAppHistory]); + }, [navigate, recordDetailUrl]); // Two names, not one (objectui#11733): the title and the toasts speak of ONE // record, so they keep the singular; the breadcrumb link opens the object's