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
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@red-hat-developer-hub/backstage-plugin-orchestrator-form-react': patch
---

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.
5 changes: 5 additions & 0 deletions workspaces/orchestrator/.changeset/new-books-matter.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@red-hat-developer-hub/backstage-plugin-orchestrator': patch
---

Fix UI styling issues in worfkflow results page
10 changes: 10 additions & 0 deletions workspaces/orchestrator/.changeset/sharp-dots-sing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
'@red-hat-developer-hub/backstage-plugin-orchestrator-form-widgets': patch
'@red-hat-developer-hub/backstage-plugin-orchestrator-form-react': patch
---

Fix multi-step workflow forms dropping or misplacing async validation errors and deep nested field paths.

- @red-hat-developer-hub/backstage-plugin-orchestrator-form-widgets: Fix safeSet deep paths and sequential async validation in getExtraErrors.

- @red-hat-developer-hub/backstage-plugin-orchestrator-form-react: Wrap extraErrors with the active step key so RJSF matches the root schema.
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,10 @@ export const NestedReviewTable: React.FC<NestedReviewTableProps> = ({
data,
}) => {
const { classes } = useStyles();
const safeData = data ?? {};

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
import { useTranslation } from '../hooks/useTranslation';
import { getActiveStepKey } from '../utils/getSortedStepEntries';
import { useStepperContext } from '../utils/StepperContext';
import { toRootExtraErrors } from '../utils/toRootExtraErrors';
import useValidator from '../utils/useValidator';
import { AuthRequester } from './AuthRequester';
import { createHiddenFieldTemplate } from './HiddenFieldTemplate';
Expand Down Expand Up @@ -108,15 +109,7 @@ const FormComponent = (decoratorProps: FormDecoratorProps) => {
extraErrorsUiSchema,
);

if (activeKey) {
setExtraErrors(
_extraErrors?.[activeKey]
? (_extraErrors[activeKey] as ErrorSchema<JsonObject>)
: undefined,
);
} else {
setExtraErrors(_extraErrors);
}
setExtraErrors(toRootExtraErrors(activeKey, _extraErrors));
} catch (err) {
_validationError = err as Error;
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,4 +351,21 @@ describe('mapSchemaToData', () => {
const result = generateReviewTableData(schema, data);
expect(result).toEqual(expectedResult);
});

it('returns an empty object when form data is empty (e.g. display-only ActiveText)', () => {
const schema: JSONSchema7 = {
type: 'object',
title: 'Access request',
properties: {
infoMessage: {
type: 'string',
title: 'Info',
'ui:widget': 'ActiveText',
} as JSONSchema7,
},
};

const result = generateReviewTableData(schema, {});
expect(result).toEqual({});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ function generateReviewTableData(
data,
options?.includeHiddenFields ?? false,
);
return result[''] as JsonObject;
const root = result[''] as JsonObject | undefined;
return root ?? {};
}

export default generateReviewTableData;
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* Copyright Red Hat, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { ERRORS_KEY } from '@rjsf/utils';

import { toRootExtraErrors } from './toRootExtraErrors';

describe('toRootExtraErrors', () => {
it('returns undefined when extraErrors is undefined', () => {
expect(toRootExtraErrors('my-step', undefined)).toBeUndefined();
});

it('returns extraErrors unchanged when activeKey is undefined', () => {
const tree = { a: { [ERRORS_KEY]: ['x'] } };
expect(toRootExtraErrors(undefined, tree)).toBe(tree);
});

it('wraps the active step slice so RJSF receives root-shaped errors', () => {
const inner = {
xParams: {
fieldA: { [ERRORS_KEY]: ['invalid A'] },
fieldB: { [ERRORS_KEY]: ['invalid B'] },
},
};
const fromGetExtraErrors = {
'my-solution': inner,
};
expect(toRootExtraErrors('my-solution', fromGetExtraErrors)).toEqual({
'my-solution': inner,
});
});

it('returns undefined when the active step has no errors in the tree', () => {
expect(
toRootExtraErrors('missing-step', {
'other-step': { [ERRORS_KEY]: ['e'] },
}),
).toBeUndefined();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright Red Hat, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { JsonObject } from '@backstage/types';

import { ErrorSchema } from '@rjsf/utils';

/**
* RJSF `extraErrors` must match the root `schema.properties` shape. For
* multi-step forms, `getExtraErrors` returns a tree keyed by step name; pass
* `activeKey` so the slice is wrapped again for the root form.
*/
export function toRootExtraErrors(
activeKey: string | undefined,
extraErrors: ErrorSchema<JsonObject> | undefined,
): ErrorSchema<JsonObject> | undefined {
if (!extraErrors) {
return undefined;
}
if (!activeKey) {
return extraErrors;
}
const slice = extraErrors[activeKey];
if (!slice) {
return undefined;
}
return { [activeKey]: slice } as ErrorSchema<JsonObject>;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Copyright Red Hat, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { ERRORS_KEY } from '@rjsf/utils';

import { safeSet } from './safeSet';
import { JsonObject } from '@backstage/types/index';

describe('safeSet', () => {
it('splits only on the first dot so deep paths (e.g. step.x.y) are not truncated', () => {
// `String.prototype.split(".", 2)` truncates remainder (e.g. "a.b.c" -> ["a","b"]);
// paths with three or more segments must recurse on `a` + `b.c`, not `a` + `b` only.
const errors: JsonObject = {};
safeSet(errors, 'a.b.c.d', { [ERRORS_KEY]: ['deep'] });
expect(errors).toEqual({
a: {
b: {
c: {
d: {
[ERRORS_KEY]: ['deep'],
},
},
},
},
});
});

it('sets a single-segment path', () => {
const errors: JsonObject = {};
safeSet(errors, 'onlyKey', { [ERRORS_KEY]: ['msg'] });
expect(errors).toEqual({ onlyKey: { [ERRORS_KEY]: ['msg'] } });
});

it('merges sibling leaf paths under a shared prefix when applied sequentially', () => {
const errors: JsonObject = {};
safeSet(errors, 'my-solution.xParams.fieldA', {
[ERRORS_KEY]: ['a'],
});
safeSet(errors, 'my-solution.xParams.fieldB', {
[ERRORS_KEY]: ['b'],
});
safeSet(errors, 'my-solution.xParams.fieldC', {
[ERRORS_KEY]: ['c'],
});
expect(errors).toEqual({
'my-solution': {
xParams: {
fieldA: { [ERRORS_KEY]: ['a'] },
fieldB: { [ERRORS_KEY]: ['b'] },
fieldC: { [ERRORS_KEY]: ['c'] },
},
},
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@ export const safeSet: (
path: string,
value: JsonValue,
) => void = (errors, path, value) => {
const steps = path.split('.', 2);
if (steps.length === 1) {
errors[steps[0]] = value;
const dot = path.indexOf('.');
if (dot === -1) {
errors[path] = value;
} else {
const safeObject = (errors[steps[0]] ?? {}) as JsonObject;
errors[steps[0]] = safeObject;
safeSet(safeObject, steps[1], value);
const head = path.slice(0, dot);
const rest = path.slice(dot + 1);
const safeObject = (errors[head] ?? {}) as JsonObject;
errors[head] = safeObject;
safeSet(safeObject, rest, value);
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
/*
* Copyright Red Hat, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { renderHook } from '@testing-library/react';

import { JsonObject } from '@backstage/types';
import { ERRORS_KEY } from '@rjsf/utils';

import { useGetExtraErrors } from './useGetExtraErrors';

jest.mock('./evaluateTemplate', () => ({
evaluateTemplateString: jest
.fn()
.mockResolvedValue('https://example.com/validate'),
}));

jest.mock('./useRequestInit', () => ({
getRequestInit: jest.fn().mockResolvedValue({}),
}));

jest.mock('./useTemplateUnitEvaluator', () => ({
useTemplateUnitEvaluator: jest.fn(() => async () => undefined),
}));

const mockFetch = jest.fn();

jest.mock('@backstage/core-plugin-api', () => {
const actual = jest.requireActual('@backstage/core-plugin-api');
return {
...actual,
useApi: (ref: unknown) => {
if (ref === actual.fetchApiRef) {
return { fetch: mockFetch };
}
throw new Error(
`useGetExtraErrors.test: unmocked api ref: ${String(ref)}`,
);
},
};
});

describe('useGetExtraErrors', () => {
beforeEach(() => {
mockFetch.mockReset();
});

it('aggregates async validation errors for all nested fields under a shared path prefix', async () => {
let fetchCall = 0;
mockFetch.mockImplementation(async () => {
const n = ++fetchCall;
return {
status: 422,
json: async () => ({ issues: [`validation failed for request ${n}`] }),
};
});

const { result } = renderHook(() => useGetExtraErrors());

const uiSchema: JsonObject = {
'my-solution': {
xParams: {
fieldA: {
'ui:widget': 'ActiveTextInput',
'ui:props': { 'validate:url': 'https://example.com/validate' },
},
fieldB: {
'ui:widget': 'ActiveTextInput',
'ui:props': { 'validate:url': 'https://example.com/validate' },
},
fieldC: {
'ui:widget': 'ActiveTextInput',
'ui:props': { 'validate:url': 'https://example.com/validate' },
},
},
},
};

const formData: JsonObject = {
'my-solution': {
xParams: {
fieldA: '',
fieldB: '',
fieldC: '',
},
},
};

const errors = await result.current(formData, uiSchema);

expect(mockFetch).toHaveBeenCalledTimes(3);
expect(errors).toEqual({
'my-solution': {
xParams: {
fieldA: {
[ERRORS_KEY]: ['validation failed for request 1'],
},
fieldB: {
[ERRORS_KEY]: ['validation failed for request 2'],
},
fieldC: {
[ERRORS_KEY]: ['validation failed for request 3'],
},
},
},
});
});
});
Loading
Loading