Skip to content

Commit 6a8f615

Browse files
Orchestrator 1.9 backport (#2874)
* fix(orchestrator): preserve nested extraErrors and async validation errors (#2818) * fix(orchestrator-form): Review step with empty form data (#2834) Empty pruned form data (e.g. display-only ActiveText) made generateReviewTableData return undefined and NestedReviewTable throw. Return {} from generateReviewTableData and guard NestedReviewTable. Add regression test and changeset. Made-with: Cursor * fix(orchestrator-form-react): fix ui styling issues in workflow results page (#2808) --------- Co-authored-by: Karthik Jeeyar <karthik@redhat.com>
1 parent 155b45c commit 6a8f615

18 files changed

Lines changed: 356 additions & 24 deletions
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@red-hat-developer-hub/backstage-plugin-orchestrator-form-react': patch
3+
---
4+
5+
fix: avoid Review step crash when form data is empty (for example display-only `ActiveText` with no submitted values). `generateReviewTableData` returns `{}` instead of undefined, and `NestedReviewTable` handles missing data.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@red-hat-developer-hub/backstage-plugin-orchestrator': patch
3+
---
4+
5+
Fix UI styling issues in worfkflow results page
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@red-hat-developer-hub/backstage-plugin-orchestrator-form-widgets': patch
3+
'@red-hat-developer-hub/backstage-plugin-orchestrator-form-react': patch
4+
---
5+
6+
Fix multi-step workflow forms dropping or misplacing async validation errors and deep nested field paths.
7+
8+
- @red-hat-developer-hub/backstage-plugin-orchestrator-form-widgets: Fix safeSet deep paths and sequential async validation in getExtraErrors.
9+
10+
- @red-hat-developer-hub/backstage-plugin-orchestrator-form-react: Wrap extraErrors with the active step key so RJSF matches the root schema.

workspaces/orchestrator/plugins/orchestrator-form-react/src/components/NestedReviewTable.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,10 @@ export const NestedReviewTable: React.FC<NestedReviewTableProps> = ({
112112
data,
113113
}) => {
114114
const { classes } = useStyles();
115+
const safeData = data ?? {};
115116

116117
// Group top-level sections
117-
const sections: [string, JsonValue][] = Object.entries(data).filter(
118+
const sections: [string, JsonValue][] = Object.entries(safeData).filter(
118119
([_, value]) => value !== undefined,
119120
) as [string, JsonValue][];
120121

workspaces/orchestrator/plugins/orchestrator-form-react/src/components/OrchestratorFormWrapper.tsx

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import {
3535
import { useTranslation } from '../hooks/useTranslation';
3636
import { getActiveStepKey } from '../utils/getSortedStepEntries';
3737
import { useStepperContext } from '../utils/StepperContext';
38+
import { toRootExtraErrors } from '../utils/toRootExtraErrors';
3839
import useValidator from '../utils/useValidator';
3940
import { AuthRequester } from './AuthRequester';
4041
import { createHiddenFieldTemplate } from './HiddenFieldTemplate';
@@ -108,15 +109,7 @@ const FormComponent = (decoratorProps: FormDecoratorProps) => {
108109
extraErrorsUiSchema,
109110
);
110111

111-
if (activeKey) {
112-
setExtraErrors(
113-
_extraErrors?.[activeKey]
114-
? (_extraErrors[activeKey] as ErrorSchema<JsonObject>)
115-
: undefined,
116-
);
117-
} else {
118-
setExtraErrors(_extraErrors);
119-
}
112+
setExtraErrors(toRootExtraErrors(activeKey, _extraErrors));
120113
} catch (err) {
121114
_validationError = err as Error;
122115
} finally {

workspaces/orchestrator/plugins/orchestrator-form-react/src/utils/generateReviewTableData.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,4 +351,21 @@ describe('mapSchemaToData', () => {
351351
const result = generateReviewTableData(schema, data);
352352
expect(result).toEqual(expectedResult);
353353
});
354+
355+
it('returns an empty object when form data is empty (e.g. display-only ActiveText)', () => {
356+
const schema: JSONSchema7 = {
357+
type: 'object',
358+
title: 'Access request',
359+
properties: {
360+
infoMessage: {
361+
type: 'string',
362+
title: 'Info',
363+
'ui:widget': 'ActiveText',
364+
} as JSONSchema7,
365+
},
366+
};
367+
368+
const result = generateReviewTableData(schema, {});
369+
expect(result).toEqual({});
370+
});
354371
});

workspaces/orchestrator/plugins/orchestrator-form-react/src/utils/generateReviewTableData.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,8 @@ function generateReviewTableData(
101101
data,
102102
options?.includeHiddenFields ?? false,
103103
);
104-
return result[''] as JsonObject;
104+
const root = result[''] as JsonObject | undefined;
105+
return root ?? {};
105106
}
106107

107108
export default generateReviewTableData;
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* Copyright Red Hat, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import { ERRORS_KEY } from '@rjsf/utils';
18+
19+
import { toRootExtraErrors } from './toRootExtraErrors';
20+
21+
describe('toRootExtraErrors', () => {
22+
it('returns undefined when extraErrors is undefined', () => {
23+
expect(toRootExtraErrors('my-step', undefined)).toBeUndefined();
24+
});
25+
26+
it('returns extraErrors unchanged when activeKey is undefined', () => {
27+
const tree = { a: { [ERRORS_KEY]: ['x'] } };
28+
expect(toRootExtraErrors(undefined, tree)).toBe(tree);
29+
});
30+
31+
it('wraps the active step slice so RJSF receives root-shaped errors', () => {
32+
const inner = {
33+
xParams: {
34+
fieldA: { [ERRORS_KEY]: ['invalid A'] },
35+
fieldB: { [ERRORS_KEY]: ['invalid B'] },
36+
},
37+
};
38+
const fromGetExtraErrors = {
39+
'my-solution': inner,
40+
};
41+
expect(toRootExtraErrors('my-solution', fromGetExtraErrors)).toEqual({
42+
'my-solution': inner,
43+
});
44+
});
45+
46+
it('returns undefined when the active step has no errors in the tree', () => {
47+
expect(
48+
toRootExtraErrors('missing-step', {
49+
'other-step': { [ERRORS_KEY]: ['e'] },
50+
}),
51+
).toBeUndefined();
52+
});
53+
});
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* Copyright Red Hat, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import { JsonObject } from '@backstage/types';
18+
19+
import { ErrorSchema } from '@rjsf/utils';
20+
21+
/**
22+
* RJSF `extraErrors` must match the root `schema.properties` shape. For
23+
* multi-step forms, `getExtraErrors` returns a tree keyed by step name; pass
24+
* `activeKey` so the slice is wrapped again for the root form.
25+
*/
26+
export function toRootExtraErrors(
27+
activeKey: string | undefined,
28+
extraErrors: ErrorSchema<JsonObject> | undefined,
29+
): ErrorSchema<JsonObject> | undefined {
30+
if (!extraErrors) {
31+
return undefined;
32+
}
33+
if (!activeKey) {
34+
return extraErrors;
35+
}
36+
const slice = extraErrors[activeKey];
37+
if (!slice) {
38+
return undefined;
39+
}
40+
return { [activeKey]: slice } as ErrorSchema<JsonObject>;
41+
}
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/*
2+
* Copyright Red Hat, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import { ERRORS_KEY } from '@rjsf/utils';
18+
19+
import { safeSet } from './safeSet';
20+
import { JsonObject } from '@backstage/types/index';
21+
22+
describe('safeSet', () => {
23+
it('splits only on the first dot so deep paths (e.g. step.x.y) are not truncated', () => {
24+
// `String.prototype.split(".", 2)` truncates remainder (e.g. "a.b.c" -> ["a","b"]);
25+
// paths with three or more segments must recurse on `a` + `b.c`, not `a` + `b` only.
26+
const errors: JsonObject = {};
27+
safeSet(errors, 'a.b.c.d', { [ERRORS_KEY]: ['deep'] });
28+
expect(errors).toEqual({
29+
a: {
30+
b: {
31+
c: {
32+
d: {
33+
[ERRORS_KEY]: ['deep'],
34+
},
35+
},
36+
},
37+
},
38+
});
39+
});
40+
41+
it('sets a single-segment path', () => {
42+
const errors: JsonObject = {};
43+
safeSet(errors, 'onlyKey', { [ERRORS_KEY]: ['msg'] });
44+
expect(errors).toEqual({ onlyKey: { [ERRORS_KEY]: ['msg'] } });
45+
});
46+
47+
it('merges sibling leaf paths under a shared prefix when applied sequentially', () => {
48+
const errors: JsonObject = {};
49+
safeSet(errors, 'my-solution.xParams.fieldA', {
50+
[ERRORS_KEY]: ['a'],
51+
});
52+
safeSet(errors, 'my-solution.xParams.fieldB', {
53+
[ERRORS_KEY]: ['b'],
54+
});
55+
safeSet(errors, 'my-solution.xParams.fieldC', {
56+
[ERRORS_KEY]: ['c'],
57+
});
58+
expect(errors).toEqual({
59+
'my-solution': {
60+
xParams: {
61+
fieldA: { [ERRORS_KEY]: ['a'] },
62+
fieldB: { [ERRORS_KEY]: ['b'] },
63+
fieldC: { [ERRORS_KEY]: ['c'] },
64+
},
65+
},
66+
});
67+
});
68+
});

0 commit comments

Comments
 (0)