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
35 changes: 35 additions & 0 deletions .changeset/settings-per-field-validation-errors.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
"@object-ui/console": patch
---

feat(settings): a rejected save marks the fields that caused it — objectstack#4224 follow-up

A `SETTINGS_VALIDATION` rejection names the offending keys, and the settings page
threw all of it away. Every failure collapsed into one toast carrying the
server's summary sentence, with nothing marked on the inputs — so on a namespace
with a dozen keys the user was told a value was wrong and left to find which.

**That was not the console's fault, which is the part worth recording.** The
server sent `fields` as a `Record<key, message>` hung *beside* `error.code`, a
position `ApiErrorSchema` never declared — it survived only because the schema
is a plain `z.object` and strips undeclared keys rather than rejecting them.
`extractFieldErrors` reads arrays (`details.fields`, `fields`,
`validationErrors`), so a map at an undeclared position matched nothing and
returned `null`. objectstack#4224 moved it to `error.details.fields` as the
declared `FieldError[]`, which is what makes this wiring a few lines rather than
a parser.

What changes for a user: the server's message now renders against the input that
caused it, in the slot the help text occupies, and clears the moment that field
is edited, on Discard, or on the next successful save. `SettingsField` gained an
`error` prop; it sets `aria-invalid` and `aria-describedby` on the control and
gives the message `role="alert"`, so the rejection is announced rather than being
conveyed by colour alone.

The toast still fires alongside the per-field marks. The offending field can be
scrolled out of view or hidden behind a `visible` expression, and a save that
appears to do nothing is the worse failure.

Fields the server did not name are left unmarked — a wrong mark on an innocent
input is worse than the generic toast that was already there — and a failure
carrying no field array (a 500, an unknown namespace) behaves exactly as before.
50 changes: 46 additions & 4 deletions apps/console/src/pages/settings/SettingsField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* (which carries provenance and lock state).
*/

