Merge pull request #18804 from AmbrishRamachandiran/master
Limit the use of the same shortcut title and url when adding a shortcut
This commit is contained in:
@@ -0,0 +1,5 @@
|
||||
---
|
||||
'@backstage/plugin-shortcuts': patch
|
||||
---
|
||||
|
||||
Limit the use of the duplicate shortcut name when adding a shortcut
|
||||
@@ -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(<AddShortcut {...props} />);
|
||||
await renderInTestApp(
|
||||
<TestApiProvider
|
||||
apis={[
|
||||
[shortcutsApiRef, new DefaultShortcutsApi(MockStorageApi.create())],
|
||||
]}
|
||||
>
|
||||
<AddShortcut {...props} />
|
||||
</TestApiProvider>,
|
||||
);
|
||||
|
||||
expect(screen.getByText('Add Shortcut')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('closes the popup', async () => {
|
||||
await renderInTestApp(<AddShortcut {...props} />);
|
||||
await renderInTestApp(
|
||||
<TestApiProvider
|
||||
apis={[
|
||||
[shortcutsApiRef, new DefaultShortcutsApi(MockStorageApi.create())],
|
||||
]}
|
||||
>
|
||||
<AddShortcut {...props} />
|
||||
</TestApiProvider>,
|
||||
);
|
||||
|
||||
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(<AddShortcut {...props} />);
|
||||
await renderInTestApp(
|
||||
<TestApiProvider
|
||||
apis={[
|
||||
[shortcutsApiRef, new DefaultShortcutsApi(MockStorageApi.create())],
|
||||
]}
|
||||
>
|
||||
<AddShortcut {...props} />
|
||||
</TestApiProvider>,
|
||||
);
|
||||
|
||||
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(
|
||||
<TestApiProvider apis={[[analyticsApiRef, analyticsSpy]]}>
|
||||
<TestApiProvider
|
||||
apis={[
|
||||
[analyticsApiRef, analyticsSpy],
|
||||
[shortcutsApiRef, new DefaultShortcutsApi(MockStorageApi.create())],
|
||||
]}
|
||||
>
|
||||
<AddShortcut {...props} />
|
||||
</TestApiProvider>,
|
||||
);
|
||||
@@ -106,9 +135,18 @@ describe('AddShortcut', () => {
|
||||
it('pastes the values', async () => {
|
||||
const spy = jest.spyOn(api, 'add');
|
||||
|
||||
await renderInTestApp(<AddShortcut {...props} />, {
|
||||
routeEntries: ['/some-initial-url'],
|
||||
});
|
||||
await renderInTestApp(
|
||||
<TestApiProvider
|
||||
apis={[
|
||||
[shortcutsApiRef, new DefaultShortcutsApi(MockStorageApi.create())],
|
||||
]}
|
||||
>
|
||||
<AddShortcut {...props} />,
|
||||
</TestApiProvider>,
|
||||
{
|
||||
routeEntries: ['/some-initial-url'],
|
||||
},
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByText('Use current page'));
|
||||
fireEvent.click(screen.getByText('Save'));
|
||||
@@ -125,8 +163,14 @@ describe('AddShortcut', () => {
|
||||
|
||||
await renderInTestApp(
|
||||
<>
|
||||
<AlertDisplay />
|
||||
<AddShortcut {...props} />
|
||||
<TestApiProvider
|
||||
apis={[
|
||||
[shortcutsApiRef, new DefaultShortcutsApi(MockStorageApi.create())],
|
||||
]}
|
||||
>
|
||||
<AlertDisplay />
|
||||
<AddShortcut {...props} />
|
||||
</TestApiProvider>
|
||||
</>,
|
||||
);
|
||||
|
||||
|
||||
@@ -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(<EditShortcut {...props} />);
|
||||
await renderInTestApp(
|
||||
<TestApiProvider
|
||||
apis={[
|
||||
[shortcutsApiRef, new DefaultShortcutsApi(MockStorageApi.create())],
|
||||
]}
|
||||
>
|
||||
<EditShortcut {...props} />
|
||||
</TestApiProvider>,
|
||||
);
|
||||
|
||||
expect(screen.getByText('Edit Shortcut')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('closes the popup', async () => {
|
||||
await renderInTestApp(<EditShortcut {...props} />);
|
||||
await renderInTestApp(
|
||||
<TestApiProvider
|
||||
apis={[
|
||||
[shortcutsApiRef, new DefaultShortcutsApi(MockStorageApi.create())],
|
||||
]}
|
||||
>
|
||||
<EditShortcut {...props} />
|
||||
</TestApiProvider>,
|
||||
);
|
||||
|
||||
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(<EditShortcut {...props} />);
|
||||
await renderInTestApp(
|
||||
<TestApiProvider
|
||||
apis={[
|
||||
[shortcutsApiRef, new DefaultShortcutsApi(MockStorageApi.create())],
|
||||
]}
|
||||
>
|
||||
<EditShortcut {...props} />
|
||||
</TestApiProvider>,
|
||||
);
|
||||
|
||||
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(
|
||||
<TestApiProvider apis={[[analyticsApiRef, analyticsSpy]]}>
|
||||
<TestApiProvider
|
||||
apis={[
|
||||
[analyticsApiRef, analyticsSpy],
|
||||
[shortcutsApiRef, new DefaultShortcutsApi(MockStorageApi.create())],
|
||||
]}
|
||||
>
|
||||
<EditShortcut {...props} />
|
||||
</TestApiProvider>,
|
||||
);
|
||||
@@ -115,7 +144,15 @@ describe('EditShortcut', () => {
|
||||
it('removes the shortcut', async () => {
|
||||
const spy = jest.spyOn(api, 'remove');
|
||||
|
||||
await renderInTestApp(<EditShortcut {...props} />);
|
||||
await renderInTestApp(
|
||||
<TestApiProvider
|
||||
apis={[
|
||||
[shortcutsApiRef, new DefaultShortcutsApi(MockStorageApi.create())],
|
||||
]}
|
||||
>
|
||||
<EditShortcut {...props} />
|
||||
</TestApiProvider>,
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByText('Remove'));
|
||||
expect(spy).toHaveBeenCalledWith('id');
|
||||
@@ -132,8 +169,14 @@ describe('EditShortcut', () => {
|
||||
|
||||
await renderInTestApp(
|
||||
<>
|
||||
<AlertDisplay />
|
||||
<EditShortcut {...props} />
|
||||
<TestApiProvider
|
||||
apis={[
|
||||
[shortcutsApiRef, new DefaultShortcutsApi(MockStorageApi.create())],
|
||||
]}
|
||||
>
|
||||
<AlertDisplay />
|
||||
<EditShortcut {...props} />
|
||||
</TestApiProvider>
|
||||
</>,
|
||||
);
|
||||
|
||||
|
||||
@@ -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(<ShortcutForm {...props} />);
|
||||
await renderInTestApp(
|
||||
<TestApiProvider
|
||||
apis={[
|
||||
[shortcutsApiRef, new DefaultShortcutsApi(MockStorageApi.create())],
|
||||
]}
|
||||
>
|
||||
<ShortcutForm {...props} />
|
||||
</TestApiProvider>,
|
||||
);
|
||||
|
||||
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(
|
||||
<TestApiProvider apis={[[shortcutsApiRef, mockShortcutApi]]}>
|
||||
<ShortcutForm {...props} />
|
||||
</TestApiProvider>,
|
||||
);
|
||||
|
||||
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(<ShortcutForm allowExternalLinks {...props} />);
|
||||
await renderInTestApp(
|
||||
<TestApiProvider
|
||||
apis={[
|
||||
[shortcutsApiRef, new DefaultShortcutsApi(MockStorageApi.create())],
|
||||
]}
|
||||
>
|
||||
<ShortcutForm allowExternalLinks {...props} />
|
||||
</TestApiProvider>,
|
||||
);
|
||||
|
||||
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(<ShortcutForm allowExternalLinks {...props} />);
|
||||
await renderInTestApp(
|
||||
<TestApiProvider
|
||||
apis={[
|
||||
[shortcutsApiRef, new DefaultShortcutsApi(MockStorageApi.create())],
|
||||
]}
|
||||
>
|
||||
<ShortcutForm allowExternalLinks {...props} />
|
||||
</TestApiProvider>,
|
||||
);
|
||||
|
||||
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(
|
||||
<ShortcutForm
|
||||
{...props}
|
||||
formValues={{ url: '/some-url', title: 'some title' }}
|
||||
/>,
|
||||
<TestApiProvider
|
||||
apis={[
|
||||
[shortcutsApiRef, new DefaultShortcutsApi(MockStorageApi.create())],
|
||||
]}
|
||||
>
|
||||
<ShortcutForm
|
||||
{...props}
|
||||
formValues={{ url: '/some-url', title: 'some title' }}
|
||||
/>
|
||||
,
|
||||
</TestApiProvider>,
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByText('Save'));
|
||||
@@ -105,7 +172,15 @@ describe('ShortcutForm', () => {
|
||||
});
|
||||
|
||||
it('calls the close handler', async () => {
|
||||
await renderInTestApp(<ShortcutForm {...props} />);
|
||||
await renderInTestApp(
|
||||
<TestApiProvider
|
||||
apis={[
|
||||
[shortcutsApiRef, new DefaultShortcutsApi(MockStorageApi.create())],
|
||||
]}
|
||||
>
|
||||
<ShortcutForm {...props} />
|
||||
</TestApiProvider>,
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByText('Cancel'));
|
||||
await waitFor(() => {
|
||||
|
||||
@@ -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',
|
||||
|
||||
Reference in New Issue
Block a user