diff --git a/.changeset/tame-hornets-shake.md b/.changeset/tame-hornets-shake.md new file mode 100644 index 0000000000..bf96193b55 --- /dev/null +++ b/.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 `>`. diff --git a/plugins/scaffolder-react/src/next/components/ReviewState/ReviewState.test.tsx b/plugins/scaffolder-react/src/next/components/ReviewState/ReviewState.test.tsx index 194c5f3e77..69b651e672 100644 --- a/plugins/scaffolder-react/src/next/components/ReviewState/ReviewState.test.tsx +++ b/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', () => { , ); - 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', () => { , ); - 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', () => { , ); - 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(); }); }); diff --git a/plugins/scaffolder-react/src/next/components/ReviewState/ReviewState.tsx b/plugins/scaffolder-react/src/next/components/ReviewState/ReviewState.tsx index 725db8bee1..fb6a592dce 100644 --- a/plugins/scaffolder-react/src/next/components/ReviewState/ReviewState.tsx +++ b/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 ; + const options = { + titleFormat: formatKey, + }; + return ; }; diff --git a/plugins/scaffolder-react/src/next/components/ReviewState/util.test.ts b/plugins/scaffolder-react/src/next/components/ReviewState/util.test.ts index 649d0c3160..cca4a211cf 100644 --- a/plugins/scaffolder-react/src/next/components/ReviewState/util.test.ts +++ b/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@!#$%^&*()', + ); }); }); diff --git a/plugins/scaffolder-react/src/next/components/ReviewState/util.ts b/plugins/scaffolder-react/src/next/components/ReviewState/util.ts index d2caf71964..417a0ec5b0 100644 --- a/plugins/scaffolder-react/src/next/components/ReviewState/util.ts +++ b/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(' > '); }