diff --git a/.changeset/28289-create-user-required-field-mark.md b/.changeset/28289-create-user-required-field-mark.md new file mode 100644 index 00000000000..85f92d4cf02 --- /dev/null +++ b/.changeset/28289-create-user-required-field-mark.md @@ -0,0 +1,14 @@ +--- +'@object-ui/core': patch +'@object-ui/app-shell': patch +--- + +fix(console): Create User left without an email marks the Email input as required, in the user's language + +Clause-②: yes (widening) +Metadata-protocol: none + +Create User with neither an email nor a phone number answers `400 VALIDATION_FAILED` with one `required` entry per input that would satisfy it on `error.fields`. The Console showed only the server's English sentence above the dialog's buttons and marked no input. + +- **`@object-ui/core`**: `refusalFieldErrors`, the reader behind `ActionResult.fieldErrors`, now also reads a refusal's `error.fields` (the spec's `EnhancedApiErrorSchema.fields`), beside `error.details.fields` and the legacy inner `fields`. It is exported. +- **`@object-ui/app-shell`**: a refused `type: 'api'` action carries the refusal's field entries as `fieldErrors`, on the list and record pages alike. The action parameter dialog marks each `required` entry on the param it names, with its own translated "required" sentence and the param's label. When every entry is marked, the refusal box is not drawn. A refusal that names no field, or names one the dialog does not draw, keeps the box with the server's text. diff --git a/ui/packages/app-shell/src/hooks/__tests__/useConsoleActionRuntime.createUserRequiredField-28289.test.tsx b/ui/packages/app-shell/src/hooks/__tests__/useConsoleActionRuntime.createUserRequiredField-28289.test.tsx new file mode 100644 index 00000000000..ebab10fbda8 --- /dev/null +++ b/ui/packages/app-shell/src/hooks/__tests__/useConsoleActionRuntime.createUserRequiredField-28289.test.tsx @@ -0,0 +1,202 @@ +/** + * 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#28289 — Create User left without an email marks the Email input + * as required, in the user's language. + * + * The whole console seam: a real `ActionRunner` with the console runtime's own + * api and param handlers, the real `ActionParamDialog`, and a real Chinese + * `I18nProvider`. The action door is the mocked authenticated fetch, answering + * the body plugin-auth's Create User serves for an empty email (pinned + * server-side by `request-json-body-refusal.test.ts`) as the action door + * relays it whole: `400 VALIDATION_FAILED` with `error.fields`, one `required` + * entry per input that would satisfy it, and an English sentence. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, fireEvent, waitFor, act, cleanup } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; +import { ActionRunner, type ActionResult } from '@object-ui/core'; +import { I18nProvider } from '@object-ui/i18n'; +// Module scope, per AGENTS.md 测试纪律: the dialog's widgets sit behind +// `React.lazy`, and this barrel statically re-exports them. +import '@object-ui/fields'; + +vi.mock('react-router-dom', () => ({ useNavigate: () => vi.fn() })); + +const authFetchSpy = vi.fn(); +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: { id: 'u1', name: 'User', image: null }, activeOrganization: null }), + createAuthenticatedFetch: () => authFetchSpy, +})); +// Only the metadata label lookups are stood in for (no object metadata is +// mounted); translation itself is the real provider's. +vi.mock('@object-ui/i18n', async (importOriginal) => ({ + ...(await importOriginal()), + useObjectLabel: () => ({ + fieldLabel: (_o: any, _n: any, l: any) => l, + fieldOptionLabel: (_o: any, _f: any, _v: any, l: any) => l, + actionParamText: (_o: any, _a: any, _p: any, _attr: any, fallback: any) => fallback, + actionParamOptionLabel: (_o: any, _a: any, _p: any, _v: any, fallback: any) => fallback, + actionDescription: (_o: any, _a: any, fallback: any) => fallback, + actionSubmitText: (_o: any, _a: any, fallback: any) => fallback, + }), +})); +vi.mock('../useActionModal', () => ({ + useActionModal: () => ({ + modalHandler: vi.fn(async () => ({ success: true })), + modalElement: null, + closeModal: () => {}, + resolveModalTarget: vi.fn(async () => null), + }), +})); +vi.mock('sonner', () => { + const fn: any = vi.fn(); + fn.error = vi.fn(); + fn.success = vi.fn(); + return { toast: fn }; +}); + +import { useConsoleActionRuntime } from '../useConsoleActionRuntime'; + +afterEach(() => cleanup()); +beforeEach(() => authFetchSpy.mockReset()); + +/** The declared action, as `sys_user` serves it (the door looks it up by name and target). */ +const CREATE_USER = { + name: 'create_user', + label: 'Create User', + type: 'api', + target: '/api/v1/auth/admin/create-user', +}; +/** The params the dialog draws: the labels a Chinese console resolves for them. */ +const EMAIL = { name: 'email', label: '邮箱', type: 'email' }; +const PHONE = { name: 'phoneNumber', label: '手机号', type: 'text' }; +const NAME = { name: 'name', label: '姓名', type: 'text' }; + +/** The refusal Create User answers for neither an email nor a phone number. */ +function requiredRefusal(fields: string[], message: string) { + return { + success: false, + error: { + code: 'VALIDATION_FAILED', + message, + httpStatus: 400, + fields: fields.map((field) => ({ field, code: 'required', message })), + }, + }; +} + +function answer(status: number, body: unknown) { + authFetchSpy.mockResolvedValue({ ok: status >= 200 && status < 300, status, json: async () => body }); +} + +function Harness({ onReady }: { onReady: (rt: ReturnType) => void }) { + const runtime = useConsoleActionRuntime({ + dataSource: {}, + objects: [{ name: 'sys_user', actions: [CREATE_USER] }] as any[], + objectName: 'sys_user', + }); + const ready = React.useRef(false); + if (!ready.current) { + ready.current = true; + onReady(runtime); + } + return <>{runtime.dialogs}; +} + +/** Mount under Chinese, open Create User with `params`, and press 确认 with nothing typed. */ +async function confirmEmpty(params: unknown[]) { + let runtime!: ReturnType; + render( + + { runtime = rt; }} /> + , + ); + const runner = new ActionRunner({}); + runner.setParamCollectionHandler(runtime.paramCollectionHandler); + runner.setToastHandler(vi.fn()); + runner.registerHandler('api', runtime.apiHandler); + const settled: { result?: ActionResult } = {}; + await act(async () => { + void runner.execute({ ...CREATE_USER, params } as any).then((r) => { settled.result = r; }); + }); + const email = await screen.findByLabelText('邮箱'); + fireEvent.click(screen.getByRole('button', { name: '确认' })); + await waitFor(() => expect(authFetchSpy).toHaveBeenCalledTimes(1)); + return { email, settled }; +} + +describe('objectstack#28289 — Create User marks a required field the server named, in the user\'s language', () => { + it('an empty email marks the Email input with the Chinese required sentence, not the server\'s English', async () => { + answer(400, requiredRefusal(['email'], 'Email is required')); + const { email, settled } = await confirmEmpty([EMAIL, NAME]); + + expect(String(authFetchSpy.mock.calls[0][0]).endsWith('/api/v1/actions/sys_user/create_user')).toBe(true); + expect(await screen.findByText('邮箱 为必填项')).toBeInTheDocument(); + expect(email).toHaveAttribute('aria-invalid', 'true'); + expect(screen.getByLabelText('姓名')).not.toHaveAttribute('aria-invalid', 'true'); + // Every entry is marked, so the English sentence is not drawn beside it. + expect(screen.queryByTestId('action-param-refusal')).toBeNull(); + expect(screen.queryByText('Email is required')).toBeNull(); + expect(screen.getByRole('dialog')).toBeInTheDocument(); + expect(settled.result).toBeUndefined(); + }); + + it('with phone sign-in on, both the Email and the Phone Number inputs are marked', async () => { + answer(400, requiredRefusal(['email', 'phoneNumber'], 'Email or phone number is required')); + const { email } = await confirmEmpty([EMAIL, PHONE, NAME]); + + expect(await screen.findByText('邮箱 为必填项')).toBeInTheDocument(); + expect(screen.getByText('手机号 为必填项')).toBeInTheDocument(); + expect(email).toHaveAttribute('aria-invalid', 'true'); + expect(screen.getByLabelText('手机号')).toHaveAttribute('aria-invalid', 'true'); + expect(screen.queryByTestId('action-param-refusal')).toBeNull(); + }); + + it('typing into the marked input clears its mark', async () => { + answer(400, requiredRefusal(['email'], 'Email is required')); + const { email } = await confirmEmpty([EMAIL, NAME]); + await screen.findByText('邮箱 为必填项'); + + fireEvent.change(email, { target: { value: 'ada@example.com' } }); + + expect(screen.queryByText('邮箱 为必填项')).toBeNull(); + expect(email).toHaveAttribute('aria-invalid', 'false'); + }); + + it('control: a refusal that names no field keeps the server\'s sentence in the box and marks nothing', async () => { + answer(400, { success: false, error: { code: 'INVALID_REQUEST', message: 'A valid email is required', httpStatus: 400 } }); + const { email } = await confirmEmpty([EMAIL, NAME]); + + const box = await screen.findByTestId('action-param-refusal'); + expect(box).toHaveTextContent('A valid email is required'); + expect(screen.queryByText('邮箱 为必填项')).toBeNull(); + expect(email).not.toHaveAttribute('aria-invalid', 'true'); + }); + + it('control: an entry the dialog cannot mark keeps the box beside the marks it can make', async () => { + const body = requiredRefusal(['email'], 'Email is required'); + body.error.fields.push({ field: 'tenant', code: 'required', message: 'Email is required' }); + answer(400, body); + await confirmEmpty([EMAIL, NAME]); + + expect(await screen.findByText('邮箱 为必填项')).toBeInTheDocument(); + expect(screen.getByTestId('action-param-refusal')).toHaveTextContent('Email is required'); + }); + + it('control: a created user closes the dialog', async () => { + answer(200, { success: true, data: { user: { id: 'u_new' } } }); + const { settled } = await confirmEmpty([EMAIL, NAME]); + + await waitFor(() => expect(settled.result?.success).toBe(true)); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + }); +}); diff --git a/ui/packages/app-shell/src/hooks/useConsoleActionRuntime.tsx b/ui/packages/app-shell/src/hooks/useConsoleActionRuntime.tsx index b280c03e7f4..3ef60365488 100644 --- a/ui/packages/app-shell/src/hooks/useConsoleActionRuntime.tsx +++ b/ui/packages/app-shell/src/hooks/useConsoleActionRuntime.tsx @@ -52,7 +52,7 @@ import { EnvironmentEntitlementDialog, type EntitlementDialogState } from '../en import { entitlementDialogFromError, type EntitlementDialogSpec } from '../environment/entitlements.js'; import { resolvePageVarTokens } from '../utils/resolvePageVarTokens.js'; import { launchConsoleFlow } from '../utils/flowLaunch.js'; -import { apiActionFailureMessage } from '../utils/flowResponse.js'; +import { apiActionRefusalResult } from '../utils/flowResponse.js'; import { findDeclaredApiAction, isSameOriginApiTarget, @@ -463,8 +463,9 @@ export function useConsoleActionRuntime(opts: ConsoleActionRuntimeOptions): Cons return { success: false }; } // A flow-backed endpoint's refused run reads its inner refusal, not - // the node chain (objectstack#27487). - return { success: false, error: apiActionFailureMessage(res, body, `HTTP ${res.status}`, language) }; + // the node chain (objectstack#27487); a refusal naming fields carries + // them for the param dialog to mark (objectstack#28289). + return apiActionRefusalResult(res, body, `HTTP ${res.status}`, language); }; // The action door (objectstack#27611). The server assembles the diff --git a/ui/packages/app-shell/src/utils/flowResponse.ts b/ui/packages/app-shell/src/utils/flowResponse.ts index 94ecd0cc170..7ba78e30e84 100644 --- a/ui/packages/app-shell/src/utils/flowResponse.ts +++ b/ui/packages/app-shell/src/utils/flowResponse.ts @@ -218,7 +218,7 @@ * own failure prose (`error`) is passed through untouched — it has no key. */ -import { actionCopySource, actionErrorDetail } from '@object-ui/core'; +import { actionCopySource, actionErrorDetail, refusalFieldErrors } from '@object-ui/core'; import type { ActionResult } from '@object-ui/core'; import { getLoadedBuiltInLocales } from '@object-ui/i18n'; import { declaredUserMessage, delegationScopeMessage, isPermissionError } from '@object-ui/react'; @@ -469,6 +469,29 @@ export function apiActionFailureMessage( return actionErrorDetail(json, fallback); } +/** + * The failed `ActionResult` a `type: 'api'` action answers for a non-2xx: the + * text {@link apiActionFailureMessage} reads, and the refusal's per-field + * entries as `fieldErrors` when it named any (core's `refusalFieldErrors`, the + * reader `/actions` refusals already go through). The parameter dialog marks + * a `required` entry on the param it names (objectstack#28289), so Create + * User left without an email reads "required" under the Email input, in the + * user's language. + */ +export function apiActionRefusalResult( + res: { status: number }, + json: unknown, + fallback: string, + locale: string | undefined, +): ActionResult { + const fieldErrors = refusalFieldErrors(json); + return { + success: false, + error: apiActionFailureMessage(res, json, fallback, locale), + ...(fieldErrors ? { fieldErrors } : {}), + }; +} + /** * Classify a `POST /api/v1/automation/{flow}/trigger` or * `.../runs/{runId}/resume` response. diff --git a/ui/packages/app-shell/src/views/ActionParamDialog.tsx b/ui/packages/app-shell/src/views/ActionParamDialog.tsx index 11f12eda40b..29087c77262 100644 --- a/ui/packages/app-shell/src/views/ActionParamDialog.tsx +++ b/ui/packages/app-shell/src/views/ActionParamDialog.tsx @@ -42,10 +42,19 @@ * run closes it, then the runner toasts as before. A refused run leaves it * open with every value as typed and the refusal shown above the buttons, so * the user can fix the input and confirm again, or cancel. The refusal is the - * run's own `error`, shown as it came: the dialog marks no field from it, - * because nothing in the result names one (the runner's `ActionResult` - * carries the refusal as text only). Without `run` the dialog closes on + * run's own `error`, shown as it came. Without `run` the dialog closes on * Confirm, as it always did. + * + * ## A refusal naming a required param marks it (objectstack#28289) + * + * A refused run whose `ActionResult.fieldErrors` holds a `required` entry for + * a param this dialog draws marks that param the way the dialog's own + * required check does: the translated `actionDialog.requiredError` sentence + * with the param's label, so the user reads it in their language, not the + * server's English. Create User left without an email answers that way. When + * every entry is marked, the refusal box is not drawn, since it would only + * repeat the marks in English; any entry the dialog cannot mark (another + * code, a param it does not draw) keeps the box, with the server's text. */ import { Suspense, useState, useEffect, useMemo } from 'react'; @@ -138,6 +147,29 @@ function refusalText(result: ActionResult): string | null { return typeof nested === 'string' && nested ? nested : null; } +/** + * The marks a refused run's `fieldErrors` put on the dialog's params, as the + * error keys the dialog's own check uses (see the header, objectstack#28289), + * and whether they cover every entry. Only a `required` entry whose `field` + * names a collected param is marked: that is the one the dialog can word + * itself. `complete` is false when there is nothing to mark, so a refusal + * without entries keeps its box. + */ +export function refusalParamMarks( + result: ActionResult, + params: readonly ActionParamDef[], +): { marks: Record; complete: boolean } { + const entries = result.fieldErrors ?? []; + const collected = new Set(params.filter((p) => !p.unresolvedField && !p.carryOver).map((p) => p.name)); + const marks: Record = {}; + let unmarked = 0; + for (const entry of entries) { + if (entry.code === 'required' && collected.has(entry.field)) marks[entry.field] = 'actionDialog.requiredError'; + else unmarked += 1; + } + return { marks, complete: entries.length > 0 && unmarked === 0 }; +} + interface ActionParamDialogProps { state: ParamDialogState; onOpenChange: (open: boolean) => void; @@ -589,7 +621,9 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp setSubmitting(false); } if (!result.success) { - setRefusal(refusalText(result)); + const { marks, complete } = refusalParamMarks(result, visibleParams); + setErrors(marks); + setRefusal(complete ? null : refusalText(result)); return; } } diff --git a/ui/packages/app-shell/src/views/RecordDetailView.tsx b/ui/packages/app-shell/src/views/RecordDetailView.tsx index 7e0804502e9..72200d7f485 100644 --- a/ui/packages/app-shell/src/views/RecordDetailView.tsx +++ b/ui/packages/app-shell/src/views/RecordDetailView.tsx @@ -48,7 +48,7 @@ import { actionContextOrg } from '../utils/actionContextOrg.js'; import { resolveHostAppSegment } from '../utils/appRoute.js'; import { hostInConsolePath } from '../utils/hostInConsolePath.js'; import { launchConsoleFlow } from '../utils/flowLaunch.js'; -import { apiActionFailureMessage } from '../utils/flowResponse.js'; +import { apiActionRefusalResult } from '../utils/flowResponse.js'; import { findDeclaredApiAction, isSameOriginApiTarget, readApiActionDoorSuccess, sendApiActionThroughDoor } from '../utils/apiActionDoor.js'; import { useNavigationContext, useRecordBreadcrumbTitle } from '../context/NavigationContext.js'; // Audit provenance renders as the one-line ; the other @@ -1529,9 +1529,7 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri params, }); if (!door.answered) return door.result; - if (!door.res.ok) { - return { success: false, error: apiActionFailureMessage(door.res, door.json, `HTTP ${door.res.status}`, language) }; - } + if (!door.res.ok) return apiActionRefusalResult(door.res, door.json, `HTTP ${door.res.status}`, language); const answer = readApiActionDoorSuccess(door, `Action "${action.name}"`); if (!answer.ok) return { success: false, error: answer.error }; if (action.refreshAfter === true) notifyRecordChanged(); @@ -1638,7 +1636,7 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri if (!res.ok) { let errBody: any = null; try { errBody = await res.json(); } catch { /* response body not JSON */ } - return { success: false, error: apiActionFailureMessage(res, errBody, `HTTP ${res.status}`, language) }; + return apiActionRefusalResult(res, errBody, `HTTP ${res.status}`, language); } const data = await res.json().catch(() => ({})); if (action.refreshAfter === true) notifyRecordChanged(); diff --git a/ui/packages/core/src/actions/__tests__/actionRefusalFields-28289.test.ts b/ui/packages/core/src/actions/__tests__/actionRefusalFields-28289.test.ts new file mode 100644 index 00000000000..922bc5d91e5 --- /dev/null +++ b/ui/packages/core/src/actions/__tests__/actionRefusalFields-28289.test.ts @@ -0,0 +1,53 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * objectstack-ai/objectstack#28289 — a refusal that names its fields on + * `error.fields` keeps them, as one that names them on `error.details.fields` + * always has. + * + * Both members are the spec's `EnhancedApiErrorSchema`: the data and action + * doors put a thrown validation failure under `details.fields`, and a door + * that builds its own refusal names the list `fields` beside `code`. The body + * below is plugin-auth's Create User answer for an empty email, as the action + * door relays it whole. + */ + +import { describe, it, expect } from 'vitest'; +import { interpretActionResponse, refusalFieldErrors } from '../actionResponse'; + +const CREATE_USER_REFUSAL = { + success: false, + error: { + code: 'VALIDATION_FAILED', + message: 'Email is required', + httpStatus: 400, + fields: [{ field: 'email', code: 'required', message: 'Email is required' }], + }, +}; + +describe('a refusal naming its fields on error.fields keeps them (objectstack#28289)', () => { + it('reads the entries off error.fields', () => { + expect(refusalFieldErrors(CREATE_USER_REFUSAL)).toEqual([ + { field: 'email', code: 'required', message: 'Email is required' }, + ]); + }); + + it('an /actions refusal carrying them answers them as fieldErrors beside its text', () => { + const out = interpretActionResponse({ ok: false, status: 400 }, CREATE_USER_REFUSAL, 'Action "create_user"'); + expect(out).toMatchObject({ ok: false, error: 'Email is required' }); + expect(out.fieldErrors).toEqual([{ field: 'email', code: 'required', message: 'Email is required' }]); + }); + + it('control: error.details.fields is still read', () => { + const body = { + success: false, + error: { code: 'VALIDATION_FAILED', message: 'Issued On is required', details: { fields: [{ field: 'issued_on', code: 'required' }] } }, + }; + expect(refusalFieldErrors(body)).toEqual([{ field: 'issued_on', code: 'required' }]); + }); + + it('control: a refusal naming no field answers none', () => { + expect(refusalFieldErrors({ success: false, error: { code: 'INVALID_REQUEST', message: 'A valid email is required' } })).toBeUndefined(); + }); +}); diff --git a/ui/packages/core/src/actions/actionResponse.ts b/ui/packages/core/src/actions/actionResponse.ts index 192dc3bc374..00c84412e26 100644 --- a/ui/packages/core/src/actions/actionResponse.ts +++ b/ui/packages/core/src/actions/actionResponse.ts @@ -139,14 +139,19 @@ export function interpretActionResponse( } /** - * The `fields` list of a refusal body: `error.details.fields` on the current - * envelope, `fields` on the legacy inner one. Entries with no string `field` - * are dropped rather than guessed at; the rest keep the server's own `code`, - * `message` and `label`. + * The `fields` list of a refusal body, as `ActionResult.fieldErrors` carries + * it. The ADR-0112 envelope carries the list in one of two members, both on + * the spec's `EnhancedApiErrorSchema`: `error.details.fields`, where the data + * and action doors put a thrown validation failure, and `error.fields`, where + * a door that builds its own refusal names it (plugin-auth's Create User, a + * `type: 'api'` target whose envelope the action door relays whole). The + * legacy inner envelope carries it as `fields`. Entries with no string + * `field` are dropped rather than guessed at; the rest keep the server's own + * `code`, `message` and `label`. */ -function refusalFieldErrors(body: unknown): ActionFieldError[] | undefined { - const b = body as { error?: { details?: { fields?: unknown } }; fields?: unknown } | null | undefined; - const raw = b?.error?.details?.fields ?? b?.fields; +export function refusalFieldErrors(body: unknown): ActionFieldError[] | undefined { + const b = body as { error?: { details?: { fields?: unknown }; fields?: unknown }; fields?: unknown } | null | undefined; + const raw = b?.error?.details?.fields ?? b?.error?.fields ?? b?.fields; if (!Array.isArray(raw)) return undefined; const out: ActionFieldError[] = []; for (const entry of raw) {