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
13 changes: 13 additions & 0 deletions .changeset/studio-flow-problems-read-publish-verdict.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
'@object-ui/app-shell': patch
'@object-ui/i18n': patch
---

fix(studio): the flow Problems list says what the publish will refuse, and a blank expression reads "This value needs an expression"

Clause-②: no
Metadata-protocol: none

On the Studio's Automations page, a flow's Problems list judged only the canvas structure, so an Update Records value written as an expression and left blank showed a refusal under the value while Problems said "No problems — this flow is structurally valid", and Publish then refused the flow.

The Problems list now reads the verdict the pending draft's read already carries, the one the step inspectors read and the one Publish enforces, so a value the publish refuses is listed, located by the server's own words naming the step and the field. The value cell and the Problems row say the same thing, in business words: "This value needs an expression. Write one, or switch the value back to text." Any other refusal is shown as before.
16 changes: 16 additions & 0 deletions ui/packages/app-shell/src/views/metadata-admin/draftVerdict.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,22 @@ export function withServerOnlyErrors<T extends { path: string; message: string }
return [...clientErrors, ...extra];
}

/**
* objectstack#27799 — a served verdict as the flat, severity-tagged list a
* canvas preview's Problems list reads (`MetadataPreviewProps.diagnostics`):
* the errors, then the warnings, each at the path the server wrote. A page
* that keeps the verdict and draws a flow hands it this, so the Problems list
* says what the publish will refuse, as the inspectors already do.
*/
export function verdictPreviewEntries(
diagnostics: ServerDiagnostics | undefined,
): Array<{ path?: string; message: string; severity: 'error' | 'warning' }> {
return [
...(diagnostics?.errors ?? []).map((e) => ({ path: e.path || undefined, message: e.message, severity: 'error' as const })),
...(diagnostics?.warnings ?? []).map((w) => ({ path: w.path || undefined, message: w.message, severity: 'warning' as const })),
];
}

