fix(#26139): handle duplicate end key parts in ReviewState table

Signed-off-by: Rich Barton-Cooper <rich@syntasso.io>

.changeset/tame-hornets-shake.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-scaffolder-react': patch
+---
+
+Fix an issue where keys with duplicate final key parts are not all displayed in the `ReviewState`. Change the way the keys are formatted to include the full schema path, separated by `>`.

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

@@ -170,7 +170,8 @@ describe('ReviewState', () => {
     );
 
     expect(getByRole('row', { name: 'Name lols' })).toBeInTheDocument();
-    expect(getByRole('row', { name: 'Foo lols' })).toBeInTheDocument();
+    expect(getByRole('row', { name: 'Test bob' })).toBeInTheDocument();
+    expect(getByRole('row', { name: 'Nest > Foo lols' })).toBeInTheDocument();
   });
 
   it('should display enum label from enumNames', async () => {
@@ -219,7 +220,9 @@ describe('ReviewState', () => {
     expect(
       queryByRole('row', { name: 'Name Label-type2' }),
     ).toBeInTheDocument();
-    expect(queryByRole('row', { name: 'Foo Label-type2' })).toBeInTheDocument();
+    expect(
+      queryByRole('row', { name: 'Nest > Foo Label-type2' }),
+    ).toBeInTheDocument();
   });
 
   it('should display enum value if no corresponding enumNames', async () => {
@@ -292,8 +295,12 @@ describe('ReviewState', () => {
       <ReviewState formState={formState} schemas={schemas} />,
     );
 
-    expect(queryByRole('row', { name: 'Foo type3' })).toBeInTheDocument();
-    expect(queryByRole('row', { name: 'Bar type4' })).toBeInTheDocument();
+    expect(
+      queryByRole('row', { name: 'Name > Foo type3' }),
+    ).toBeInTheDocument();
+    expect(
+      queryByRole('row', { name: 'Name > Bar type4' }),
+    ).toBeInTheDocument();
   });
 
   it('should display nested objects in separate rows', async () => {
@@ -303,6 +310,7 @@ describe('ReviewState', () => {
         bar: 'type4',
         example: {
           test: 'type6',
+          foo: 'type7',
         },
       },
     };
@@ -329,6 +337,9 @@ describe('ReviewState', () => {
                     test: {
                       type: 'string',
                     },
+                    foo: {
+                      type: 'string',
+                    },
                   },
                 },
               },
@@ -345,9 +356,18 @@ describe('ReviewState', () => {
       <ReviewState formState={formState} schemas={schemas} />,
     );
 
-    expect(queryByRole('row', { name: 'Foo type3' })).toBeInTheDocument();
-    expect(queryByRole('row', { name: 'Bar type4' })).toBeInTheDocument();
-    expect(queryByRole('row', { name: 'Test type6' })).toBeInTheDocument();
+    expect(
+      queryByRole('row', { name: 'Name > Foo type3' }),
+    ).toBeInTheDocument();
+    expect(
+      queryByRole('row', { name: 'Name > Bar type4' }),
+    ).toBeInTheDocument();
+    expect(
+      queryByRole('row', { name: 'Name > Example > Test type6' }),
+    ).toBeInTheDocument();
+    expect(
+      queryByRole('row', { name: 'Name > Example > Foo type7' }),
+    ).toBeInTheDocument();
   });
 
   it('should display partially nested objects', async () => {
@@ -404,8 +424,14 @@ describe('ReviewState', () => {
       <ReviewState formState={formState} schemas={schemas} />,
     );
 
-    expect(queryByRole('row', { name: 'Foo type3' })).toBeInTheDocument();
-    expect(queryByRole('row', { name: 'Bar type4' })).toBeInTheDocument();
-    expect(queryByRole('row', { name: 'Test type6' })).not.toBeInTheDocument();
+    expect(
+      queryByRole('row', { name: 'Name > Foo type3' }),
+    ).toBeInTheDocument();
+    expect(
+      queryByRole('row', { name: 'Name > Bar type4' }),
+    ).toBeInTheDocument();
+    expect(
+      queryByRole('row', { name: 'Name > Example > Test type6' }),
+    ).not.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, getLastKey } from './util';
+import { isJsonObject, formatKey } from './util';
 
 /**
  * The props for the {@link ReviewState} component.
@@ -45,7 +45,7 @@ function processSchema(
     const backstageReviewOptions = definitionInSchema['ui:backstage']?.review;
     if (backstageReviewOptions) {
       if (backstageReviewOptions.mask) {
-        return [[getLastKey(key), backstageReviewOptions.mask]];
+        return [[key, backstageReviewOptions.mask]];
       }
       if (backstageReviewOptions.show === false) {
         return [];
@@ -56,13 +56,13 @@ function processSchema(
       definitionInSchema['ui:widget'] === 'password' ||
       definitionInSchema['ui:field']?.toLocaleLowerCase('en-us') === 'secret'
     ) {
-      return [[getLastKey(key), '******']];
+      return [[key, '******']];
     }
 
     if (definitionInSchema.enum && definitionInSchema.enumNames) {
       return [
         [
-          getLastKey(key),
+          key,
           definitionInSchema.enumNames[
             definitionInSchema.enum.indexOf(value)
           ] || value,
@@ -78,7 +78,7 @@ function processSchema(
     }
   }
 
-  return [[getLastKey(key), value]];
+  return [[key, value]];
 }
 
 /**
@@ -96,5 +96,8 @@ export const ReviewState = (props: ReviewStateProps) => {
       })
       .filter(prop => prop.length > 0),
   );
-  return <StructuredMetadataTable metadata={reviewData} />;
+  const options = {
+    titleFormat: formatKey,
+  };
+  return <StructuredMetadataTable metadata={reviewData} options={options} />;
 };

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

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { isJsonObject, getLastKey } from './util';
+import { isJsonObject, formatKey } from './util';
 
 describe('isJsonObject', () => {
   it('should return true for non-null objects', () => {
@@ -39,40 +39,44 @@ describe('isJsonObject', () => {
   });
 });
 
-describe('getLastKey', () => {
-  it('should return the last part of a simple key', () => {
-    expect(getLastKey('simple')).toBe('simple');
+describe('formatKey', () => {
+  it('should replace / with > globally in the key', () => {
+    expect(formatKey('simple/key')).toBe('Simple > Key');
   });
 
-  it('should return the last part of a nested key', () => {
-    expect(getLastKey('parent/child')).toBe('child');
+  it('should leave a top-level key untouched', () => {
+    expect(formatKey('topLevel')).toBe('TopLevel');
   });
 
-  it('should return the last part of a deeply nested key', () => {
-    expect(getLastKey('grandparent/parent/child')).toBe('child');
+  it('should handle keys with a leading slash', () => {
+    expect(formatKey('/simple/key')).toBe('Simple > Key');
   });
 
   it('should handle keys with trailing slash', () => {
-    expect(getLastKey('parent/child/')).toBe('');
+    expect(formatKey('parent/child/')).toBe('Parent > Child');
   });
 
   it('should handle empty string', () => {
-    expect(getLastKey('')).toBe('');
+    expect(formatKey('')).toBe('');
   });
 
   it('should handle keys with multiple consecutive slashes', () => {
-    expect(getLastKey('parent//child')).toBe('child');
+    expect(formatKey('parent//child')).toBe('Parent > Child');
   });
 
   it('should handle keys with only slashes', () => {
-    expect(getLastKey('////')).toBe('');
+    expect(formatKey('////')).toBe('');
   });
 
   it('should handle keys with spaces', () => {
-    expect(getLastKey('parent/child with spaces')).toBe('child with spaces');
+    expect(formatKey('parent/child with spaces')).toBe(
+      'Parent > Child with spaces',
+    );
   });
 
   it('should handle keys with special characters', () => {
-    expect(getLastKey('parent/child@!#$%^&*()')).toBe('child@!#$%^&*()');
+    expect(formatKey('parent/child@!#$%^&*()')).toBe(
+      'Parent > Child@!#$%^&*()',
+    );
   });
 });

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

@@ -20,8 +20,11 @@ export function isJsonObject(value?: JsonValue): value is JsonObject {
   return typeof value === 'object' && value !== null && !Array.isArray(value);
 }
 
-// Helper function to get the last part of the key
-export function getLastKey(key: string): string {
+// Helper function to format a key into a human-readable string
+export function formatKey(key: string): string {
   const parts = key.split('/');
-  return parts[parts.length - 1];
+  return parts
+    .filter(Boolean)
+    .map(part => part.charAt(0).toLocaleUpperCase('en-US') + part.slice(1))
+    .join(' > ');
 }