diff --git a/.changeset/nervous-suits-give.md b/.changeset/nervous-suits-give.md new file mode 100644 index 0000000000..e08d8d89a2 --- /dev/null +++ b/.changeset/nervous-suits-give.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-shortcuts': patch +--- + +Limit the use of the duplicate shortcut name when adding a shortcut diff --git a/plugins/shortcuts/src/AddShortcut.test.tsx b/plugins/shortcuts/src/AddShortcut.test.tsx index f8786e2b24..35f37bc265 100644 --- a/plugins/shortcuts/src/AddShortcut.test.tsx +++ b/plugins/shortcuts/src/AddShortcut.test.tsx @@ -17,7 +17,7 @@ import React from 'react'; import { screen, fireEvent, waitFor } from '@testing-library/react'; import { AddShortcut } from './AddShortcut'; -import { DefaultShortcutsApi } from './api'; +import { DefaultShortcutsApi, shortcutsApiRef } from './api'; import { MockAnalyticsApi, MockStorageApi, @@ -42,13 +42,29 @@ describe('AddShortcut', () => { }); it('displays the title', async () => { - await renderInTestApp(); + await renderInTestApp( + + + , + ); expect(screen.getByText('Add Shortcut')).toBeInTheDocument(); }); it('closes the popup', async () => { - await renderInTestApp(); + await renderInTestApp( + + + , + ); fireEvent.click(screen.getByText('Cancel')); expect(props.onClose).toHaveBeenCalledTimes(1); @@ -57,7 +73,15 @@ describe('AddShortcut', () => { it('saves the input', async () => { const spy = jest.spyOn(api, 'add'); - await renderInTestApp(); + await renderInTestApp( + + + , + ); const urlInput = screen.getByPlaceholderText('Enter a URL'); const titleInput = screen.getByPlaceholderText('Enter a display name'); @@ -78,7 +102,12 @@ describe('AddShortcut', () => { const spy = jest.spyOn(api, 'add'); await renderInTestApp( - + , ); @@ -106,9 +135,18 @@ describe('AddShortcut', () => { it('pastes the values', async () => { const spy = jest.spyOn(api, 'add'); - await renderInTestApp(, { - routeEntries: ['/some-initial-url'], - }); + await renderInTestApp( + + , + , + { + routeEntries: ['/some-initial-url'], + }, + ); fireEvent.click(screen.getByText('Use current page')); fireEvent.click(screen.getByText('Save')); @@ -125,8 +163,14 @@ describe('AddShortcut', () => { await renderInTestApp( <> - - + + + + , ); diff --git a/plugins/shortcuts/src/EditShortcut.test.tsx b/plugins/shortcuts/src/EditShortcut.test.tsx index 72a89c13b6..3077168193 100644 --- a/plugins/shortcuts/src/EditShortcut.test.tsx +++ b/plugins/shortcuts/src/EditShortcut.test.tsx @@ -18,7 +18,7 @@ import React from 'react'; import { screen, fireEvent, waitFor } from '@testing-library/react'; import { EditShortcut } from './EditShortcut'; import { Shortcut } from './types'; -import { DefaultShortcutsApi } from './api'; +import { DefaultShortcutsApi, shortcutsApiRef } from './api'; import { MockAnalyticsApi, MockStorageApi, @@ -48,13 +48,29 @@ describe('EditShortcut', () => { }); it('displays the title', async () => { - await renderInTestApp(); + await renderInTestApp( + + + , + ); expect(screen.getByText('Edit Shortcut')).toBeInTheDocument(); }); it('closes the popup', async () => { - await renderInTestApp(); + await renderInTestApp( + + + , + ); fireEvent.click(screen.getByText('Cancel')); expect(props.onClose).toHaveBeenCalledTimes(1); @@ -63,7 +79,15 @@ describe('EditShortcut', () => { it('updates the shortcut', async () => { const spy = jest.spyOn(api, 'update'); - await renderInTestApp(); + await renderInTestApp( + + + , + ); const urlInput = screen.getByPlaceholderText('Enter a URL'); const titleInput = screen.getByPlaceholderText('Enter a display name'); @@ -86,7 +110,12 @@ describe('EditShortcut', () => { const spy = jest.spyOn(api, 'update'); await renderInTestApp( - + , ); @@ -115,7 +144,15 @@ describe('EditShortcut', () => { it('removes the shortcut', async () => { const spy = jest.spyOn(api, 'remove'); - await renderInTestApp(); + await renderInTestApp( + + + , + ); fireEvent.click(screen.getByText('Remove')); expect(spy).toHaveBeenCalledWith('id'); @@ -132,8 +169,14 @@ describe('EditShortcut', () => { await renderInTestApp( <> - - + + + + , ); diff --git a/plugins/shortcuts/src/ShortcutForm.test.tsx b/plugins/shortcuts/src/ShortcutForm.test.tsx index 3a52356b5d..388c072725 100644 --- a/plugins/shortcuts/src/ShortcutForm.test.tsx +++ b/plugins/shortcuts/src/ShortcutForm.test.tsx @@ -16,7 +16,12 @@ import React from 'react'; import { screen, fireEvent, waitFor } from '@testing-library/react'; import { ShortcutForm } from './ShortcutForm'; -import { renderInTestApp } from '@backstage/test-utils'; +import { DefaultShortcutsApi, shortcutsApiRef } from './api'; +import { + renderInTestApp, + TestApiProvider, + MockStorageApi, +} from '@backstage/test-utils'; describe('ShortcutForm', () => { const props = { @@ -25,7 +30,15 @@ describe('ShortcutForm', () => { }; it('displays validation messages', async () => { - await renderInTestApp(); + await renderInTestApp( + + + , + ); const urlInput = screen.getByPlaceholderText('Enter a URL'); const titleInput = screen.getByPlaceholderText('Enter a display name'); @@ -43,8 +56,47 @@ describe('ShortcutForm', () => { }); }); + it('displays duplicate validation messages for title and URL', async () => { + const mockShortcutApi = new DefaultShortcutsApi(MockStorageApi.create()); + mockShortcutApi.add({ title: 'Existing Title', url: '/existing-url' }); + + await renderInTestApp( + + + , + ); + + const urlInput = screen.getByPlaceholderText('Enter a URL'); + const titleInput = screen.getByPlaceholderText('Enter a display name'); + + fireEvent.change(urlInput, { target: { value: '/existing-url' } }); + fireEvent.change(titleInput, { target: { value: 'Existing Title' } }); + + fireEvent.click(screen.getByText('Save')); + + await waitFor(() => { + expect( + screen.getByText('A shortcut with this title already exists'), + ).toBeInTheDocument(); + expect( + screen.getByText('A shortcut with this url already exists'), + ).toBeInTheDocument(); + }); + + expect(props.onSave).not.toHaveBeenCalled(); + expect(props.onClose).not.toHaveBeenCalled(); + }); + it('allows external links', async () => { - await renderInTestApp(); + await renderInTestApp( + + + , + ); const urlInput = screen.getByPlaceholderText('Enter a URL'); const titleInput = screen.getByPlaceholderText('Enter a display name'); @@ -66,7 +118,15 @@ describe('ShortcutForm', () => { }); it('allows relative links when external links are enabled', async () => { - await renderInTestApp(); + await renderInTestApp( + + + , + ); const urlInput = screen.getByPlaceholderText('Enter a URL'); const titleInput = screen.getByPlaceholderText('Enter a display name'); @@ -89,10 +149,17 @@ describe('ShortcutForm', () => { it('calls the save handler', async () => { await renderInTestApp( - , + + + , + , ); fireEvent.click(screen.getByText('Save')); @@ -105,7 +172,15 @@ describe('ShortcutForm', () => { }); it('calls the close handler', async () => { - await renderInTestApp(); + await renderInTestApp( + + + , + ); fireEvent.click(screen.getByText('Cancel')); await waitFor(() => { diff --git a/plugins/shortcuts/src/ShortcutForm.tsx b/plugins/shortcuts/src/ShortcutForm.tsx index 99aa0d29b7..e8cd8144d0 100644 --- a/plugins/shortcuts/src/ShortcutForm.tsx +++ b/plugins/shortcuts/src/ShortcutForm.tsx @@ -15,6 +15,7 @@ */ import React, { useEffect } from 'react'; +import useObservable from 'react-use/lib/useObservable'; import { useForm, SubmitHandler, Controller } from 'react-hook-form'; import { Button, @@ -24,6 +25,8 @@ import { TextField, } from '@material-ui/core'; import { FormValues } from './types'; +import { shortcutsApiRef } from './api'; +import { useApi } from '@backstage/core-plugin-api'; const useStyles = makeStyles(theme => ({ field: { @@ -50,6 +53,11 @@ export const ShortcutForm = ({ allowExternalLinks, }: Props) => { const classes = useStyles(); + const shortcutApi = useApi(shortcutsApiRef); + const shortcutData = useObservable( + shortcutApi.shortcut$(), + shortcutApi.get(), + ); const { handleSubmit, reset, @@ -63,6 +71,18 @@ export const ShortcutForm = ({ }, }); + const titleIsUnique = (title: string) => { + if (shortcutData.some(shortcutTitle => shortcutTitle.title === title)) + return 'A shortcut with this title already exists'; + return true; + }; + + const urlIsUnique = (url: string) => { + if (shortcutData.some(shortcutUrl => shortcutUrl.url === url)) + return 'A shortcut with this url already exists'; + return true; + }; + useEffect(() => { reset(formValues); }, [reset, formValues]); @@ -75,6 +95,7 @@ export const ShortcutForm = ({ control={control} rules={{ required: true, + validate: urlIsUnique, ...(allowExternalLinks ? { pattern: { @@ -112,6 +133,7 @@ export const ShortcutForm = ({ control={control} rules={{ required: true, + validate: titleIsUnique, minLength: { value: 2, message: 'Must be at least 2 characters',