fix review step processing using multi step templates

Signed-off-by: Stephen Glass <stephen@stephen.glass>

.changeset/popular-pumpkins-rush.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-scaffolder-react': patch
+---
+
+Fix scaffolder review step issue where schema options are not handled for fields on multi-step templates.

plugins/scaffolder-react/src/next/components/ReviewState/ReviewState.test.tsx

@@ -497,4 +497,109 @@ describe('ReviewState', () => {
     expect(queryByRole('row', { name: 'Bar test' })).toBeInTheDocument();
     expect(queryByRole('row', { name: 'Foo test' })).not.toBeInTheDocument();
   });
+
+  it('should handle options in multiple schemas', async () => {
+    const formState = {
+      foo1: 'bar1',
+      foo2: 'bar2',
+      foo3: {
+        foo4: 'bar4',
+      },
+      foo5: 'bar5',
+    };
+
+    const schemas: ParsedTemplateSchema[] = [
+      {
+        mergedSchema: {
+          type: 'object',
+          properties: {
+            foo1: {
+              type: 'string',
+              'ui:backstage': {
+                review: {
+                  name: 'Test 1',
+                },
+              },
+            },
+          },
+        },
+        schema: {},
+        title: 'Schema 1',
+        uiSchema: {},
+      },
+      {
+        mergedSchema: {
+          type: 'object',
+          properties: {
+            foo2: {
+              type: 'string',
+              'ui:backstage': {
+                review: {
+                  name: 'Test 2',
+                },
+              },
+            },
+            foo3: {
+              type: 'object',
+              properties: {
+                foo4: {
+                  type: 'string',
+                  'ui:backstage': {
+                    review: {
+                      name: 'Test 4',
+                    },
+                  },
+                },
+              },
+            },
+          },
+        },
+        schema: {},
+        title: 'Schema 2',
+        uiSchema: {},
+      },
+      {
+        mergedSchema: {
+          type: 'object',
+          dependencies: {
+            foo1: {
+              oneOf: [
+                {
+                  properties: {
+                    foo5: {
+                      type: 'string',
+                      'ui:backstage': {
+                        review: {
+                          name: 'Test 5',
+                        },
+                      },
+                    },
+                  },
+                },
+              ],
+            },
+          },
+        },
+        schema: {},
+        title: 'Schema 3',
+        uiSchema: {},
+      },
+    ];
+
+    const { queryByRole } = render(
+      <ReviewState formState={formState} schemas={schemas} />,
+    );
+
+    // handles options in first schema
+    expect(queryByRole('row', { name: 'Test 1 bar1' })).toBeInTheDocument();
+
+    // handles options in second schema
+    expect(queryByRole('row', { name: 'Test 2 bar2' })).toBeInTheDocument();
+
+    // handles options for nested object in second schema
+    expect(queryByRole('row', { name: 'Test 4 bar4' })).toBeInTheDocument();
+
+    // handles options for property in dependencies in third schema
+    expect(queryByRole('row', { name: 'Test 5 bar5' })).toBeInTheDocument();
+  });
 });

plugins/scaffolder-react/src/next/components/ReviewState/ReviewState.tsx

@@ -18,7 +18,7 @@ import { StructuredMetadataTable } from '@backstage/core-components';
 import { JsonObject, JsonValue } from '@backstage/types';
 import { Draft07 as JSONSchema } from 'json-schema-library';
 import { ParsedTemplateSchema } from '../../hooks/useTemplateSchema';
-import { isJsonObject, formatKey } from './util';
+import { isJsonObject, formatKey, findSchemaForKey } from './util';
 
 /**
  * The props for the {@link ReviewState} component.
@@ -94,10 +94,10 @@ export const ReviewState = (props: ReviewStateProps) => {
   const reviewData = Object.fromEntries(
     Object.entries(props.formState)
       .flatMap(([key, value]) => {
-        for (const step of props.schemas) {
-          return processSchema(key, value, step, props.formState);
-        }
-        return [[key, value]];
+        const schema = findSchemaForKey(key, props.schemas, props.formState);
+        return schema
+          ? processSchema(key, value, schema, props.formState)
+          : [[key, value]];
       })
       .filter(prop => prop.length > 0),
   );

plugins/scaffolder-react/src/next/components/ReviewState/util.test.ts

@@ -14,7 +14,8 @@
  * limitations under the License.
  */
 
