Merge pull request #6617 from weseek/fix/page-attachment-inuse

fix: PageAttachmentList

packages/app/src/components/PageAttachment.tsx

@@ -1,65 +1,49 @@
-import React, { useCallback, useEffect, useState } from 'react';
+import React, {
+  useCallback, useMemo, useState,
+} from 'react';
 
-import { useTranslation } from 'next-i18next';
+import { HasObjectId, IAttachment } from '@growi/core';
 
 import { useSWRxAttachments } from '~/stores/attachment';
 import { useEditingMarkdown, useCurrentPageId, useIsGuestUser } from '~/stores/context';
 
-import DeleteAttachmentModal from './PageAttachment/DeleteAttachmentModal';
-import PageAttachmentList from './PageAttachment/PageAttachmentList';
+import { DeleteAttachmentModal } from './PageAttachment/DeleteAttachmentModal';
+import { PageAttachmentList } from './PageAttachment/PageAttachmentList';
 import PaginationWrapper from './PaginationWrapper';
 
 // Utility
-const checkIfFileInUse = (markdown: string, attachment) => {
-  return markdown.match(attachment._id);
-};
-
-// Custom hook that handles processes related to inUseAttachments
-const useInUseAttachments = (attachments) => {
-  const { data: markdown } = useEditingMarkdown();
-  const [inUse, setInUse] = useState<any>({});
-
-  // Update inUse when either of attachments or markdown is updated
-  useEffect(() => {
-    if (markdown == null) {
-      return;
-    }
-
-    const newInUse = {};
-
-    for (const attachment of attachments) {
-      newInUse[attachment._id] = checkIfFileInUse(markdown, attachment);
-    }
-
-    setInUse(newInUse);
-  }, [attachments, markdown]);
-
-  return inUse;
+const checkIfFileInUse = (markdown: string, attachment): boolean => {
+  return markdown.indexOf(attachment._id) >= 0;
 };
 
 const PageAttachment = (): JSX.Element => {
-  const { t } = useTranslation();
-
   // Static SWRs
   const { data: pageId } = useCurrentPageId();
   const { data: isGuestUser } = useIsGuestUser();
+  const { data: markdown } = useEditingMarkdown();
 
   // States
   const [pageNumber, setPageNumber] = useState(1);
-  const [attachmentToDelete, setAttachmentToDelete] = useState<any>(undefined);
+  const [attachmentToDelete, setAttachmentToDelete] = useState<(IAttachment & HasObjectId) | null>(null);
   const [deleting, setDeleting] = useState(false);
   const [deleteError, setDeleteError] = useState('');
 
   // SWRs
   const { data: dataAttachments, remove } = useSWRxAttachments(pageId, pageNumber);
-  const {
-    attachments = [],
-    totalAttachments = 0,
-    limit,
-  } = dataAttachments ?? {};
 
   // Custom hooks
-  const inUseAttachments = useInUseAttachments(attachments);
+  const inUseAttachmentsMap: { [id: string]: boolean } | undefined = useMemo(() => {
+    if (markdown == null || dataAttachments == null) {
+      return undefined;
+    }
+
+    const attachmentEntries = dataAttachments.attachments
+      .map((attachment) => {
+        return [attachment._id, checkIfFileInUse(markdown, attachment)];
+      });
+
+    return Object.fromEntries(attachmentEntries);
+  }, [dataAttachments, markdown]);
 
   // Methods
   const onChangePageHandler = useCallback((newPageNumber: number) => {
@@ -70,7 +54,7 @@ const PageAttachment = (): JSX.Element => {
     setAttachmentToDelete(attachment);
   }, []);
 
-  const onAttachmentDeleteClickedConfirmHandler = useCallback(async(attachment) => {
+  const onAttachmentDeleteClickedConfirmHandler = useCallback(async(attachment: IAttachment & HasObjectId) => {
     setDeleting(true);
 
     try {
@@ -91,22 +75,32 @@ const PageAttachment = (): JSX.Element => {
   }, []);
 
   // Renderers
-  const renderDeleteAttachmentModal = useCallback(() => {
-    if (isGuestUser) {
-      return <></>;
-    }
-
-    if (attachments.length === 0) {
+  const renderPageAttachmentList = useCallback(() => {
+    if (dataAttachments == null || inUseAttachmentsMap == null) {
       return (
-        <div data-testid="page-attachment">
-          {t('No_attachments_yet')}
+        <div className="text-muted text-center">
+          <i className="fa fa-2x fa-spinner fa-pulse mr-1"></i>
         </div>
       );
     }
 
-    let deleteInUse = null;
-    if (attachmentToDelete != null) {
-      deleteInUse = inUseAttachments[attachmentToDelete._id] || false;
+    return (
+      <PageAttachmentList
+        attachments={dataAttachments.attachments}
+        inUse={inUseAttachmentsMap}
+        onAttachmentDeleteClicked={onAttachmentDeleteClicked}
+        isUserLoggedIn={!isGuestUser}
+      />
+    );
+  }, [dataAttachments, inUseAttachmentsMap, isGuestUser, onAttachmentDeleteClicked]);
+
+  const renderDeleteAttachmentModal = useCallback(() => {
+    if (isGuestUser) {
+      return <></>;
+    }
+
+    if (dataAttachments == null || dataAttachments.attachments.length === 0 || attachmentToDelete == null) {
+      return <></>;
     }
 
     const isOpen = attachmentToDelete != null;
@@ -114,36 +108,39 @@ const PageAttachment = (): JSX.Element => {
     return (
       <DeleteAttachmentModal
         isOpen={isOpen}
-        animation="false"
         toggle={onToggleHandler}
         attachmentToDelete={attachmentToDelete}
-        inUse={deleteInUse}
         deleting={deleting}
         deleteError={deleteError}
         onAttachmentDeleteClickedConfirm={onAttachmentDeleteClickedConfirmHandler}
       />
     );
   // eslint-disable-next-line max-len
-  }, [attachmentToDelete, attachments.length, deleteError, deleting, inUseAttachments, isGuestUser, onAttachmentDeleteClickedConfirmHandler, onToggleHandler, t]);
+  }, [attachmentToDelete, dataAttachments, deleteError, deleting, isGuestUser, onAttachmentDeleteClickedConfirmHandler, onToggleHandler]);
 
-  return (
-    <div data-testid="page-attachment">
-      <PageAttachmentList
-        attachments={attachments}
-        inUse={inUseAttachments}
-        onAttachmentDeleteClicked={onAttachmentDeleteClicked}
-        isUserLoggedIn={!isGuestUser}
-      />
-
-      {renderDeleteAttachmentModal()}
+  const renderPaginationWrapper = useCallback(() => {
+    if (dataAttachments == null || dataAttachments.attachments.length === 0) {
+      return <></>;
+    }
 
+    return (
       <PaginationWrapper
         activePage={pageNumber}
         changePage={onChangePageHandler}
-        totalItemsCount={totalAttachments}
-        pagingLimit={limit}
+        totalItemsCount={dataAttachments.totalAttachments}
+        pagingLimit={dataAttachments.limit}
         align="center"
       />
+    );
+  }, [dataAttachments, onChangePageHandler, pageNumber]);
+
+  return (
+    <div data-testid="page-attachment">
+      {renderPageAttachmentList()}
+
+      {renderDeleteAttachmentModal()}
+
+      {renderPaginationWrapper()}
     </div>
   );
 };

packages/app/src/components/PageAttachment/DeleteAttachmentModal.jsx

@@ -1,97 +0,0 @@
-/* eslint-disable react/prop-types */
-import React from 'react';
-
-import {
-  Button,
-  Modal, ModalHeader, ModalBody, ModalFooter,
-} from 'reactstrap';
-
-import { UserPicture } from '@growi/ui';
-import Username from '../User/Username';
-
-export default class DeleteAttachmentModal extends React.Component {
-
-  constructor(props) {
-    super(props);
-
-    this._onDeleteConfirm = this._onDeleteConfirm.bind(this);
-  }
-
-  _onDeleteConfirm() {
-    this.props.onAttachmentDeleteClickedConfirm(this.props.attachmentToDelete);
-  }
-
-  iconNameByFormat(format) {
-    if (format.match(/image\/.+/i)) {
-      return 'icon-picture';
-    }
-
-    return 'icon-doc';
-  }
-
-  renderByFileFormat(attachment) {
-    const content = (attachment.fileFormat.match(/image\/.+/i))
-      ? <img src={attachment.filePathProxied} alt="deleting image" />
-      : '';
-
-
-    return (
-      <div className="attachment-delete-image">
-        <p>
-          <i className={this.iconNameByFormat(attachment.fileFormat)}></i> {attachment.originalName}
-        </p>
-        <p>
-          uploaded by <UserPicture user={attachment.creator} size="sm"></UserPicture> <Username user={attachment.creator}></Username>
-        </p>
-        {content}
-      </div>
-    );
-  }
-
-  render() {
-    const attachment = this.props.attachmentToDelete;
-    if (attachment === null) {
-      return null;
-    }
-
-    const props = Object.assign({}, this.props);
-    delete props.onAttachmentDeleteClickedConfirm;
-    delete props.attachmentToDelete;
-    delete props.inUse;
-    delete props.deleting;
-    delete props.deleteError;
-
-    let deletingIndicator = '';
-    if (this.props.deleting) {
-      deletingIndicator = <div className="speeding-wheel-sm"></div>;
-    }
-    if (this.props.deleteError) {
-      deletingIndicator = <span>{this.props.deleteError}</span>;
-    }
-
-    const renderAttachment = this.renderByFileFormat(attachment);
-
-    return (
-      <Modal {...props} className="attachment-delete-modal" bssize="large" aria-labelledby="contained-modal-title-lg">
-        <ModalHeader tag="h4" toggle={this.props.toggle} className="bg-danger text-light">
-          <span id="contained-modal-title-lg">Delete attachment?</span>
-        </ModalHeader>
-        <ModalBody>
-          {renderAttachment}
-        </ModalBody>
-        <ModalFooter>
-          <div className="mr-3 d-inline-block">
-            {deletingIndicator}
-          </div>
-          <Button
-            color="danger"
-            onClick={this._onDeleteConfirm}
-            disabled={this.props.deleting}
-          >Delete!
-          </Button>
-        </ModalFooter>
-      </Modal>
-    );
-  }
-
-}

packages/app/src/components/PageAttachment/DeleteAttachmentModal.tsx

@@ -0,0 +1,98 @@
+/* eslint-disable react/prop-types */
+import React, { useCallback } from 'react';
+
+import { HasObjectId, IAttachment } from '@growi/core';
+import { UserPicture } from '@growi/ui';
+import {
+  Button,
+  Modal, ModalHeader, ModalBody, ModalFooter,
+} from 'reactstrap';
+
+import Username from '../User/Username';
+
+
+function iconNameByFormat(format: string): string {
+  if (format.match(/image\/.+/i)) {
+    return 'icon-picture';
+  }
+
+  return 'icon-doc';
+}
+
+
+type Props = {
+  isOpen: boolean,
+  toggle: () => void,
+  attachmentToDelete: IAttachment & HasObjectId | null,
+  deleting: boolean,
+  deleteError: string,
+  onAttachmentDeleteClickedConfirm?: (attachment: IAttachment & HasObjectId) => Promise<void>,
+}
+
+export const DeleteAttachmentModal = (props: Props): JSX.Element => {
+
+  const {
+    isOpen, toggle,
+    attachmentToDelete, deleting, deleteError,
+    onAttachmentDeleteClickedConfirm,
+  } = props;
+
+  const onDeleteConfirm = useCallback(() => {
+    if (attachmentToDelete == null || onAttachmentDeleteClickedConfirm == null) {
+      return;
+    }
+    onAttachmentDeleteClickedConfirm(attachmentToDelete);
+  }, [attachmentToDelete, onAttachmentDeleteClickedConfirm]);
+
+  const renderByFileFormat = useCallback((attachment) => {
+    const content = (attachment.fileFormat.match(/image\/.+/i))
+      // eslint-disable-next-line @next/next/no-img-element
+      ? <img src={attachment.filePathProxied} alt="deleting image" />
+      : '';
+
+
+    return (
+      <div className="attachment-delete-image">
+        <p>
+          <i className={iconNameByFormat(attachment.fileFormat)}></i> {attachment.originalName}
+        </p>
+        <p>
+          uploaded by <UserPicture user={attachment.creator} size="sm"></UserPicture> <Username user={attachment.creator}></Username>
+        </p>
+        {content}
+      </div>
+    );
+  }, []);
+
+  let deletingIndicator = <></>;
+  if (deleting) {
+    deletingIndicator = <div className="speeding-wheel-sm"></div>;
+  }
+  if (deleteError) {
+    deletingIndicator = <span>{deleteError}</span>;
+  }
+
+
+  return (
+    <Modal isOpen={isOpen} className="attachment-delete-modal" size="lg" aria-labelledby="contained-modal-title-lg" fade={false}>
+      <ModalHeader tag="h4" toggle={toggle} className="bg-danger text-light">
+        <span id="contained-modal-title-lg">Delete attachment?</span>
+      </ModalHeader>
+      <ModalBody>
+        {renderByFileFormat(attachmentToDelete)}
+      </ModalBody>
+      <ModalFooter>
+        <div className="mr-3 d-inline-block">
+          {deletingIndicator}
+        </div>
+        <Button
+          color="danger"
+          onClick={onDeleteConfirm}
+          disabled={deleting}
+        >Delete!
+        </Button>
+      </ModalFooter>
+    </Modal>
+  );
+
+};

packages/app/src/components/PageAttachment/PageAttachmentList.jsx

@@ -1,45 +0,0 @@
-import React from 'react';
-import PropTypes from 'prop-types';
-
-import { Attachment } from '@growi/ui';
-
-export default class PageAttachmentList extends React.Component {
-
-  render() {
-    if (this.props.attachments <= 0) {
-      return null;
-    }
-
-    const attachmentList = this.props.attachments.map((attachment, idx) => {
-      return (
-        <Attachment
-          key={`page:attachment:${attachment._id}`}
-          attachment={attachment}
-          inUse={this.props.inUse[attachment._id] || false}
-          onAttachmentDeleteClicked={this.props.onAttachmentDeleteClicked}
-          isUserLoggedIn={this.props.isUserLoggedIn}
-        />
-      );
-    });
-
-    return (
-      <div>
-        <ul className="pl-2">
-          {attachmentList}
-        </ul>
-      </div>
-    );
-  }
-
-}
-
-PageAttachmentList.propTypes = {
-  attachments: PropTypes.arrayOf(PropTypes.object),
-  inUse: PropTypes.objectOf(PropTypes.bool),
-  onAttachmentDeleteClicked: PropTypes.func,
-  isUserLoggedIn: PropTypes.bool,
-};
-PageAttachmentList.defaultProps = {
-  attachments: [],
-  inUse: {},
-};

packages/app/src/components/PageAttachment/PageAttachmentList.tsx

@@ -0,0 +1,47 @@
+import React from 'react';
+
+
+import { HasObjectId, IAttachment } from '@growi/core';
+import { Attachment } from '@growi/ui';
+import { useTranslation } from 'next-i18next';
+
+
+type Props = {
+  attachments: (IAttachment & HasObjectId)[],
+  inUse: { [id: string]: boolean },
+  onAttachmentDeleteClicked?: (attachment: IAttachment & HasObjectId) => void,
+  isUserLoggedIn?: boolean,
+}
+
+export const PageAttachmentList = (props: Props): JSX.Element => {
+  const { t } = useTranslation();
+
+  const {
+    attachments, inUse, onAttachmentDeleteClicked, isUserLoggedIn,
+  } = props;
+
+  if (attachments.length === 0) {
+    return <>{t('No_attachments_yet')}</>;
+  }
+
+  const attachmentList = attachments.map((attachment) => {
+    return (
+      <Attachment
+        key={`page:attachment:${attachment._id}`}
+        attachment={attachment}
+        inUse={inUse[attachment._id] || false}
+        onAttachmentDeleteClicked={onAttachmentDeleteClicked}
+        isUserLoggedIn={isUserLoggedIn}
+      />
+    );
+  });
+
+  return (
+    <div>
+      <ul className="pl-2">
+        {attachmentList}
+      </ul>
+    </div>
+  );
+
+};

packages/app/src/interfaces/attachment.ts

@@ -1,10 +1,8 @@
-import type { IAttachment } from '@growi/core';
+import type { HasObjectId, IAttachment } from '@growi/core';
 
 import type { PaginateResult } from './mongoose-utils';
 
 
 export type IResAttachmentList = {
-  data: {
-    paginateResult: PaginateResult<IAttachment>
-  }
+  paginateResult: PaginateResult<IAttachment & HasObjectId>
 };

packages/app/src/stores/attachment.tsx

@@ -1,11 +1,13 @@
 import { useCallback } from 'react';
 
 import {
+  HasObjectId,
   IAttachment, Nullable, SWRResponseWithUtils, withUtils,
 } from '@growi/core';
 import useSWR from 'swr';
 
-import { apiGet, apiPost } from '~/client/util/apiv1-client';
+import { apiPost } from '~/client/util/apiv1-client';
+import { apiv3Get } from '~/client/util/apiv3-client';
 import { IResAttachmentList } from '~/interfaces/attachment';
 
 type Util = {
@@ -13,7 +15,7 @@ type Util = {
 };
 
 type IDataAttachmentList = {
-  attachments: IAttachment[]
+  attachments: (IAttachment & HasObjectId)[]
   totalAttachments: number
   limit: number
 };
@@ -21,17 +23,19 @@ type IDataAttachmentList = {
 export const useSWRxAttachments = (pageId?: Nullable<string>, pageNumber?: number): SWRResponseWithUtils<Util, IDataAttachmentList, Error> => {
   const shouldFetch = pageId != null && pageNumber != null;
 
-  const fetcher = useCallback(async(endpoint) => {
-    const res = await apiGet<IResAttachmentList>(endpoint, { pageId, pageNumber });
+  const fetcher = useCallback(async(endpoint, pageId, pageNumber) => {
+    const res = await apiv3Get<IResAttachmentList>(endpoint, { pageId, pageNumber });
+    const resAttachmentList = res.data;
+    const { paginateResult } = resAttachmentList;
     return {
-      attachments: res.data.paginateResult.docs,
-      totalAttachments: res.data.paginateResult.totalDocs,
-      limit: res.data.paginateResult.limit,
+      attachments: paginateResult.docs,
+      totalAttachments: paginateResult.totalDocs,
+      limit: paginateResult.limit,
     };
-  }, [pageId, pageNumber]);
+  }, []);
 
   const swrResponse = useSWR(
-    shouldFetch ? ['/attachments/list', pageId, pageNumber] : null,
+    shouldFetch ? ['/attachment/list', pageId, pageNumber] : null,
     fetcher,
   );
 

packages/ui/src/components/Attachment/Attachment.jsx

@@ -60,7 +60,9 @@ export class Attachment extends React.Component {
         <span className="mr-1 attachment-userpicture">
           <UserPicture user={attachment.creator} size="sm"></UserPicture>
         </span>
-        <a className="mr-2" href={attachment.filePathProxied}><i className={formatIcon}></i> {attachment.originalName}</a>
+        <a className="mr-2" href={attachment.filePathProxied} target="_blank" rel="noopener noreferrer">
+          <i className={formatIcon}></i> {attachment.originalName}
+        </a>
         <span className="mr-2">{fileType}</span>
         <span className="mr-2">{fileInUse}</span>
         <span className="mr-2">{btnDownload}</span>