From b5ba33a9275076483fdfa6a39d42e86e022ce673 Mon Sep 17 00:00:00 2001 From: AmbrishRamachandiran Date: Thu, 17 Aug 2023 16:33:56 +0530 Subject: [PATCH 01/13] Limit the use of the same playlist name when adding a playlist Signed-off-by: AmbrishRamachandiran --- .changeset/swift-frogs-drop.md | 5 ++ .../PlaylistEditDialog.test.tsx | 82 ++++++++++++++++++- .../PlaylistEditDialog/PlaylistEditDialog.tsx | 32 +++++++- 3 files changed, 116 insertions(+), 3 deletions(-) create mode 100644 .changeset/swift-frogs-drop.md diff --git a/.changeset/swift-frogs-drop.md b/.changeset/swift-frogs-drop.md new file mode 100644 index 0000000000..3edff21ca2 --- /dev/null +++ b/.changeset/swift-frogs-drop.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-playlist': patch +--- + +Limit the use of the same playlist name when adding a playlist diff --git a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.test.tsx b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.test.tsx index 9f30d7608c..b01b5acbf2 100644 --- a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.test.tsx +++ b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.test.tsx @@ -18,11 +18,32 @@ import { IdentityApi, identityApiRef } from '@backstage/core-plugin-api'; import { renderInTestApp, TestApiProvider } from '@backstage/test-utils'; import { fireEvent, getByRole, waitFor } from '@testing-library/react'; import { act } from '@testing-library/react-hooks'; +import { PlaylistApi, playlistApiRef } from '../../api'; import React from 'react'; import { PlaylistEditDialog } from './PlaylistEditDialog'; describe('', () => { + const samplePlaylists = [ + { + id: 'id1', + name: 'playlist-1', + owner: 'group:default/some-owner', + public: true, + entities: 1, + followers: 2, + isFollowing: false, + }, + { + id: 'id2', + name: 'playlist-2', + owner: 'group:default/another-owner', + public: true, + entities: 2, + followers: 1, + isFollowing: true, + }, + ]; it('handle saving with an edited playlist', async () => { const identityApi: Partial = { getBackstageIdentity: async () => ({ @@ -33,8 +54,18 @@ describe('', () => { }; const mockOnSave = jest.fn().mockImplementation(async () => {}); + const playlistApi: Partial = { + getAllPlaylists: jest + .fn() + .mockImplementation(async () => samplePlaylists), + }; const rendered = await renderInTestApp( - + , ); @@ -83,4 +114,53 @@ describe('', () => { }); }); }); + + it('displays duplicate validation message for playlist name', async () => { + const identityApi: Partial = { + getBackstageIdentity: async () => ({ + type: 'user', + userEntityRef: 'user:default/me', + ownershipEntityRefs: ['group:default/test-owner', 'user:default/me'], + }), + }; + + const mockOnSave = jest.fn().mockImplementation(async () => {}); + const playlistApi: Partial = { + getAllPlaylists: jest + .fn() + .mockImplementation(async () => [...samplePlaylists]), + }; + + const rendered = await renderInTestApp( + + + , + ); + + act(() => { + fireEvent.input( + getByRole(rendered.getByTestId('edit-dialog-name-input'), 'textbox'), + { + target: { + value: 'playlist-1', + }, + }, + ); + + fireEvent.click(rendered.getByTestId('edit-dialog-save-button')); + }); + + await waitFor(() => { + expect( + rendered.getByText('A playlist with this name already exists'), + ).toBeInTheDocument(); + }); + + expect(mockOnSave).not.toHaveBeenCalled(); + }); }); diff --git a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx index a213c7c51c..67749042cb 100644 --- a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx +++ b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx @@ -18,6 +18,7 @@ import { parseEntityRef } from '@backstage/catalog-model'; import { identityApiRef, useApi } from '@backstage/core-plugin-api'; import { humanizeEntityRef } from '@backstage/plugin-catalog-react'; import { PlaylistMetadata } from '@backstage/plugin-playlist-common'; +import { playlistApiRef } from '../../api'; import { Button, CircularProgress, @@ -35,7 +36,7 @@ import { Select, TextField, } from '@material-ui/core'; -import React from 'react'; +import React, { useState } from 'react'; // Import useState import { useForm, Controller } from 'react-hook-form'; import useAsync from 'react-use/lib/useAsync'; import useAsyncFn from 'react-use/lib/useAsyncFn'; @@ -74,6 +75,15 @@ export const PlaylistEditDialog = ({ }: PlaylistEditDialogProps) => { const classes = useStyles(); const identityApi = useApi(identityApiRef); + const playlistApi = useApi(playlistApiRef); + const playListApiData = playlistApi.getAllPlaylists(); + + const [editingOtherFields, setEditingOtherFields] = useState(false); + + const fetchAndProcessData = async () => { + const playlistArray = await playListApiData; + return playlistArray; + }; const { loading: loadingOwnership, value: ownershipRefs } = useAsync(async () => { @@ -81,6 +91,18 @@ export const PlaylistEditDialog = ({ return ownershipEntityRefs; }, []); + const nameIsUnique = async (name: string) => { + const playlistArray = await fetchAndProcessData(); + if ( + editingOtherFields || + (await playlistArray.some( + (playlistData: { name: string }) => playlistData.name === name, + )) + ) + return 'A playlist with this name already exists'; + return true; + }; + const defaultValues = { ...playlist, public: playlist.public.toString(), @@ -103,6 +125,7 @@ export const PlaylistEditDialog = ({ if (!saving.loading) { onClose(); reset(defaultValues); + setEditingOtherFields(false); } }; @@ -117,13 +140,17 @@ export const PlaylistEditDialog = ({ ( setEditingOtherFields(true)} /> )} /> From 4892d7d6584f22d78b8978c7f2dbed4a2dda90b6 Mon Sep 17 00:00:00 2001 From: AmbrishRamachandiran Date: Thu, 17 Aug 2023 16:44:04 +0530 Subject: [PATCH 02/13] Limit the use of the same playlist name when adding a playlist Signed-off-by: AmbrishRamachandiran --- .../PlaylistPage/PlaylistHeader.tsx | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/plugins/playlist/src/components/PlaylistPage/PlaylistHeader.tsx b/plugins/playlist/src/components/PlaylistPage/PlaylistHeader.tsx index f892e8de7f..13a802f9a8 100644 --- a/plugins/playlist/src/components/PlaylistPage/PlaylistHeader.tsx +++ b/plugins/playlist/src/components/PlaylistPage/PlaylistHeader.tsx @@ -43,7 +43,7 @@ import { } from '@material-ui/core'; import EditIcon from '@material-ui/icons/Edit'; import DeleteIcon from '@material-ui/icons/Delete'; -import React, { useCallback, useState } from 'react'; +import React, { useCallback, useState, useEffect } from 'react'; import { useNavigate } from 'react-router-dom'; import useAsyncFn from 'react-use/lib/useAsyncFn'; @@ -83,6 +83,7 @@ export const PlaylistHeader = ({ playlist, onUpdate }: PlaylistHeaderProps) => { const rootRoute = useRouteRef(rootRouteRef); const [openEditDialog, setOpenEditDialog] = useState(false); const [openDeleteDialog, setOpenDeleteDialog] = useState(false); + const [popupMessage, setPopupMessage] = useState(''); const { allowed: editAllowed } = usePermission({ permission: permissions.playlistListUpdate, @@ -94,22 +95,35 @@ export const PlaylistHeader = ({ playlist, onUpdate }: PlaylistHeaderProps) => { resourceRef: playlist.id, }); + useEffect(() => { + if (popupMessage) { + alertApi.post({ + message: popupMessage, + severity: 'success', + display: 'transient', + }); + setPopupMessage(''); + } + }, [popupMessage, alertApi]); + const updatePlaylist = useCallback( async (update: Omit) => { try { await playlistApi.updatePlaylist({ ...update, id: playlist.id }); setOpenEditDialog(false); + if (update.name !== playlist.name) { + setPopupMessage( + `Updated playlist name '${playlist.name}' to '${update.name}'`, + ); + } else { + setPopupMessage(`Updated playlist '${playlist.name}'`); + } onUpdate(); - alertApi.post({ - message: `Updated playlist '${playlist.name}'`, - severity: 'success', - display: 'transient', - }); } catch (e) { errorApi.post(e); } }, - [errorApi, onUpdate, playlist, playlistApi, alertApi], + [errorApi, onUpdate, playlist, playlistApi, setPopupMessage], ); const [deleting, deletePlaylist] = useAsyncFn(async () => { From d1d11827414cbc7e4649c606f3a7214dd68a0321 Mon Sep 17 00:00:00 2001 From: AmbrishRamachandiran Date: Thu, 17 Aug 2023 16:52:33 +0530 Subject: [PATCH 03/13] Limit the use of the same playlist name when adding a playlist Signed-off-by: AmbrishRamachandiran --- .../src/components/PlaylistEditDialog/PlaylistEditDialog.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx index 67749042cb..526f7dbe58 100644 --- a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx +++ b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx @@ -76,7 +76,7 @@ export const PlaylistEditDialog = ({ const classes = useStyles(); const identityApi = useApi(identityApiRef); const playlistApi = useApi(playlistApiRef); - const playListApiData = playlistApi.getAllPlaylists(); + const playListApiData = playlistApi.getAllPlaylists({ editable: true }); const [editingOtherFields, setEditingOtherFields] = useState(false); From 5e0e4975466f73eed240c06686c8246cb247cc67 Mon Sep 17 00:00:00 2001 From: AmbrishRamachandiran Date: Thu, 17 Aug 2023 20:27:38 +0530 Subject: [PATCH 04/13] fixed issues of edit and create playlist popup custom validations Signed-off-by: AmbrishRamachandiran --- .../PlaylistEditDialog/PlaylistEditDialog.tsx | 37 ++++++++++++++----- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx index 526f7dbe58..5cee8bf531 100644 --- a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx +++ b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx @@ -91,15 +91,31 @@ export const PlaylistEditDialog = ({ return ownershipEntityRefs; }, []); - const nameIsUnique = async (name: string) => { - const playlistArray = await fetchAndProcessData(); - if ( - editingOtherFields || - (await playlistArray.some( - (playlistData: { name: string }) => playlistData.name === name, - )) - ) - return 'A playlist with this name already exists'; + const nameIsUnique = async ( + name: string, + isEditing: boolean, + originalName: string, + ) => { + if (!isEditing) { + const playlistArray = await fetchAndProcessData(); + if ( + playlistArray.some( + (playlistData: { name: string }) => playlistData.name === name, + ) + ) { + return 'A playlist with this name already exists'; + } + } else if (name !== originalName) { + const playlistArray = await fetchAndProcessData(); + if ( + playlistArray.some( + (playlistData: { name: string }) => playlistData.name === name, + ) + ) { + return 'A playlist with this name already exists'; + } + } + return true; }; @@ -142,7 +158,8 @@ export const PlaylistEditDialog = ({ control={control} rules={{ required: true, - validate: editingOtherFields ? undefined : nameIsUnique, + validate: value => + nameIsUnique(value, editingOtherFields, playlist.name), }} render={({ field }) => ( Date: Thu, 17 Aug 2023 21:52:08 +0530 Subject: [PATCH 05/13] fixed issues of edit and create playlist popup custom validations Signed-off-by: AmbrishRamachandiran --- .../src/components/PlaylistEditDialog/PlaylistEditDialog.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx index 5cee8bf531..7ca5bfd6f6 100644 --- a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx +++ b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx @@ -36,7 +36,7 @@ import { Select, TextField, } from '@material-ui/core'; -import React, { useState } from 'react'; // Import useState +import React, { useState } from 'react'; import { useForm, Controller } from 'react-hook-form'; import useAsync from 'react-use/lib/useAsync'; import useAsyncFn from 'react-use/lib/useAsyncFn'; From 3e6b36fb3ea30330a82ae0f4adc50e11873e2be8 Mon Sep 17 00:00:00 2001 From: AmbrishRamachandiran Date: Fri, 18 Aug 2023 19:39:39 +0530 Subject: [PATCH 06/13] Added changes as per maintaners review Signed-off-by: AmbrishRamachandiran --- .../PlaylistEditDialog/PlaylistEditDialog.tsx | 39 +++++++------------ .../PlaylistPage/PlaylistHeader.tsx | 38 +++++++++--------- 2 files changed, 32 insertions(+), 45 deletions(-) diff --git a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx index 7ca5bfd6f6..49b4122e13 100644 --- a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx +++ b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx @@ -36,7 +36,7 @@ import { Select, TextField, } from '@material-ui/core'; -import React, { useState } from 'react'; +import React, { useEffect, useState } from 'react'; import { useForm, Controller } from 'react-hook-form'; import useAsync from 'react-use/lib/useAsync'; import useAsyncFn from 'react-use/lib/useAsyncFn'; @@ -76,14 +76,17 @@ export const PlaylistEditDialog = ({ const classes = useStyles(); const identityApi = useApi(identityApiRef); const playlistApi = useApi(playlistApiRef); - const playListApiData = playlistApi.getAllPlaylists({ editable: true }); - + const [playlistArray, setPlaylistArray] = useState([]); const [editingOtherFields, setEditingOtherFields] = useState(false); - const fetchAndProcessData = async () => { - const playlistArray = await playListApiData; - return playlistArray; - }; + useEffect(() => { + const fetchPlaylists = async () => { + const playlists = await playlistApi.getAllPlaylists({ editable: true }); + setPlaylistArray(playlists); + }; + + fetchPlaylists(); + }, [playlistApi]); const { loading: loadingOwnership, value: ownershipRefs } = useAsync(async () => { @@ -96,24 +99,10 @@ export const PlaylistEditDialog = ({ isEditing: boolean, originalName: string, ) => { - if (!isEditing) { - const playlistArray = await fetchAndProcessData(); - if ( - playlistArray.some( - (playlistData: { name: string }) => playlistData.name === name, - ) - ) { - return 'A playlist with this name already exists'; - } - } else if (name !== originalName) { - const playlistArray = await fetchAndProcessData(); - if ( - playlistArray.some( - (playlistData: { name: string }) => playlistData.name === name, - ) - ) { - return 'A playlist with this name already exists'; - } + if (!isEditing || name !== originalName) { + return playlistArray.some(p => p.name === name) + ? 'A playlist with this name already exists' + : true; } return true; diff --git a/plugins/playlist/src/components/PlaylistPage/PlaylistHeader.tsx b/plugins/playlist/src/components/PlaylistPage/PlaylistHeader.tsx index 13a802f9a8..8a970ba5ac 100644 --- a/plugins/playlist/src/components/PlaylistPage/PlaylistHeader.tsx +++ b/plugins/playlist/src/components/PlaylistPage/PlaylistHeader.tsx @@ -43,7 +43,7 @@ import { } from '@material-ui/core'; import EditIcon from '@material-ui/icons/Edit'; import DeleteIcon from '@material-ui/icons/Delete'; -import React, { useCallback, useState, useEffect } from 'react'; +import React, { useCallback, useEffect, useState } from 'react'; import { useNavigate } from 'react-router-dom'; import useAsyncFn from 'react-use/lib/useAsyncFn'; @@ -81,9 +81,9 @@ export const PlaylistHeader = ({ playlist, onUpdate }: PlaylistHeaderProps) => { const playlistApi = useApi(playlistApiRef); const navigate = useNavigate(); const rootRoute = useRouteRef(rootRouteRef); + const [openEditDialog, setOpenEditDialog] = useState(false); const [openDeleteDialog, setOpenDeleteDialog] = useState(false); - const [popupMessage, setPopupMessage] = useState(''); const { allowed: editAllowed } = usePermission({ permission: permissions.playlistListUpdate, @@ -95,50 +95,48 @@ export const PlaylistHeader = ({ playlist, onUpdate }: PlaylistHeaderProps) => { resourceRef: playlist.id, }); - useEffect(() => { - if (popupMessage) { - alertApi.post({ - message: popupMessage, - severity: 'success', - display: 'transient', - }); - setPopupMessage(''); - } - }, [popupMessage, alertApi]); - const updatePlaylist = useCallback( async (update: Omit) => { try { await playlistApi.updatePlaylist({ ...update, id: playlist.id }); setOpenEditDialog(false); if (update.name !== playlist.name) { - setPopupMessage( - `Updated playlist name '${playlist.name}' to '${update.name}'`, - ); + const message = `Updated playlist name '${playlist.name}' to '${update.name}'`; + alertApi.post({ + message, + severity: 'success', + display: 'transient', + }); } else { - setPopupMessage(`Updated playlist '${playlist.name}'`); + const message = `Updated playlist '${playlist.name}'`; + alertApi.post({ + message, + severity: 'success', + display: 'transient', + }); } onUpdate(); } catch (e) { errorApi.post(e); } }, - [errorApi, onUpdate, playlist, playlistApi, setPopupMessage], + [errorApi, onUpdate, playlist, playlistApi, alertApi], ); const [deleting, deletePlaylist] = useAsyncFn(async () => { try { await playlistApi.deletePlaylist(playlist.id); navigate(rootRoute()); + const message = `Deleted playlist '${playlist.name}'`; alertApi.post({ - message: `Deleted playlist '${playlist.name}'`, + message, severity: 'success', display: 'transient', }); } catch (e) { errorApi.post(e); } - }, [playlistApi]); + }, [playlistApi, alertApi]); const singularTitle = useTitle({ pluralize: false, From be681bd7b5e96dcb1e1c4f187cf43f257d833eeb Mon Sep 17 00:00:00 2001 From: AmbrishRamachandiran Date: Fri, 18 Aug 2023 20:01:07 +0530 Subject: [PATCH 07/13] Added changes as per maintaners review Signed-off-by: AmbrishRamachandiran --- .../src/components/PlaylistEditDialog/PlaylistEditDialog.tsx | 2 +- plugins/playlist/src/components/PlaylistPage/PlaylistHeader.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx index 49b4122e13..4f3415445f 100644 --- a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx +++ b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx @@ -76,7 +76,7 @@ export const PlaylistEditDialog = ({ const classes = useStyles(); const identityApi = useApi(identityApiRef); const playlistApi = useApi(playlistApiRef); - const [playlistArray, setPlaylistArray] = useState([]); + const [playlistArray, setPlaylistArray] = useState([]); const [editingOtherFields, setEditingOtherFields] = useState(false); useEffect(() => { diff --git a/plugins/playlist/src/components/PlaylistPage/PlaylistHeader.tsx b/plugins/playlist/src/components/PlaylistPage/PlaylistHeader.tsx index 8a970ba5ac..0aa473ee6a 100644 --- a/plugins/playlist/src/components/PlaylistPage/PlaylistHeader.tsx +++ b/plugins/playlist/src/components/PlaylistPage/PlaylistHeader.tsx @@ -43,7 +43,7 @@ import { } from '@material-ui/core'; import EditIcon from '@material-ui/icons/Edit'; import DeleteIcon from '@material-ui/icons/Delete'; -import React, { useCallback, useEffect, useState } from 'react'; +import React, { useCallback, useState } from 'react'; import { useNavigate } from 'react-router-dom'; import useAsyncFn from 'react-use/lib/useAsyncFn'; From d34f809ff6a42c6cefbc2d013542999bdde28579 Mon Sep 17 00:00:00 2001 From: AmbrishRamachandiran Date: Mon, 21 Aug 2023 15:23:10 +0530 Subject: [PATCH 08/13] Review changes done added useref Signed-off-by: AmbrishRamachandiran --- .../PlaylistEditDialog/PlaylistEditDialog.tsx | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx index 4f3415445f..3d9799388d 100644 --- a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx +++ b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx @@ -36,7 +36,7 @@ import { Select, TextField, } from '@material-ui/core'; -import React, { useEffect, useState } from 'react'; +import React, { useState, useRef } from 'react'; import { useForm, Controller } from 'react-hook-form'; import useAsync from 'react-use/lib/useAsync'; import useAsyncFn from 'react-use/lib/useAsyncFn'; @@ -76,31 +76,25 @@ export const PlaylistEditDialog = ({ const classes = useStyles(); const identityApi = useApi(identityApiRef); const playlistApi = useApi(playlistApiRef); - const [playlistArray, setPlaylistArray] = useState([]); const [editingOtherFields, setEditingOtherFields] = useState(false); - - useEffect(() => { - const fetchPlaylists = async () => { - const playlists = await playlistApi.getAllPlaylists({ editable: true }); - setPlaylistArray(playlists); - }; - - fetchPlaylists(); - }, [playlistApi]); - const { loading: loadingOwnership, value: ownershipRefs } = useAsync(async () => { const { ownershipEntityRefs } = await identityApi.getBackstageIdentity(); return ownershipEntityRefs; }, []); + const playlistPromise = useRef( + playlistApi.getAllPlaylists({ editable: true }), + ); const nameIsUnique = async ( name: string, isEditing: boolean, originalName: string, ) => { + const playlists = await playlistPromise.current; + if (!isEditing || name !== originalName) { - return playlistArray.some(p => p.name === name) + return playlists.some(p => p.name === name) ? 'A playlist with this name already exists' : true; } From bedea3cae182702d4f7dbc2b1f65997f58349fe4 Mon Sep 17 00:00:00 2001 From: AmbrishRamachandiran Date: Mon, 21 Aug 2023 15:32:43 +0530 Subject: [PATCH 09/13] Review changes done added useref Signed-off-by: AmbrishRamachandiran --- .../components/PlaylistEditDialog/PlaylistEditDialog.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx index 3d9799388d..50274a5b8c 100644 --- a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx +++ b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx @@ -76,15 +76,15 @@ export const PlaylistEditDialog = ({ const classes = useStyles(); const identityApi = useApi(identityApiRef); const playlistApi = useApi(playlistApiRef); + const playlistPromise = useRef( + playlistApi.getAllPlaylists({ editable: true }), + ); const [editingOtherFields, setEditingOtherFields] = useState(false); const { loading: loadingOwnership, value: ownershipRefs } = useAsync(async () => { const { ownershipEntityRefs } = await identityApi.getBackstageIdentity(); return ownershipEntityRefs; }, []); - const playlistPromise = useRef( - playlistApi.getAllPlaylists({ editable: true }), - ); const nameIsUnique = async ( name: string, From 0357fee6d523657549818966ae6097ed81897a58 Mon Sep 17 00:00:00 2001 From: AmbrishRamachandiran Date: Tue, 22 Aug 2023 07:52:29 +0530 Subject: [PATCH 10/13] Review changes done reduce code size Signed-off-by: AmbrishRamachandiran --- .../PlaylistEditDialog/PlaylistEditDialog.tsx | 22 +++++++------------ .../PlaylistPage/PlaylistHeader.tsx | 21 +++++++----------- 2 files changed, 16 insertions(+), 27 deletions(-) diff --git a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx index 50274a5b8c..7eb44403e3 100644 --- a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx +++ b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx @@ -76,9 +76,7 @@ export const PlaylistEditDialog = ({ const classes = useStyles(); const identityApi = useApi(identityApiRef); const playlistApi = useApi(playlistApiRef); - const playlistPromise = useRef( - playlistApi.getAllPlaylists({ editable: true }), - ); + const playlistPromise = useRef(playlistApi.getAllPlaylists()); const [editingOtherFields, setEditingOtherFields] = useState(false); const { loading: loadingOwnership, value: ownershipRefs } = useAsync(async () => { @@ -86,17 +84,14 @@ export const PlaylistEditDialog = ({ return ownershipEntityRefs; }, []); - const nameIsUnique = async ( - name: string, - isEditing: boolean, - originalName: string, - ) => { + const nameIsUnique = async (name: string, isEditing: boolean) => { const playlists = await playlistPromise.current; - if (!isEditing || name !== originalName) { - return playlists.some(p => p.name === name) - ? 'A playlist with this name already exists' - : true; + if (!isEditing || name !== playlist.name) { + return ( + !playlists.some(p => p.name === name) || + 'A playlist with this name already exists' + ); } return true; @@ -141,8 +136,7 @@ export const PlaylistEditDialog = ({ control={control} rules={{ required: true, - validate: value => - nameIsUnique(value, editingOtherFields, playlist.name), + validate: value => nameIsUnique(value, editingOtherFields), }} render={({ field }) => ( { try { await playlistApi.updatePlaylist({ ...update, id: playlist.id }); setOpenEditDialog(false); + let message = `Updated playlist '${playlist.name}'`; if (update.name !== playlist.name) { - const message = `Updated playlist name '${playlist.name}' to '${update.name}'`; - alertApi.post({ - message, - severity: 'success', - display: 'transient', - }); - } else { - const message = `Updated playlist '${playlist.name}'`; - alertApi.post({ - message, - severity: 'success', - display: 'transient', - }); + message = `Updated playlist name '${playlist.name}' to '${update.name}'`; } + + alertApi.post({ + message, + severity: 'success', + display: 'transient', + }); onUpdate(); } catch (e) { errorApi.post(e); From ba17063261424513fb2786c4e7a156157cb9a958 Mon Sep 17 00:00:00 2001 From: AmbrishRamachandiran Date: Tue, 22 Aug 2023 08:01:32 +0530 Subject: [PATCH 11/13] Review changes done reduce code size Signed-off-by: AmbrishRamachandiran --- .../src/components/PlaylistEditDialog/PlaylistEditDialog.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx index 7eb44403e3..628eda5ecc 100644 --- a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx +++ b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx @@ -76,7 +76,7 @@ export const PlaylistEditDialog = ({ const classes = useStyles(); const identityApi = useApi(identityApiRef); const playlistApi = useApi(playlistApiRef); - const playlistPromise = useRef(playlistApi.getAllPlaylists()); + const playlistPromise = useRef(playlistApi.getAllPlaylists({})); const [editingOtherFields, setEditingOtherFields] = useState(false); const { loading: loadingOwnership, value: ownershipRefs } = useAsync(async () => { From f45e3f585b7b11774f6f7a19f8e51358c1d3aea8 Mon Sep 17 00:00:00 2001 From: AmbrishRamachandiran Date: Tue, 22 Aug 2023 20:23:53 +0530 Subject: [PATCH 12/13] remove isediting param Signed-off-by: AmbrishRamachandiran --- .../components/PlaylistEditDialog/PlaylistEditDialog.tsx | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx index 628eda5ecc..06f6bcdbee 100644 --- a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx +++ b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx @@ -77,17 +77,16 @@ export const PlaylistEditDialog = ({ const identityApi = useApi(identityApiRef); const playlistApi = useApi(playlistApiRef); const playlistPromise = useRef(playlistApi.getAllPlaylists({})); - const [editingOtherFields, setEditingOtherFields] = useState(false); const { loading: loadingOwnership, value: ownershipRefs } = useAsync(async () => { const { ownershipEntityRefs } = await identityApi.getBackstageIdentity(); return ownershipEntityRefs; }, []); - const nameIsUnique = async (name: string, isEditing: boolean) => { + const nameIsUnique = async (name: string) => { const playlists = await playlistPromise.current; - if (!isEditing || name !== playlist.name) { + if (name !== playlist.name) { return ( !playlists.some(p => p.name === name) || 'A playlist with this name already exists' @@ -119,7 +118,6 @@ export const PlaylistEditDialog = ({ if (!saving.loading) { onClose(); reset(defaultValues); - setEditingOtherFields(false); } }; @@ -136,7 +134,7 @@ export const PlaylistEditDialog = ({ control={control} rules={{ required: true, - validate: value => nameIsUnique(value, editingOtherFields), + validate: value => nameIsUnique(value), }} render={({ field }) => ( setEditingOtherFields(true)} /> )} /> From 2d2cb350da0bbe63a82453c4817e8125cf5dd556 Mon Sep 17 00:00:00 2001 From: AmbrishRamachandiran Date: Tue, 22 Aug 2023 20:24:39 +0530 Subject: [PATCH 13/13] remove isediting param Signed-off-by: AmbrishRamachandiran --- .../src/components/PlaylistEditDialog/PlaylistEditDialog.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx index 06f6bcdbee..11f972afbe 100644 --- a/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx +++ b/plugins/playlist/src/components/PlaylistEditDialog/PlaylistEditDialog.tsx @@ -36,7 +36,7 @@ import { Select, TextField, } from '@material-ui/core'; -import React, { useState, useRef } from 'react'; +import React, { useRef } from 'react'; import { useForm, Controller } from 'react-hook-form'; import useAsync from 'react-use/lib/useAsync'; import useAsyncFn from 'react-use/lib/useAsyncFn';