-import { isJsonObject, formatKey } from './util';
+import { isJsonObject, formatKey, findSchemaForKey } from './util';
+import { ParsedTemplateSchema } from '../../hooks/useTemplateSchema';
 
 describe('isJsonObject', () => {
   it('should return true for non-null objects', () => {
@@ -78,3 +79,95 @@ describe('formatKey', () => {
     expect(formatKey('parent/child@!#$%^&*()')).toBe('Parent > Child');
   });
 });
+
+describe('findSchemaForKey', () => {
+  const schemas: ParsedTemplateSchema[] = [
+    {
+      mergedSchema: {
+        type: 'object',
+        properties: {
+          foo: {
+            type: 'string',
+          },
+        },
+      },
+      schema: {},
+      title: 'Schema 1',
+      uiSchema: {},
+    },
+    {
+      mergedSchema: {
+        type: 'object',
+        properties: {
+          bar: {
+            type: 'string',
+          },
+          hello: {
+            type: 'object',
+            properties: {
+              world: {
+                type: 'string',
+              },
+            },
+          },
+        },
+      },
+      schema: {},
+      title: 'Schema 2',
+      uiSchema: {},
+    },
+    {
+      mergedSchema: {
+        type: 'object',
+        dependencies: {
+          foo: {
+            oneOf: [
+              {
+                properties: {
+                  artifact: {
+                    type: 'string',
+                  },
+                },
+              },
+            ],
+          },
+        },
+      },
+      schema: {},
+      title: 'Schema 3',
+      uiSchema: {},
+    },
+  ];
+
+  const formState = {
+    foo: 'value',
+    bar: 'value',
+    hello: { world: 'value' },
+    artifact: 'value',
+  };
+
+  it('should return the schema for the direct key', () => {
+    const result = findSchemaForKey('foo', schemas, formState);
+    expect(result).toBe(schemas[0]);
+  });
+
+  it('should return the schema for a key in dependencies', () => {
+    const result = findSchemaForKey('artifact', schemas, formState);
+    expect(result).toBe(schemas[2]);
+  });
+
+  it('should return null if the key does not exist', () => {
+    const result = findSchemaForKey('nonexistentKey', schemas, formState);
+    expect(result).toBeNull();
+  });
+
+  it('should return the schema for a key in the second schema', () => {
+    const result = findSchemaForKey('bar', schemas, formState);
+    expect(result).toBe(schemas[1]);
+  });
+
+  it('should return the schema for an object key', () => {
+    const result = findSchemaForKey('hello', schemas, formState);
+    expect(result).toBe(schemas[1]);
+  });
+});

plugins/scaffolder-react/src/next/components/ReviewState/util.ts

@@ -16,6 +16,8 @@
 
 import { JsonObject, JsonValue } from '@backstage/types';
 import { startCase } from 'lodash';
+import { ParsedTemplateSchema } from '../../hooks/useTemplateSchema';
+import { Draft07 as JSONSchema } from 'json-schema-library';
 
 export function isJsonObject(value?: JsonValue): value is JsonObject {
   return typeof value === 'object' && value !== null && !Array.isArray(value);
@@ -29,3 +31,46 @@ export function formatKey(key: string): string {
     .map(part => startCase(part))
     .join(' > ');
 }
+
+export function findSchemaForKey(
+  key: string,
+  schemas: ParsedTemplateSchema[],
+  formState: Partial<{ [key: string]: JsonValue }>,
+): ParsedTemplateSchema | null {
+  for (const step of schemas) {
+    /*
+      To determine if a key is defined in a schema we need to call getSchema
+      with an empty form state. Otherwise, it will never return undefined as it
+      will fallback to a default schema based on the form state. 
+
+      An exception to this is when your schema is dynamic i.e. using dependencies 
+      because the form state is required for generating the schema. In this case,
+      we add only the dependencies data to the getSchema call.
+    */
+
+    const schema = step.mergedSchema;
+
+    // Declare data to be a subset of formState
+    const data: typeof formState = {};
+
+    if (schema.dependencies && isJsonObject(schema.dependencies)) {
+      for (const dep in schema.dependencies) {
+        if (formState.hasOwnProperty(dep)) {
+          data[dep] = formState[dep]; // Add each dependency key from formState
+        }
+      }
+    }
+
+    const draft = new JSONSchema(schema);
+    const res = draft.getSchema({
+      pointer: `#/${key}`,
+      data,
+    });
+
+    if (!!res) {
+      return step;
+    }
+  }
+
+  return null; // Return null if the key isn't found in any schema
+}