From 39b731441f3fb73f3150cf5709dde182ba464fbc Mon Sep 17 00:00:00 2001 From: Kim Lan Phan Hoang Date: Tue, 21 Nov 2023 11:22:08 +0100 Subject: [PATCH] feat: prevent move on shared page (#855) --- cypress/e2e/item/move/listMoveItem.cy.ts | 27 +------------- cypress/e2e/item/shared/sharedItems.cy.ts | 36 +++++++++++++++++++ cypress/fixtures/items.ts | 15 ++++++++ src/components/SharedItems.tsx | 2 +- src/components/item/ItemContent.tsx | 4 ++- .../item/header/ItemHeaderActions.tsx | 3 +- src/components/main/Home.tsx | 4 +-- src/components/main/ItemCard.tsx | 6 +++- src/components/main/ItemMenu.tsx | 27 +++++++++----- src/components/main/Items.tsx | 6 +++- src/components/main/ItemsGrid.tsx | 3 ++ src/components/main/ItemsTable.tsx | 5 ++- src/components/main/MainMenu.tsx | 2 +- src/components/table/ActionsCellRenderer.tsx | 4 ++- src/config/env.ts | 2 +- src/langs/constants.ts | 1 + src/langs/en.json | 3 +- src/langs/fr.json | 3 +- 18 files changed, 105 insertions(+), 48 deletions(-) create mode 100644 cypress/e2e/item/shared/sharedItems.cy.ts diff --git a/cypress/e2e/item/move/listMoveItem.cy.ts b/cypress/e2e/item/move/listMoveItem.cy.ts index 58899b1cc..32858a9ea 100644 --- a/cypress/e2e/item/move/listMoveItem.cy.ts +++ b/cypress/e2e/item/move/listMoveItem.cy.ts @@ -1,18 +1,12 @@ -import { - HOME_PATH, - SHARED_ITEMS_PATH, - buildItemPath, -} from '../../../../src/config/paths'; +import { HOME_PATH, buildItemPath } from '../../../../src/config/paths'; import { ITEM_MENU_MOVE_BUTTON_CLASS, TREE_MODAL_MY_ITEMS_ID, - TREE_MODAL_SHARED_ITEMS_ID, buildItemMenu, buildItemMenuButtonId, } from '../../../../src/config/selectors'; import { ITEM_LAYOUT_MODES } from '../../../../src/enums'; import { SAMPLE_ITEMS } from '../../../fixtures/items'; -import { SHARED_ITEMS } from '../../../fixtures/sharedItems'; import { TABLE_ITEM_RENDER_TIME } from '../../../support/constants'; const moveItem = ({ @@ -91,23 +85,4 @@ describe('Move Item in List', () => { expect(url).to.contain(movedItem); }); }); - - it('move item in shared item', () => { - cy.setUpApi(SHARED_ITEMS); - - // go to children item - cy.visit(SHARED_ITEMS_PATH); - - cy.switchMode(ITEM_LAYOUT_MODES.LIST); - - // move - const { path: toItemPath, id: toItemId } = SHARED_ITEMS.items[0]; - const { id: movedItem } = SHARED_ITEMS.items[1]; - moveItem({ id: movedItem, toItemPath, rootId: TREE_MODAL_SHARED_ITEMS_ID }); - - cy.wait('@moveItems').then(({ request: { body, url } }) => { - expect(body.parentId).to.equal(toItemId); - expect(url).to.contain(movedItem); - }); - }); }); diff --git a/cypress/e2e/item/shared/sharedItems.cy.ts b/cypress/e2e/item/shared/sharedItems.cy.ts new file mode 100644 index 000000000..9339433d0 --- /dev/null +++ b/cypress/e2e/item/shared/sharedItems.cy.ts @@ -0,0 +1,36 @@ +import { + ITEM_MENU_MOVE_BUTTON_CLASS, + buildItemMenu, + buildItemMenuButtonId, +} from '@/config/selectors'; + +import { SHARED_ITEMS_PATH } from '../../../../src/config/paths'; +import ITEM_LAYOUT_MODES from '../../../../src/enums/itemLayoutModes'; +import { SAMPLE_ITEMS } from '../../../fixtures/items'; +import { MEMBERS } from '../../../fixtures/members'; + +describe('Shared Items', () => { + beforeEach(() => { + cy.setUpApi({ ...SAMPLE_ITEMS, currentMember: MEMBERS.BOB }); + cy.visit(SHARED_ITEMS_PATH); + cy.wait(5000); + }); + it('move should be prevented in list', () => { + cy.switchMode(ITEM_LAYOUT_MODES.LIST); + const itemId = SAMPLE_ITEMS.items[1].id; + const menuSelector = `#${buildItemMenuButtonId(itemId)}`; + cy.get(menuSelector).click(); + cy.get(`#${buildItemMenu(itemId)} .${ITEM_MENU_MOVE_BUTTON_CLASS}`).should( + 'not.exist', + ); + }); + it('move should be prevented in grid', () => { + cy.switchMode(ITEM_LAYOUT_MODES.GRID); + const itemId = SAMPLE_ITEMS.items[1].id; + const menuSelector = `#${buildItemMenuButtonId(itemId)}`; + cy.get(menuSelector).click(); + cy.get(`#${buildItemMenu(itemId)} .${ITEM_MENU_MOVE_BUTTON_CLASS}`).should( + 'not.exist', + ); + }); +}); diff --git a/cypress/fixtures/items.ts b/cypress/fixtures/items.ts index 88c3a659d..535d08128 100644 --- a/cypress/fixtures/items.ts +++ b/cypress/fixtures/items.ts @@ -59,30 +59,45 @@ const sampleItems: DiscriminatedItem[] = [ id: 'fdf09f5a-5688-11eb-ae93-0242ac130002', name: 'own_item_name2', path: 'fdf09f5a_5688_11eb_ae93_0242ac130002', + settings: { + hasThumbnail: false, + }, }, { ...DEFAULT_FOLDER_ITEM, id: 'fdf09f5a-5688-11eb-ae93-0242ac130003', name: 'own_item_name3', path: 'ecafbd2a_5688_11eb_ae93_0242ac130002.fdf09f5a_5688_11eb_ae93_0242ac130003', + settings: { + hasThumbnail: false, + }, }, { ...DEFAULT_FOLDER_ITEM, id: 'fdf09f5a-5688-11eb-ae93-0242ac130004', name: 'own_item_name4', path: 'ecafbd2a_5688_11eb_ae93_0242ac130002.fdf09f5a_5688_11eb_ae93_0242ac130004', + settings: { + hasThumbnail: false, + }, }, { ...DEFAULT_FOLDER_ITEM, id: 'fdf09f5a-5688-11eb-ae93-0242ac130005', name: 'own_item_name5', path: 'ecafbd2a_5688_11eb_ae93_0242ac130002.fdf09f5a_5688_11eb_ae93_0242ac130005', + settings: { + hasThumbnail: false, + }, }, { ...DEFAULT_FOLDER_ITEM, id: 'ecafbd2a-5688-11eb-ae93-0242ac130006', name: 'own_item_name6', path: 'ecafbd2a_5688_11eb_ae93_0242ac130006', + settings: { + hasThumbnail: false, + }, }, ]; export const SAMPLE_ITEMS: ApiConfig = { diff --git a/src/components/SharedItems.tsx b/src/components/SharedItems.tsx index dd700fb69..7c352e8d4 100644 --- a/src/components/SharedItems.tsx +++ b/src/components/SharedItems.tsx @@ -18,7 +18,6 @@ import Main from './main/Main'; const SharedItemsLoadableContent = (): JSX.Element => { const { t: translateBuilder } = useBuilderTranslation(); const { data: sharedItems, isLoading, isError } = hooks.useSharedItems(); - if (isError) { return ; } @@ -34,6 +33,7 @@ const SharedItemsLoadableContent = (): JSX.Element => { id={SHARED_ITEMS_ID} title={translateBuilder(BUILDER.SHARED_ITEMS_TITLE)} items={sharedItems} + canMove={false} showCreator /> diff --git a/src/components/item/ItemContent.tsx b/src/components/item/ItemContent.tsx index 5a9c56780..e88158d91 100644 --- a/src/components/item/ItemContent.tsx +++ b/src/components/item/ItemContent.tsx @@ -196,7 +196,9 @@ const FolderContent = ({ headerElements={ enableEditing ? [] : undefined } - ToolbarActions={ItemActions} + // todo: not exactly correct, since you could have write rights on some child, + // but it's more tedious to check permissions over all selected items + ToolbarActions={enableEditing ? ItemActions : undefined} showCreator /> ); diff --git a/src/components/item/header/ItemHeaderActions.tsx b/src/components/item/header/ItemHeaderActions.tsx index 3b85ce08e..d7ab9b119 100644 --- a/src/components/item/header/ItemHeaderActions.tsx +++ b/src/components/item/header/ItemHeaderActions.tsx @@ -72,7 +72,8 @@ const ItemHeaderActions = ({ item }: Props): JSX.Element => { const activeActions = ( <> {showEditButton && } - + {/* prevent moving from top header to avoid confusion */} + { title={translateBuilder(BUILDER.MY_ITEMS_TITLE)} items={ownItems} headerElements={[]} - ToolbarActions={ItemActionsRenderer} + ToolbarActions={ItemActions} /> diff --git a/src/components/main/ItemCard.tsx b/src/components/main/ItemCard.tsx index 5394fc522..ec43a2f68 100644 --- a/src/components/main/ItemCard.tsx +++ b/src/components/main/ItemCard.tsx @@ -35,12 +35,14 @@ type Props = { item: DiscriminatedItem; memberships?: ItemMembership[]; itemsStatuses?: ItemsStatuses; + canMove?: boolean; }; const ItemComponent = ({ item, memberships, itemsStatuses, + canMove = true, }: Props): JSX.Element => { const { id, name } = item; const { data: thumbnailUrl, isLoading } = hooks.useItemThumbnailUrl({ id }); @@ -100,7 +102,9 @@ const ItemComponent = ({ Badges={} name={item.name} creator={member?.name} - ItemMenu={} + ItemMenu={ + + } Thumbnail={ThumbnailComponent} cardId={buildItemCard(item.id)} NameWrapper={NameWrapper({ diff --git a/src/components/main/ItemMenu.tsx b/src/components/main/ItemMenu.tsx index 66bbfc48a..1d1ef9b0f 100644 --- a/src/components/main/ItemMenu.tsx +++ b/src/components/main/ItemMenu.tsx @@ -34,9 +34,14 @@ import CopyButton from './CopyButton'; type Props = { item: DiscriminatedItem; canEdit?: boolean; + canMove?: boolean; }; -const ItemMenu = ({ item, canEdit = false }: Props): JSX.Element => { +const ItemMenu = ({ + item, + canEdit = false, + canMove = true, +}: Props): JSX.Element => { const { data: member } = useCurrentUserContext(); const [anchorEl, setAnchorEl] = useState(null); const { t: translateBuilder } = useBuilderTranslation(); @@ -67,13 +72,17 @@ const ItemMenu = ({ item, canEdit = false }: Props): JSX.Element => { if (!canEdit) { return null; } - return [ - , + const result = canMove + ? [ + , + ] + : []; + return result.concat([ , , { itemIds={[item.id]} onClick={handleClose} />, - ]; + ]); }; const renderAuthenticatedActions = () => { diff --git a/src/components/main/Items.tsx b/src/components/main/Items.tsx index e96cca38f..4ff51050d 100644 --- a/src/components/main/Items.tsx +++ b/src/components/main/Items.tsx @@ -16,7 +16,7 @@ type Props = { items?: DiscriminatedItem[]; title: string; headerElements?: JSX.Element[]; - actions?: ({ data }: { data: { id: string } }) => JSX.Element; + actions?: ({ data }: { data: DiscriminatedItem }) => JSX.Element; ToolbarActions?: ({ selectedIds }: { selectedIds: string[] }) => JSX.Element; clickable?: boolean; defaultSortedColumn?: { @@ -29,6 +29,7 @@ type Props = { showThumbnails?: boolean; showCreator?: boolean; enableMemberships?: boolean; + canMove?: boolean; }; const Items = ({ @@ -44,6 +45,7 @@ const Items = ({ showThumbnails = true, showCreator = false, enableMemberships = true, + canMove = true, }: Props): JSX.Element => { const { mode } = useLayoutContext(); const itemSearch = useItemSearch(items); @@ -65,6 +67,7 @@ const Items = ({ case ITEM_LAYOUT_MODES.GRID: return ( ); } diff --git a/src/components/main/ItemsGrid.tsx b/src/components/main/ItemsGrid.tsx index 9f497f17a..47e79399d 100644 --- a/src/components/main/ItemsGrid.tsx +++ b/src/components/main/ItemsGrid.tsx @@ -42,6 +42,7 @@ type Props = { }; headerElements?: JSX.Element[]; parentId?: string; + canMove?: boolean; }; const ItemsGrid = ({ @@ -53,6 +54,7 @@ const ItemsGrid = ({ manyMemberships, itemsStatuses, parentId, + canMove = true, }: Props): JSX.Element => { const { t: translateBuilder } = useBuilderTranslation(); const [page, setPage] = useState(1); @@ -87,6 +89,7 @@ const ItemsGrid = ({ return itemsInPage.map((item) => ( JSX.Element; + actions?: ({ data }: { data: DiscriminatedItem }) => JSX.Element; ToolbarActions?: ({ selectedIds }: { selectedIds: string[] }) => JSX.Element; clickable?: boolean; defaultSortedColumn?: { @@ -56,6 +56,7 @@ type Props = { }; showThumbnails?: boolean; showCreator?: boolean; + canMove?: boolean; }; const ItemsTable = ({ @@ -72,6 +73,7 @@ const ItemsTable = ({ defaultSortedColumn, showThumbnails = true, showCreator = false, + canMove = true, }: Props): JSX.Element => { const { t: translateBuilder } = useBuilderTranslation(); const { t: translateCommon } = useCommonTranslation(); @@ -157,6 +159,7 @@ const ItemsTable = ({ const ActionComponent = ActionsCellRenderer({ manyMemberships, member, + canMove, }); const BadgesComponent = BadgesCellRenderer({ diff --git a/src/components/main/MainMenu.tsx b/src/components/main/MainMenu.tsx index 1ffae2e02..378ab93b8 100644 --- a/src/components/main/MainMenu.tsx +++ b/src/components/main/MainMenu.tsx @@ -79,7 +79,7 @@ const MainMenu = (): JSX.Element => { - {translateBuilder('Tutorials')} + {translateBuilder(BUILDER.TUTORIALS)} ); diff --git a/src/components/table/ActionsCellRenderer.tsx b/src/components/table/ActionsCellRenderer.tsx index f9b5b01b8..6e3e20209 100644 --- a/src/components/table/ActionsCellRenderer.tsx +++ b/src/components/table/ActionsCellRenderer.tsx @@ -18,6 +18,7 @@ import ItemMenu from '../main/ItemMenu'; type Props = { manyMemberships?: ResultOf; member?: Member | null; + canMove?: boolean; }; type ChildCompProps = { @@ -28,6 +29,7 @@ type ChildCompProps = { const ActionsCellRenderer = ({ manyMemberships, member, + canMove, }: Props): ((arg: ChildCompProps) => JSX.Element) => { const ChildComponent = ({ data: item }: ChildCompProps) => { const [canEdit, setCanEdit] = useState(false); @@ -50,7 +52,7 @@ const ActionsCellRenderer = ({ const renderAnyoneActions = () => ( <> - + ); diff --git a/src/config/env.ts b/src/config/env.ts index da4ff6960..24c00948c 100644 --- a/src/config/env.ts +++ b/src/config/env.ts @@ -13,7 +13,7 @@ export const GRAASP_LIBRARY_HOST = export const GRAASP_ANALYZER_HOST = import.meta.env.VITE_GRAASP_ANALYZER_HOST || 'http://localhost:3113'; export const GRAASP_ACCOUNT_HOST = - import.meta.env.VITE_GRAASP_ACCOUNT_HOST || 'http://localhost:3114'; + import.meta.env.VITE_GRAASP_ACCOUNT_HOST || 'http://localhost:3115'; export const H5P_INTEGRATION_URL = import.meta.env.VITE_H5P_INTEGRATION_URL || `${API_HOST}/p/h5p-integration`; diff --git a/src/langs/constants.ts b/src/langs/constants.ts index bd73ef473..c714ed1ac 100644 --- a/src/langs/constants.ts +++ b/src/langs/constants.ts @@ -332,4 +332,5 @@ export const BUILDER = { FLAG_REASON_DESCRIPTION: 'FLAG_REASON_DESCRIPTION', FLAG_MODAL_TITLE: 'FLAG_MODAL_TITLE', FLAG_MODAL_CONFIRM: 'FLAG_MODAL_CONFIRM', + TUTORIALS: 'TUTORIALS', }; diff --git a/src/langs/en.json b/src/langs/en.json index f8fa792ce..170ada6c2 100644 --- a/src/langs/en.json +++ b/src/langs/en.json @@ -270,5 +270,6 @@ "APP_LIST_LOADING_FAILED": "There was an error getting the app list. Please try again later.", "FLAG_REASON_DESCRIPTION": "Select reason for flagging this item", "FLAG_MODAL_TITLE": "Flag Item", - "FLAG_MODAL_CONFIRM": "Flag" + "FLAG_MODAL_CONFIRM": "Flag", + "TUTORIALS": "Tutorials" } diff --git a/src/langs/fr.json b/src/langs/fr.json index c518ef854..ed29227e3 100644 --- a/src/langs/fr.json +++ b/src/langs/fr.json @@ -256,5 +256,6 @@ "DELETE_MEMBERSHIP_MODAL_CONFIRM_BUTTON": "Confirmer", "DELETE_OWN_MEMBERSHIP_MESSAGE": "Êtes-vous sûr de vouloir supprimer votre propre autorisation sur cet élément ? Vous perdrez l'accès à cet élément, cette action est irréversible.", "DELETE_MEMBERSHIP_MESSAGE": "Êtes-vous sûr de vouloir supprimer l'accès en {{permissionLevel}} sur cet élément à {{name}} ? Il leur sera impossible d'accéder à cet élément.", - "DELETE_LAST_ADMIN_ALERT_MESSAGE": "Vous n'êtes pas autorisé à supprimer le dernier administrateur de cet élément." + "DELETE_LAST_ADMIN_ALERT_MESSAGE": "Vous n'êtes pas autorisé à supprimer le dernier administrateur de cet élément.", + "TUTORIALS": "Tutoriels" }