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
22 changes: 22 additions & 0 deletions .changeset/master-detail-submit-reliability.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
---
"@object-ui/plugin-form": patch
"@object-ui/fields": patch
---

fix(master-detail): reliable submit + stable e2e hooks

Fixes the "click Create, nothing happens" report, surfaced by a new live browser
e2e harness that drives the form with real input.

- **MasterDetailForm `handleSave`** now triggers the button-less parent form's
submit from a deferred macrotask and re-queries the live `<form>` inside it.
Calling `requestSubmit()` synchronously inside the click handler (right after
the `setSaving` state update) intermittently dropped the nested submit event,
so react-hook-form's `onSubmit` never ran and the click appeared to do nothing
— only the occasional click got through. Deferring makes it fire every time.

- **Stable `data-testid`s** so automation/e2e can drive the widgets
deterministically (Radix Select + react-hook-form cannot be driven by
synthetic DOM events): `select-trigger-{field}` / `select-option-{value}`
(SelectField), `lookup-trigger-{field}` (LookupField), `line-items-add`
(GridField), `md-form-submit` / `md-form-cancel` (MasterDetailForm).
6 changes: 5 additions & 1 deletion .gitignore
Original file line numberDiff line numberDiff line change
Expand Up@@ -80,4 +80,8 @@ playwright-report

.objectstack