import { useId } from 'react';
import { cloneElement, isValidElement, useId, type ReactElement } from 'react';
import {
Input,
Textarea,
Expand Down Expand Up @@ -46,6 +46,16 @@ export interface SettingsFieldProps {
locked?: boolean;
/** i18n helpers bound to the parent settings namespace. */
labels?: SettingsLabelHelpers;
/**
* Server-side rejection for THIS field, from the last failed save
* (objectstack#4224). Already localized by the server — rendered verbatim,
* not re-worded here.
*
* Replaces the help text while present, rather than stacking below it: the
* two occupy the same slot and say the same kind of thing, and a description
* sitting under a red error reads as if the field has two states at once.
*/
error?: string;
}

function InheritanceBadges({
Expand Down Expand Up @@ -132,8 +142,25 @@ function FieldDescription({ description }: { description?: string }) {
return <p className="text-xs text-muted-foreground mt-1">{description}</p>;
}

/**
* The server's rejection for this field, in the slot the help text normally
* occupies (objectstack#4224).
*
* `role="alert"` so a screen reader announces it when it appears after a save
* attempt — the sighted cue is colour, which is not a cue at all for everyone.
* The `id` is what the input points `aria-describedby` at, so the association
* survives for assistive tech rather than being purely visual adjacency.
*/
function FieldError({ id, message }: { id: string; message: string }) {
return (
<p id={id} role="alert" className="text-xs text-destructive mt-1">
{message}
</p>
);
}

export function SettingsField(props: SettingsFieldProps) {
const { spec, resolved, value, onChange, onAction, locked, saving, labels } = props;
const { spec, resolved, value, onChange, onAction, locked, saving, labels, error } = props;
const id = useId();
const disabled = Boolean(locked || saving);
const literalLabel = resolveLabel(spec.label);
Expand Down Expand Up @@ -231,11 +258,26 @@ export function SettingsField(props: SettingsFieldProps) {

// -------- Inputs --------

// Wired onto the control itself, not just rendered beside it: `aria-invalid`
// is what announces "this one was rejected", and `aria-describedby` is what
// ties the message to the input for a screen reader. Every input type goes
// through `wrapper`, so marking it here covers all of them at once instead of
// per-case (objectstack#4224).
const errorId = `${id}-error`;
const wrapper = (children: React.ReactNode) => (
<div className="space-y-1.5 py-2">
<FieldHeader spec={spec} resolved={resolved} labelText={fieldLabel} labels={labels} />
{children}
<FieldDescription description={fieldHelp} />
{error && isValidElement(children)
? cloneElement(children as ReactElement<Record<string, unknown>>, {
'aria-invalid': true,
'aria-describedby': errorId,
})
: children}
{error ? (
<FieldError id={errorId} message={error} />
) : (
<FieldDescription description={fieldHelp} />
)}
</div>
);

Expand Down
55 changes: 53 additions & 2 deletions apps/console/src/pages/settings/SettingsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { useParams, useNavigate } from 'react-router-dom';
import { toast } from 'sonner';
import { Loader2, ArrowLeft, RotateCcw } from 'lucide-react';
import { Button, Card, CardContent, Skeleton, Badge } from '@object-ui/components';
import { extractFieldErrors } from '@object-ui/react';
import { getIcon } from '../../utils/getIcon';
import { SettingsField } from './SettingsField';
import {
Expand Down Expand Up @@ -42,6 +43,12 @@ function evalVisibility(expr: string | undefined, data: Record<string, unknown>)
}
}

/** `{ ...rest }` without `key`, returned as a new object so React sees the change. */
function omitKey<T>(map: Record<string, T>, key: string): Record<string, T> {
const { [key]: _dropped, ...rest } = map;
return rest;
}

export function SettingsView() {
const params = useParams<{ namespace?: string }>();
const navigate = useNavigate();
Expand All @@ -53,6 +60,16 @@ export function SettingsView() {
const [error, setError] = useState<string | null>(null);
/** Live edit map keyed by spec.key. Falls back to resolved value when undefined. */
const [draft, setDraft] = useState<Record<string, unknown>>({});
/**
* Per-field rejections from the last failed save, keyed by field name
* (objectstack#4224). Empty until a save comes back `SETTINGS_VALIDATION`.
*
* Before #4224 the server sent this as a `Record<key, message>` hung beside
* `error.code`, which `extractFieldErrors` could not read — so the whole
* batch collapsed into one toast that named no field. It now arrives as the
* declared `FieldError[]` under `error.details.fields`.
*/
const [fieldErrors, setFieldErrors] = useState<Record<string, string>>({});

const load = useCallback(async () => {
setLoading(true);
Expand All @@ -61,6 +78,7 @@ export function SettingsView() {
const p = await getSettingsNamespace(namespace);
setPayload(p);
setDraft({});
setFieldErrors({});
} catch (err: any) {
setError(err?.message ?? 'Failed to load settings');
} finally {
Expand Down Expand Up @@ -127,6 +145,7 @@ export function SettingsView() {
const res = await saveSettingsNamespace(namespace, draft);
setPayload({ ...payload, values: { ...values, ...res.values } });
setDraft({});
setFieldErrors({});
toast.success('Settings saved');
} catch (err: any) {
const apiError = err?.payload?.error;
Expand All @@ -135,6 +154,19 @@ export function SettingsView() {
const key = lockedKeyOf(apiError);
toast.error(key ? `Locked by environment: ${key}` : 'Locked by environment');
} else {
// Per-field rejections render against the inputs that caused them
// (objectstack#4224). `extractFieldErrors` reads `details.fields`, so it
// is handed `error` — the object that carries `details` — not the
// whole body.
//
// The toast still fires: the offending field can be scrolled out of
// view, or hidden behind a `visible` expression, and a save that
// silently does nothing is the worse failure. It carries the server's
// summary message, which already names the fields.
const perField = extractFieldErrors(apiError);
if (perField?.length) {
setFieldErrors(Object.fromEntries(perField.map((f) => [f.field, f.message])));
}
toast.error(err?.message ?? 'Save failed');
}
} finally {
Expand Down Expand Up @@ -194,11 +226,20 @@ export function SettingsView() {
spec={spec}
resolved={resolved}
value={current}
onChange={(v) => key && setDraft((d) => ({ ...d, [key]: v }))}
onChange={(v) => {
if (!key) return;
setDraft((d) => ({ ...d, [key]: v }));
// Clear this field's rejection the moment it is edited. The
// error describes the value the server saw, so keeping it on
// screen while the user types contradicts what is in the box —
// and the next save re-derives the set from scratch anyway.
setFieldErrors((e) => (key in e ? omitKey(e, key) : e));
}}
onAction={spec.type === 'action_button' ? () => onAction(spec.id ?? key ?? 'test') : undefined}
locked={resolved?.locked}
saving={saving}
labels={labels}
error={key ? fieldErrors[key] : undefined}
/>
);
})}
Expand All @@ -211,7 +252,17 @@ export function SettingsView() {
{dirtyKeys.length} unsaved change{dirtyKeys.length > 1 ? 's' : ''}
</div>
<div className="flex gap-2">
<Button variant="ghost" size="sm" onClick={() => setDraft({})} disabled={saving}>
<Button
variant="ghost"
size="sm"
onClick={() => {
setDraft({});
// Discarding reverts to the stored values, so rejections of
// the edits being thrown away go with them.
setFieldErrors({});
}}
disabled={saving}
>
<RotateCcw className="h-4 w-4 mr-1" /> Discard
</Button>
<Button onClick={onSave} disabled={saving}>
Expand Down
Loading
Loading