fix(orchestrator): preserve nested extraErrors and async validation errors (#2818)

Author: karthikjeeyar

workspaces/orchestrator/.changeset/sharp-dots-sing.md

@@ -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.

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

@@ -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 HiddenObjectFieldTemplate from './HiddenObjectFieldTemplate';
@@ -102,15 +103,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 {

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

@@ -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();
+  });
+});

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

@@ -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>;
+}

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/safeSet.test.ts

@@ -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'] },
+        },
+      },
+    });
+  });
+});

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/safeSet.ts

@@ -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);
   }
 };

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.test.tsx

@@ -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'],
+          },
+        },
+      },
+    });
+  });
+});

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.ts

@@ -143,7 +143,12 @@ export const useGetExtraErrors = () => {
       '',
     );
 
-    await Promise.all(promises);
+    // Run sequentially so concurrent async callbacks cannot interleave
+    // `safeSet` mutations on the shared `errors` object (lost updates under
+    // the same path prefix, e.g. `step.xParams.fieldA` vs `fieldB`).
+    for (const p of promises) {
+      await p;
+    }
     return errors;
   };
 };