/**
* objectstack#26858 — the served verdict on the body on screen, for a field
* that edits one path of it. The page holding the draft read provides it
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import type { ScopeGroup } from './useFlowScope.js';
import { FlowExprIssue } from './FlowExprIssue.js';
import {
ASSIGNMENT_ARRAY_FORM_PRESCRIPTION,
isBlankExpressionRefusal,
isEditableValueEnvelope,
valueEnvelopeRefusal,
writeValueEnvelope,
Expand Down Expand Up @@ -227,6 +228,12 @@ export interface FlowKeyValueFieldProps {
toggleLabel: string;
/** Placeholder of an expression row's source input. */
expressionPlaceholder: string;
/**
* What the cell says where the spec refuses an expression left blank
* (`isBlankExpressionRefusal`), in place of the envelope's wire shape.
* The verdict stays the spec's; only its words change.
*/
blankExpressionMessage?: string;
};
/** The spec requires this map (objectui#10948): the label carries {@link RequiredMarker}. */
required?: boolean;
Expand Down Expand Up @@ -371,6 +378,10 @@ export function FlowKeyValueField({
const hole = valueMode === 'hole' && !expression;
const pickerGroups = expression || hole ? (scopeGroups ?? []) : [];
const refusal = envelopeSlot && writesMap ? valueRefusal(rowValue(row)) : null;
const blankText = valueEnvelope?.blankExpressionMessage;
const shownRefusal = refusal
? [...new Set(refusal.map((m) => (blankText && isBlankExpressionRefusal(m) ? blankText : m)))]
: null;
return (
<div key={row.id} className="space-y-1">
{/*
Expand Down Expand Up @@ -436,9 +447,9 @@ export function FlowKeyValueField({
</Button>
)}
</div>
{refusal ? (
{shownRefusal ? (
<div className="space-y-0.5" role="alert">
{refusal.map((message, i) => (
{shownRefusal.map((message, i) => (
<p key={i} className="text-[11px] leading-snug text-destructive">
{message}
</p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
} from '@objectstack/spec/automation';
import { FlowNodeConfigField } from './FlowNodeConfigField';
import type { FlowConfigField } from './flow-node-config';
import { t } from '../i18n';

afterEach(cleanup);

Expand Down Expand Up @@ -119,7 +120,11 @@ describe('assignment value cell — text or CEL expression (objectui#7588)', ()
expect(screen.queryByRole('alert')).toBeNull();
fireEvent.click(screen.getByRole('button', { name: TOGGLE }));
expect(onCommit).toHaveBeenLastCalledWith({ empty: { dialect: 'cel', source: '' } });
expect(screen.getByRole('alert').textContent).toContain(ASSIGNMENT_VALUE_ENVELOPE_REFUSAL);
// objectstack#27799 — the spec's refusal of a blank expression, said in
// business words rather than the envelope's wire shape.
const alert = screen.getByRole('alert').textContent ?? '';
expect(alert).toBe(t('engine.inspector.flowNode.kv.expressionNeeded'));
expect(alert).not.toContain(ASSIGNMENT_VALUE_ENVELOPE_REFUSAL);
});

it('names the legacy array form with the spec prescription, and an expression writes the map', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,7 @@ export function FlowNodeConfigField({ field, value, onCommit, disabled, locale,
? {
toggleLabel: t('engine.inspector.flowNode.kv.asExpression', locale),
expressionPlaceholder: t('engine.inspector.flowNode.kv.expressionPlaceholder', locale),
blankExpressionMessage: t('engine.inspector.flowNode.kv.expressionNeeded', locale),
}
: undefined,
// objectui#12101 — a map whose every value is a flow text slot (http
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,23 @@ export function valueEnvelopeRefusal(value: unknown): string[] | null {
const result = AssignmentValueSchema.safeParse(value);
return result.success ? null : result.error.issues.map((issue) => issue.message);
}

/**
* The spec's refusal of an expression left blank, read once from the spec's
* own judge rather than restated: the messages {@link valueEnvelopeRefusal}
* gives an envelope in the slot's dialect whose `source` is empty.
*/
const BLANK_EXPRESSION_REFUSALS: ReadonlySet<string> = new Set(
valueEnvelopeRefusal({ dialect: VALUE_DIALECT, source: '' }) ?? [],
);

/**
* Whether `message` is the spec's refusal of a blank expression in a value
* slot, so an editor can say it in business words. The node's value cell and
* the flow's Problems list both ask this of the one verdict they show (the
* cell's live spec parse, the publish verdict the draft read serves), so the
* two say the same thing.
*/
export function isBlankExpressionRefusal(message: string): boolean {
return BLANK_EXPRESSION_REFUSALS.has(message);
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ function targetLabel(
title: `${p.target.source} → ${p.target.target}`,
};
}
return { text: 'flow' };
return p.location ? { text: p.location, title: p.location } : { text: 'flow' };
}

export function ProblemsPanel({ problems, selectedKey, onSelectProblem, locale, runStatus, nodes = [] }: ProblemsPanelProps) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { describeExprSite, flowNodeIds, nodeIdPositions, type ExprSite } from '.
import { flowNodeDisplayName } from './flow-canvas-parts.js';
import { t, tFormat } from '../i18n.js';
import { uniqueId } from '../inspectors/unique-id.js';
import { isBlankExpressionRefusal } from '../inspectors/flow-value-envelope.js';

/** What a problem points at on the canvas — drives badge placement + reveal. */
export type FlowProblemTarget =
Expand All @@ -55,6 +56,14 @@ export interface FlowProblem {
* e.g. every hop of a cycle. The badge + click-reveal still use `target`.
*/
highlight?: { nodeIds: string[]; edges: Array<{ source: string; target: string }> };
/**
* Where the server located a problem it could not place on an element: its
* own locator, shown as the row's target in place of the bare flow. A
* publish finding on a node's config value is located in words (`flow 'x' ·
* node 'y' (update_record) … at config.fields.id`), and that is the only
* place the row names the node and the field.
*/
location?: string;
}

/** A server diagnostic entry (subset of the layered record's `_diagnostics`). */
Expand Down Expand Up @@ -269,12 +278,17 @@ export function buildFlowProblems({ nodes, edges, serverDiagnostics, variables,
(serverDiagnostics ?? []).forEach((diag, i) => {
const level: DiagnosticLevel = diag.severity === 'warning' ? 'warning' : 'error';
const target = serverTarget(diag.path, nodes, edges);
const location = target.kind === 'flow' ? pathSegments(diag.path).join('.') : '';
problems.push({
id: `server:${i}:${targetKey(target)}`,
level,
message: diag.message,
// The words the node's value cell uses for the same refusal.
message: isBlankExpressionRefusal(diag.message)
? t('engine.inspector.flowNode.kv.expressionNeeded', locale)
: diag.message,
target,
source: 'server',
...(location ? { location } : {}),
});
});

Expand Down
Loading
Loading