fix: some loaders to display data first and pagination bugs (#967)

* fix: move screens and loaders * fix: remove waits * fix: actions should not display before we get memberships
graasp · Feb 5, 2024 · 392a736 · 392a736
1 parent dbe6886
commit 392a736
Show file tree

Hide file tree

Showing 16 changed files with 322 additions and 316 deletions.
diff --git a/cypress/e2e/item/copy/listCopyItem.cy.ts b/cypress/e2e/item/copy/listCopyItem.cy.ts
@@ -24,8 +24,7 @@ const copyItem = ({
   toItemPath: string;
   rootId?: string;
 }) => {
-  const menuSelector = `#${buildItemMenuButtonId(id)}`;
-  cy.get(menuSelector).click();
+  cy.get(`#${buildItemMenuButtonId(id)}`).click();
   cy.get(`#${buildItemMenu(id)} .${ITEM_MENU_COPY_BUTTON_CLASS}`).click();
   cy.fillTreeModal(toItemPath, rootId);
 };

diff --git a/cypress/e2e/item/delete/listRecycleItem.cy.ts b/cypress/e2e/item/delete/listRecycleItem.cy.ts
@@ -8,8 +8,7 @@ import { ITEM_LAYOUT_MODES } from '../../../../src/enums';
 import { SAMPLE_ITEMS } from '../../../fixtures/items';
 
 const recycleItem = (id: string) => {
-  const menuSelector = `#${buildItemMenuButtonId(id)}`;
-  cy.get(menuSelector).click();
+  cy.get(`#${buildItemMenuButtonId(id)}`).click();
   cy.get(`#${buildItemMenu(id)} .${ITEM_MENU_RECYCLE_BUTTON_CLASS}`).click();
 };
 

diff --git a/cypress/e2e/item/pin/pinItem.cy.ts b/cypress/e2e/item/pin/pinItem.cy.ts
@@ -1,17 +1,34 @@
-import { HOME_PATH } from '../../../../src/config/paths';
+import { HOME_PATH, buildItemPath } from '../../../../src/config/paths';
 import {
   PIN_ITEM_BUTTON_CLASS,
+  buildDownloadButtonId,
   buildItemMenu,
   buildItemMenuButtonId,
 } from '../../../../src/config/selectors';
 import { ITEM_LAYOUT_MODES } from '../../../../src/enums';
-import { ITEMS_SETTINGS, PINNED_ITEM } from '../../../fixtures/items';
+import {
+  ITEMS_SETTINGS,
+  PINNED_ITEM,
+  PUBLISHED_ITEM,
+} from '../../../fixtures/items';
 
 const togglePinButton = (itemId: string) => {
   cy.get(`#${buildItemMenuButtonId(itemId)}`).click();
   cy.get(`#${buildItemMenu(itemId)} .${PIN_ITEM_BUTTON_CLASS}`).click();
 };
 
+describe('Anonymous', () => {
+  const itemId = PUBLISHED_ITEM.id;
+  beforeEach(() => {
+    cy.setUpApi({ currentMember: null, items: [PUBLISHED_ITEM] });
+    cy.visit(buildItemPath(itemId));
+  });
+  it("Can see item but can't pin", () => {
+    cy.get(`#${buildDownloadButtonId(itemId)}`).should('be.visible');
+    cy.get(`#${buildItemMenuButtonId(itemId)}`).should('not.exist');
+  });
+});
+
 describe('Pinning Item', () => {
   describe('Successfully pinning item in List', () => {
     beforeEach(() => {

diff --git a/cypress/support/commands/navigation.ts b/cypress/support/commands/navigation.ts
@@ -4,26 +4,19 @@ import {
   buildItemLink,
   buildItemsTableRowIdAttribute,
 } from '../../../src/config/selectors';
-import { NAVIGATE_PAUSE, WAIT_FOR_ITEM_TABLE_ROW_TIME } from '../constants';
 
 Cypress.Commands.add('goToItemInGrid', (id) => {
-  cy.wait(NAVIGATE_PAUSE);
   cy.get(`#${buildItemLink(id)}`).click();
 });
 
 Cypress.Commands.add('goToItemInList', (id) => {
-  cy.wait(NAVIGATE_PAUSE);
-  cy.get(buildItemsTableRowIdAttribute(id), {
-    timeout: WAIT_FOR_ITEM_TABLE_ROW_TIME,
-  }).click();
+  cy.get(buildItemsTableRowIdAttribute(id)).click();
 });
 
 Cypress.Commands.add('goToHome', () => {
-  cy.wait(NAVIGATE_PAUSE);
   cy.get(`#${NAVIGATION_HOME_LINK_ID}`).click();
 });
 
 Cypress.Commands.add('goToItemWithNavigation', (id) => {
-  cy.wait(NAVIGATE_PAUSE);
   cy.get(`[href="${buildItemPath(id)}"]`).click();
 });
diff --git a/src/components/item/ItemContent.tsx b/src/components/item/ItemContent.tsx
@@ -1,4 +1,4 @@
-import { Container, styled } from '@mui/material';
+import { Container, Skeleton, styled } from '@mui/material';
 
 import { Api } from '@graasp/query-client';
 import {
@@ -66,27 +66,31 @@ const FileContent = ({
   item,
 }: {
   item: LocalFileItemType | S3FileItemType;
-}): JSX.Element => {
+}): JSX.Element | null => {
   const { data: fileUrl, isLoading, isError } = useFileContentUrl(item.id);
 
+  if (fileUrl) {
+    return (
+      <StyledContainer>
+        <FileItem
+          fileUrl={fileUrl}
+          id={buildFileItemId(item.id)}
+          item={item}
+          pdfViewerLink={buildPdfViewerLink(GRAASP_ASSETS_URL)}
+        />
+      </StyledContainer>
+    );
+  }
+
   if (isLoading) {
-    return <Loader />;
+    return <Skeleton height="50vh" />;
   }
 
   if (isError) {
     return <ErrorAlert id={ITEM_SCREEN_ERROR_ALERT_ID} />;
   }
 
-  return (
-    <StyledContainer>
-      <FileItem
-        fileUrl={fileUrl}
-        id={buildFileItemId(item.id)}
-        item={item}
-        pdfViewerLink={buildPdfViewerLink(GRAASP_ASSETS_URL)}
-      />
-    </StyledContainer>
-  );
+  return null;
 };
 
 /**

diff --git a/src/components/item/header/ItemHeader.tsx b/src/components/item/header/ItemHeader.tsx
@@ -1,46 +1,25 @@
 import Stack from '@mui/material/Stack';
 
-import { Loader, useShortenURLParams } from '@graasp/ui';
-
-import { ITEM_ID_PARAMS } from '@/config/paths';
-
-import { hooks } from '../../../config/queryClient';
 import { ITEM_HEADER_ID } from '../../../config/selectors';
-import ErrorAlert from '../../common/ErrorAlert';
 import Navigation from '../../layout/Navigation';
 import ItemHeaderActions from './ItemHeaderActions';
 
-const { useItem } = hooks;
-
 type Props = {
   showNavigation?: boolean;
 };
 
-const ItemHeader = ({ showNavigation = true }: Props): JSX.Element => {
-  const itemId = useShortenURLParams(ITEM_ID_PARAMS);
-  const { data: item, isLoading: isItemLoading, isError } = useItem(itemId);
-
-  if (isItemLoading) {
-    return <Loader />;
-  }
-
-  if (isError) {
-    return <ErrorAlert />;
-  }
-
-  return (
-    <Stack
-      direction="row"
-      justifyContent="space-between"
-      alignItems="center"
-      mb={1}
-      id={ITEM_HEADER_ID}
-    >
-      {/* display empty div to render actions on the right */}
-      {showNavigation ? <Navigation /> : <div />}
-      <ItemHeaderActions item={item} />
-    </Stack>
-  );
-};
+const ItemHeader = ({ showNavigation = true }: Props): JSX.Element | null => (
+  <Stack
+    direction="row"
+    justifyContent="space-between"
+    alignItems="center"
+    mb={1}
+    id={ITEM_HEADER_ID}
+  >
+    {/* display empty div to render actions on the right */}
+    {showNavigation ? <Navigation /> : <div />}
+    <ItemHeaderActions />
+  </Stack>
+);
 
 export default ItemHeader;
diff --git a/src/components/item/header/ItemHeaderActions.tsx b/src/components/item/header/ItemHeaderActions.tsx
@@ -1,10 +1,11 @@
 import { Stack } from '@mui/material';
 
-import { DiscriminatedItem, ItemType, PermissionLevel } from '@graasp/sdk';
-import { ChatboxButton } from '@graasp/ui';
+import { ItemType, PermissionLevel } from '@graasp/sdk';
+import { ChatboxButton, useShortenURLParams } from '@graasp/ui';
 
 import EditButton from '@/components/common/EditButton';
 import DownloadButton from '@/components/main/DownloadButton';
+import { ITEM_ID_PARAMS } from '@/config/paths';
 
 import { ITEM_TYPES_WITH_CAPTIONS } from '../../../config/constants';
 import { useBuilderTranslation } from '../../../config/i18n';
@@ -25,13 +26,10 @@ import ItemMenu from '../../main/ItemMenu';
 import ItemSettingsButton from '../settings/ItemSettingsButton';
 import ModeButton from './ModeButton';
 
-const { useItemMemberships } = hooks;
+const { useItemMemberships, useItem } = hooks;
 
-type Props = {
-  item?: DiscriminatedItem;
-};
-
-const ItemHeaderActions = ({ item }: Props): JSX.Element => {
+const ItemHeaderActions = (): JSX.Element => {
+  const itemId = useShortenURLParams(ITEM_ID_PARAMS);
   const { t: translateBuilder } = useBuilderTranslation();
   const {
     editingItemId,
@@ -42,6 +40,7 @@ const ItemHeaderActions = ({ item }: Props): JSX.Element => {
   } = useLayoutContext();
 
   const { data: member } = useCurrentUserContext();
+  const { data: item } = useItem(itemId);
 
   const { data: memberships } = useItemMemberships(item?.id);
   const canEdit = isItemUpdateAllowedForUser({
@@ -73,7 +72,12 @@ const ItemHeaderActions = ({ item }: Props): JSX.Element => {
         <>
           {showEditButton && <EditButton item={item} />}
           {/* prevent moving from top header to avoid confusion */}
-          <ItemMenu item={item} canMove={false} canEdit={showEditButton} />
+          <ItemMenu
+            item={item}
+            canMove={false}
+            canAdmin={canAdmin}
+            canEdit={showEditButton}
+          />
 
           <ShareButton itemId={item.id} />
           <ChatboxButton

diff --git a/src/components/main/ItemActions.tsx b/src/components/main/ItemActions.tsx
@@ -10,7 +10,7 @@ import CopyButton from './CopyButton';
 type Props = {
   selectedIds: string[];
 };
-
+// todo: not used anymore ?
 const ItemActionsRenderer = ({ selectedIds }: Props): JSX.Element => (
   <>
     <MoveButton

diff --git a/src/components/main/ItemCard.tsx b/src/components/main/ItemCard.tsx
@@ -1,7 +1,12 @@
 import { CSSProperties, PropsWithChildren } from 'react';
 import { Link } from 'react-router-dom';
 
-import { DiscriminatedItem, ItemMembership, ItemType } from '@graasp/sdk';
+import {
+  DiscriminatedItem,
+  ItemMembership,
+  ItemType,
+  PermissionLevel,
+} from '@graasp/sdk';
 import { Card as GraaspCard, Thumbnail } from '@graasp/ui';
 
 import truncate from 'lodash.truncate';
@@ -12,7 +17,10 @@ import { hooks } from '../../config/queryClient';
 import { buildItemCard, buildItemLink } from '../../config/selectors';
 import defaultImage from '../../resources/avatar.png';
 import { stripHtml } from '../../utils/item';
-import { isItemUpdateAllowedForUser } from '../../utils/membership';
+import {
+  getHighestPermissionForMemberFromMemberships,
+  isItemUpdateAllowedForUser,
+} from '../../utils/membership';
 import EditButton from '../common/EditButton';
 import FavoriteButton from '../common/FavoriteButton';
 import { useCurrentUserContext } from '../context/CurrentUserContext';
@@ -80,6 +88,12 @@ const ItemComponent = ({
     memberships,
     memberId: member?.id,
   });
+  const canAdmin = member?.id
+    ? getHighestPermissionForMemberFromMemberships({
+        memberships,
+        memberId: member?.id,
+      })?.permission === PermissionLevel.Admin
+    : false;
 
   const Actions = (
     <>
@@ -103,7 +117,12 @@ const ItemComponent = ({
       name={item.name}
       creator={item.creator?.name}
       ItemMenu={
-        <ItemMenu item={item} canEdit={enableEdition} canMove={canMove} />
+        <ItemMenu
+          item={item}
+          canEdit={enableEdition}
+          canAdmin={canAdmin}
+          canMove={canMove}
+        />
       }
       Thumbnail={ThumbnailComponent}
       cardId={buildItemCard(item.id)}