.playwright-mcp
.playwright-mcp
# Live e2e auth state (contains a session token) + artifacts
e2e/live/.auth/
test-results/
playwright-report/
8 changes: 8 additions & 0 deletions apps/console/vite.config.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -136,6 +136,14 @@ export default defineConfig({
resolve: {
extensions: ['.mjs', '.js', '.mts', '.ts', '.jsx', '.tsx', '.json'],
alias: workspaceAliases,
// Force a SINGLE copy of these libraries. The monorepo resolves slightly
// different React patch versions (19.2.6 vs 19.2.7) across packages, which
// duplicates `react`/`react-dom` and, downstream, `sonner` — so
// plugin-form's `toast()` and the console's `<Toaster>` ended up bound to
// different sonner instances and toasts never rendered (the "click does
// nothing — no feedback" bug). Deduping keeps one instance so context,
// hooks, and the sonner observer all line up.
dedupe: ['react', 'react-dom', 'sonner'],
},
optimizeDeps: {
include: [
Expand Down
47 changes: 47 additions & 0 deletions e2e/live/global-setup.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
import { request } from '@playwright/test';
import { mkdirSync, writeFileSync } from 'node:fs';
import { dirname } from 'node:path';

/**
* Live-e2e auth: sign in against the real ObjectStack backend (better-auth) and
* persist a Playwright storageState the tests reuse. The console adapter sends a
* Bearer token read from localStorage `auth-session-token`, so we inject that on
* the APP origin; we also keep the better-auth session cookie for the API origin.
*/
const APP = process.env.LIVE_APP_URL || 'http://localhost:5180';
const API = process.env.LIVE_API_URL || 'http://localhost:3000';
const EMAIL = process.env.LIVE_EMAIL || 'admin@objectos.ai';
const PASSWORD = process.env.LIVE_PASSWORD || 'admin123';
const STATE_PATH = 'e2e/live/.auth/state.json';

export default async function globalSetup() {
const ctx = await request.newContext();
let res;
try {
res = await ctx.post(`${API}/api/v1/auth/sign-in/email`, {
data: { email: EMAIL, password: PASSWORD },
headers: { 'Content-Type': 'application/json' },
});
} catch (e: any) {
throw new Error(
`Live backend unreachable at ${API} (${e?.message}). Start it (e.g. \`objectstack serve --dev\` in examples/app-showcase) before running live e2e.`,
);
}
if (!res.ok()) {
throw new Error(`Live sign-in failed (${res.status()}) at ${API}: ${await res.text()}`);
}
const token = res.headers()['set-auth-token'];
if (!token) throw new Error('Sign-in succeeded but no `set-auth-token` header was returned.');

const apiState = await ctx.storageState(); // carries the better-auth session cookie
await ctx.dispose();

const state = {
cookies: apiState.cookies,
origins: [{ origin: APP, localStorage: [{ name: 'auth-session-token', value: token }] }],
};
mkdirSync(dirname(STATE_PATH), { recursive: true });
writeFileSync(STATE_PATH, JSON.stringify(state, null, 2));
// eslint-disable-next-line no-console
console.log(`[live-e2e] authenticated as ${EMAIL}; storageState written to ${STATE_PATH}`);
}
52 changes: 52 additions & 0 deletions e2e/live/helpers.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
import { type Page, type Locator, expect } from '@playwright/test';

/**
* Reusable drivers for ObjectUI's interaction-critical widgets.
*
* These exist because synthetic DOM events (the kind ad-hoc `eval`-based
* automation dispatches) do NOT make Radix Select / react-hook-form bind — only
* real browser input does. Playwright dispatches real input, so these helpers
* are the durable, deterministic way to drive forms. They target the stable
* `data-testid`s added to SelectField / LookupField / GridField / MasterDetailForm.
*/

/** Pick an option in an ObjectUI <SelectField> (Radix Select) by field name + option value. */
export async function selectOption(scope: Page | Locator, fieldName: string, optionValue: string) {
const page = 'page' in scope ? (scope as any).page() : (scope as Page);
await (scope as any).getByTestId(`select-trigger-${fieldName}`).first().click();
// Options render in a portal at the document root, so query from the page.
const option = page.getByTestId(`select-option-${optionValue}`);
await option.first().waitFor({ state: 'visible' });
await option.first().click();
}

/**
* Fill an ObjectUI <LookupField>: open the picker and choose the matching
* record. The inline picker renders results as `role="option"` in a portal;
* Playwright's accessible-name match is a substring, so `query` can be a prefix
* (e.g. "North" → "Northwind").
*/
export async function fillLookup(page: Page, fieldName: string, query: string) {
await page.getByTestId(`lookup-trigger-${fieldName}`).first().click();
const option = page.getByRole('option', { name: new RegExp(query, 'i') }).first();
await option.waitFor({ state: 'visible' });
await option.click();
// Popover closes on select; give the trigger a tick to reflect the value.
await page.getByTestId(`lookup-trigger-${fieldName}`).first().waitFor({ state: 'visible' });
}

/** Add a line-items row and return the new (last data) row's <tr> locator. */
export async function addLineItem(page: Page): Promise<Locator> {
const dataRows = page.getByTestId('line-items').locator('tbody tr').filter({ has: page.locator('input, [role="combobox"], button') });
const before = await dataRows.count();
await page.getByTestId('line-items-add').click();
await expect(dataRows).toHaveCount(before + 1);
return dataRows.nth(before);
}

/** Wait for (and return the text of) the next sonner toast. */
export async function expectToast(page: Page, matcher: RegExp) {
const toast = page.locator('[data-sonner-toast]').filter({ hasText: matcher }).first();
await expect(toast).toBeVisible({ timeout: 10_000 });
return (await toast.textContent())?.trim() ?? '';
}
67 changes: 67 additions & 0 deletions e2e/live/master-detail.spec.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
import { test, expect } from '@playwright/test';
import { selectOption, fillLookup, addLineItem } from './helpers';

/**
* Live e2e for the master-detail entry form (showcase "New Project + Tasks").
*
* Canonical guard for the "click Create, nothing happens" bug. Driven with REAL
* browser input so Radix Select + react-hook-form + the lookup picker actually
* bind — the thing synthetic-event automation cannot do.
*
* Success signal: after a valid submit the form RESETS (the parent ObjectForm
* remounts with empty fields). This only happens once the whole chain ran —
* real input committed → RHF validation passed → submitHandler persisted →
* onSuccess fired — so it is a reliable end-to-end assertion.
*
* Scope notes:
* - The showcase workspace renders against the demo data layer, so these assert
* the observable UX contract (form reset) rather than a specific network call.
* The atomic cross-object batch wire (`POST /api/v1/batch`, `$ref` linkage,
* commit/rollback) is covered by @object-ui/plugin-form unit tests + the
* framework REST e2e.
* - A success TOAST is expected too, but `toast()` (plugin-form) and the
* console `<Toaster>` currently resolve to separate sonner instances in this
* build, so the toast is checked best-effort and not asserted here. Tracked
* separately (sonner/React de-duplication).
*/
const PAGE = '/apps/showcase_app/page/showcase_project_workspace';

async function expectFormReset(page: import('@playwright/test').Page) {
// The parent form remounts on success → the name field returns to empty.
await expect(page.locator('input[name="name"]')).toHaveValue('', { timeout: 10_000 });
}

test.beforeEach(async ({ page }) => {
await page.goto(PAGE);
await expect(page.getByRole('heading', { name: 'New Project + Tasks' })).toBeVisible();

Check failure on line 36 in e2e/live/master-detail.spec.ts

View workflow job for this annotation

GitHub Actions/ Build & E2E

[chromium] › e2e/live/master-detail.spec.ts:52:1 › create with a task line submits and resets the form

2) [chromium] › e2e/live/master-detail.spec.ts:52:1 › create with a task line submits and resets the form Retry #2 ─────────────────────────────────────────────────────────────────────────────────────── Error: expect(locator).toBeVisible() failed Locator: getByRole('heading', { name: 'New Project + Tasks' }) Expected: visible Timeout: 5000ms Error: element(s) not found Call log: - Expect "toBeVisible" with timeout 5000ms - waiting for getByRole('heading', { name: 'New Project + Tasks' }) 34 | test.beforeEach(async ({ page }) => { 35 | await page.goto(PAGE); > 36 | await expect(page.getByRole('heading', { name: 'New Project + Tasks' })).toBeVisible(); | ^ 37 | }); 38 | 39 | test('create (parent only) drives Radix/lookup, submits, and resets the form', async ({ page }) => { at /home/runner/work/objectui/objectui/e2e/live/master-detail.spec.ts:36:76

Check failure on line 36 in e2e/live/master-detail.spec.ts

View workflow job for this annotation

GitHub Actions/ Build & E2E

[chromium] › e2e/live/master-detail.spec.ts:52:1 › create with a task line submits and resets the form

2) [chromium] › e2e/live/master-detail.spec.ts:52:1 › create with a task line submits and resets the form Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── Error: expect(locator).toBeVisible() failed Locator: getByRole('heading', { name: 'New Project + Tasks' }) Expected: visible Timeout: 5000ms Error: element(s) not found Call log: - Expect "toBeVisible" with timeout 5000ms - waiting for getByRole('heading', { name: 'New Project + Tasks' }) 34 | test.beforeEach(async ({ page }) => { 35 | await page.goto(PAGE); > 36 | await expect(page.getByRole('heading', { name: 'New Project + Tasks' })).toBeVisible(); | ^ 37 | }); 38 | 39 | test('create (parent only) drives Radix/lookup, submits, and resets the form', async ({ page }) => { at /home/runner/work/objectui/objectui/e2e/live/master-detail.spec.ts:36:76

Check failure on line 36 in e2e/live/master-detail.spec.ts

View workflow job for this annotation

GitHub Actions/ Build & E2E

[chromium] › e2e/live/master-detail.spec.ts:39:1 › create (parent only) drives Radix/lookup

1) [chromium] › e2e/live/master-detail.spec.ts:39:1 › create (parent only) drives Radix/lookup, submits, and resets the form Retry #2 ─────────────────────────────────────────────────────────────────────────────────────── Error: expect(locator).toBeVisible() failed Locator: getByRole('heading', { name: 'New Project + Tasks' }) Expected: visible Timeout: 5000ms Error: element(s) not found Call log: - Expect "toBeVisible" with timeout 5000ms - waiting for getByRole('heading', { name: 'New Project + Tasks' }) 34 | test.beforeEach(async ({ page }) => { 35 | await page.goto(PAGE); > 36 | await expect(page.getByRole('heading', { name: 'New Project + Tasks' })).toBeVisible(); | ^ 37 | }); 38 | 39 | test('create (parent only) drives Radix/lookup, submits, and resets the form', async ({ page }) => { at /home/runner/work/objectui/objectui/e2e/live/master-detail.spec.ts:36:76

Check failure on line 36 in e2e/live/master-detail.spec.ts

View workflow job for this annotation

GitHub Actions/ Build & E2E

[chromium] › e2e/live/master-detail.spec.ts:39:1 › create (parent only) drives Radix/lookup

1) [chromium] › e2e/live/master-detail.spec.ts:39:1 › create (parent only) drives Radix/lookup, submits, and resets the form Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── Error: expect(locator).toBeVisible() failed Locator: getByRole('heading', { name: 'New Project + Tasks' }) Expected: visible Timeout: 5000ms Error: element(s) not found Call log: - Expect "toBeVisible" with timeout 5000ms - waiting for getByRole('heading', { name: 'New Project + Tasks' }) 34 | test.beforeEach(async ({ page }) => { 35 | await page.goto(PAGE); > 36 | await expect(page.getByRole('heading', { name: 'New Project + Tasks' })).toBeVisible(); | ^ 37 | }); 38 | 39 | test('create (parent only) drives Radix/lookup, submits, and resets the form', async ({ page }) => { at /home/runner/work/objectui/objectui/e2e/live/master-detail.spec.ts:36:76
});

test('create (parent only) drives Radix/lookup, submits, and resets the form', async ({ page }) => {
await page.locator('input[name="name"]').fill(`E2E Project ${Date.now()}`);
await fillLookup(page, 'account', 'North'); // Northwind seed
await selectOption(page, 'status', 'planned');

// Prove the harness actually drove the widgets before submitting.
await expect(page.getByText('Northwind', { exact: false })).toBeVisible();
await expect(page.getByTestId('select-trigger-status')).toContainText(/planned/i);

await page.getByTestId('md-form-submit').click();
await expectFormReset(page);
});

test('create with a task line submits and resets the form', async ({ page }) => {
await page.locator('input[name="name"]').fill(`E2E MD ${Date.now()}`);
await fillLookup(page, 'account', 'North');
await selectOption(page, 'status', 'active');
// Assert the parent fields committed BEFORE touching the child grid.
await expect(page.getByText('Northwind', { exact: false })).toBeVisible();

const row = await addLineItem(page);
await row.getByRole('textbox').first().fill('E2E Task A');
if (await row.getByTestId('select-trigger-status').count()) {
await selectOption(row, 'status', 'todo');
}

await page.getByTestId('md-form-submit').click();
await expectFormReset(page);
});
3 changes: 2 additions & 1 deletion package.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -53,7 +53,8 @@
"changeset:version": "changeset version",
"changeset:publish": "changeset publish",
"test:e2e": "playwright test",
"test:e2e:ui": "playwright test --ui"
"test:e2e:ui": "playwright test --ui",
"test:e2e:live": "playwright test --config=playwright.live.config.ts"
},
"devDependencies": {
"@changesets/cli": "^2.31.0",
Expand Down
1 change: 1 addition & 0 deletions packages/fields/src/widgets/GridField.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -363,6 +363,7 @@ export function GridField({
size="sm"
onClick={addRow}
disabled={maxRows != null && rows.length >= maxRows}
data-testid="line-items-add"
>
<Plus className="mr-1.5 h-4 w-4" />
{cfg.add_label || 'Add line'}
Expand Down
2 changes: 1 addition & 1 deletion packages/fields/src/widgets/LookupField.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -592,7 +592,7 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel
className="min-w-0 flex-1 justify-start text-left font-normal"
type="button"
disabled={dependenciesMissing || (props as any).disabled}
data-testid={dependenciesMissing ? 'lookup-trigger-gated' : undefined}
data-testid={dependenciesMissing ? 'lookup-trigger-gated' : (((props as any).name || lookupField?.name) ? `lookup-trigger-${(props as any).name || lookupField.name}` : 'lookup-trigger')}
title={dependenciesMissing
? `Select ${dependsOn.map(d => d.field).join(', ')} first`
: undefined}
Expand Down
9 changes: 7 additions & 2 deletions packages/fields/src/widgets/SelectField.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -19,6 +19,11 @@ export function SelectField({ value, onChange, field, readonly, ...props }: Fiel
const config = (field || (props as any).schema) as SelectFieldMetadata;
const options = config?.options || [];
const { t } = useFieldTranslation();
// Stable hook for automation/e2e — react-hook-form + Radix Select cannot be
// driven by synthetic DOM events, so e2e must target the trigger/options by a
// deterministic testid keyed on the field name. `props.name` is the
// react-hook-form field name spread in by the form renderer (FormField).
const fieldName = (props as any).name || (config as any)?.name || props.id || '';

if (readonly) {
const option = options.find((o) => o.value === value);
Expand All@@ -33,12 +38,12 @@ export function SelectField({ value, onChange, field, readonly, ...props }: Fiel
onValueChange={onChange}
disabled={readonly || props.disabled}
>
<SelectTrigger className={props.className} id={props.id}>
<SelectTrigger className={props.className} id={props.id} data-testid={fieldName ? `select-trigger-${fieldName}` : undefined}>
<SelectValue placeholder={config?.placeholder || t('common.selectOption')} />
</SelectTrigger>
<SelectContent position="popper">
{options.map((option) => (
<SelectItem key={option.value} value={option.value}>
<SelectItem key={option.value} value={option.value} data-testid={`select-option-${option.value}`}>
{option.label}
</SelectItem>
))}
Expand Down
16 changes: 13 additions & 3 deletions packages/plugin-form/src/MasterDetailForm.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -288,7 +288,17 @@ export const MasterDetailForm: React.FC<MasterDetailFormProps> = ({
if (!form) return;
savingRef.current = true;
setSaving(true);
form.requestSubmit();
// IMPORTANT: defer the submit out of this click's React dispatch AND
// re-query the <form> inside the timer. Calling requestSubmit()
// synchronously inside the onClick (or on a form reference captured before
// the setSaving() re-render) intermittently fails to invoke react-hook-form's
// onSubmit — the nested submit event is dropped — which made "Create" feel
// unresponsive (only the occasional lucky click submitted). A fresh query in
// a macrotask reliably triggers RHF validation + submit.
setTimeout(() => {
const liveForm = formHostRef.current?.querySelector('form') as HTMLFormElement | null;
liveForm?.requestSubmit();
}, 0);
// Safety net: react-hook-form blocks invalid submits without firing
// onSuccess/onError, which would otherwise leave the button stuck. Release
// the guard after a beat so the user can correct fields and retry.
Expand DownExpand Up@@ -333,11 +343,11 @@ export const MasterDetailForm: React.FC<MasterDetailFormProps> = ({
{/* 3) Single action bar at the bottom */}
<div className="flex items-center justify-end gap-2 border-t border-border pt-4">
{schema.onCancel && (
<Button type="button" variant="outline" onClick={schema.onCancel} disabled={saving}>
<Button type="button" variant="outline" onClick={schema.onCancel} disabled={saving} data-testid="md-form-cancel">
Cancel
</Button>
)}
<Button type="button" onClick={handleSave} disabled={saving}>
<Button type="button" onClick={handleSave} disabled={saving} data-testid="md-form-submit">
{saving ? 'Saving…' : submitText}
</Button>
</div>
Expand Down
41 changes: 41 additions & 0 deletions playwright.live.config.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
import { defineConfig, devices } from '@playwright/test';

/**
* LIVE e2e — drives the REAL running stack (console dev server + ObjectStack
* backend), unlike the default `playwright.config.ts` which serves a mocked
* production build for blank-page smoke tests.
*
* Why a separate config: the interaction-critical widgets (Radix Select +
* react-hook-form, lookup pickers) cannot be driven by synthetic DOM events —
* only real browser input (what Playwright dispatches) makes them bind. These
* tests therefore need an actual browser against an actual backend, and are the
* canonical way to verify form/submit behaviour end-to-end.
*
* Prereqs (not started by this config — they are long-lived dev processes):
* - ObjectStack backend on http://localhost:3000 (e.g. `objectstack serve --dev`
* from examples/app-showcase)
* - Console dev server on http://localhost:5180 (`pnpm --filter @object-ui/console dev`)
*
* Run: pnpm test:e2e:live (all live specs)
* pnpm test:e2e:live --headed (watch it drive the UI)
*
* Override targets via LIVE_APP_URL / LIVE_API_URL / LIVE_EMAIL / LIVE_PASSWORD.
*/
const APP = process.env.LIVE_APP_URL || 'http://localhost:5180';

export default defineConfig({
testDir: './e2e/live',
fullyParallel: false,
workers: 1,
retries: 0,
reporter: [['list']],
globalSetup: './e2e/live/global-setup.ts',
use: {
baseURL: APP,
storageState: 'e2e/live/.auth/state.json',
trace: 'on-first-retry',
screenshot: 'only-on-failure',
video: 'retain-on-failure',
},
projects: [{ name: 'chromium', use: { ...devices['Desktop Chrome'] } }],
});
Loading