Merge pull request #8817 from weseek/fix/bookmark-for-deleted-pages

fix: BookmarkItem occures an error when the related page has been deleted

apps/app/src/components/Bookmarks/BookmarkItem.tsx

@@ -28,7 +28,7 @@ import { DragAndDropWrapper } from './DragAndDropWrapper';
 type Props = {
   isReadOnlyUser: boolean
   isOperable: boolean,
-  bookmarkedPage: IPageHasId,
+  bookmarkedPage: IPageHasId | null,
   level: number,
   parentFolder: BookmarkFolderItems | null,
   canMoveToRoot: boolean,
@@ -50,17 +50,17 @@ export const BookmarkItem = (props: Props): JSX.Element => {
   const { open: openPutBackPageModal } = usePutBackPageModal();
   const [isRenameInputShown, setRenameInputShown] = useState(false);
 
-  const { data: pageInfo, mutate: mutatePageInfo } = useSWRxPageInfo(bookmarkedPage._id);
+  const { data: pageInfo, mutate: mutatePageInfo } = useSWRxPageInfo(bookmarkedPage?._id);
   const { trigger: mutateCurrentPage } = useSWRMUTxCurrentPage();
-  const dPagePath = new DevidedPagePath(bookmarkedPage.path, false, true);
-  const { latter: pageTitle, former: formerPagePath } = dPagePath;
-  const bookmarkItemId = `bookmark-item-${bookmarkedPage._id}`;
+
   const paddingLeft = BASE_BOOKMARK_PADDING + (BASE_FOLDER_PADDING * (level));
   const dragItem: Partial<DragItemDataType> = {
     ...bookmarkedPage, parentFolder,
   };
 
   const onClickMoveToRootHandler = useCallback(async() => {
+    if (bookmarkedPage == null) return;
+
     try {
       await addBookmarkToFolder(bookmarkedPage._id, null);
       bookmarkFolderTreeMutation();
@@ -68,7 +68,7 @@ export const BookmarkItem = (props: Props): JSX.Element => {
     catch (err) {
       toastError(err);
     }
-  }, [bookmarkFolderTreeMutation, bookmarkedPage._id]);
+  }, [bookmarkFolderTreeMutation, bookmarkedPage]);
 
   const bookmarkMenuItemClickHandler = useCallback(async(pageId: string, shouldBookmark: boolean) => {
     if (shouldBookmark) {
@@ -90,6 +90,9 @@ export const BookmarkItem = (props: Props): JSX.Element => {
   }, []);
 
   const rename = useCallback(async(inputText: string) => {
+    if (bookmarkedPage == null) return;
+
+
     if (inputText.trim() === '') {
       return cancel();
     }
@@ -111,9 +114,11 @@ export const BookmarkItem = (props: Props): JSX.Element => {
       setRenameInputShown(true);
       toastError(err);
     }
-  }, [bookmarkedPage.path, bookmarkedPage._id, bookmarkedPage.revision, cancel, bookmarkFolderTreeMutation, mutatePageInfo]);
+  }, [bookmarkedPage, cancel, bookmarkFolderTreeMutation, mutatePageInfo]);
 
   const deleteMenuItemClickHandler = useCallback(async(_pageId: string, pageInfo: IPageInfoAll | undefined): Promise<void> => {
+    if (bookmarkedPage == null) return;
+
     if (bookmarkedPage._id == null || bookmarkedPage.path == null) {
       throw Error('_id and path must not be null.');
     }
@@ -128,9 +133,11 @@ export const BookmarkItem = (props: Props): JSX.Element => {
     };
 
     onClickDeleteMenuItemHandler(pageToDelete);
-  }, [bookmarkedPage._id, bookmarkedPage.path, bookmarkedPage.revision, onClickDeleteMenuItemHandler]);
+  }, [bookmarkedPage, onClickDeleteMenuItemHandler]);
 
   const putBackClickHandler = useCallback(() => {
+    if (bookmarkedPage == null) return;
+
     const { _id: pageId, path } = bookmarkedPage;
     const putBackedHandler = async() => {
       try {
@@ -148,6 +155,15 @@ export const BookmarkItem = (props: Props): JSX.Element => {
     openPutBackPageModal({ pageId, path }, { onPutBacked: putBackedHandler });
   }, [bookmarkedPage, openPutBackPageModal, bookmarkFolderTreeMutation, router, mutateCurrentPage, t]);
 
+  if (bookmarkedPage == null) {
+    return <></>;
+  }
+
+  const dPagePath = new DevidedPagePath(bookmarkedPage.path, false, true);
+  const { latter: pageTitle, former: formerPagePath } = dPagePath;
+
+  const bookmarkItemId = `bookmark-item-${bookmarkedPage._id}`;
+
   return (
     <DragAndDropWrapper
       item={dragItem}

apps/app/src/server/service/page/index.ts

@@ -1869,8 +1869,7 @@ class PageService implements IPageService {
   }
 
   async deleteCompletelyOperation(pageIds, pagePaths): Promise<void> {
-    // Delete Bookmarks, Attachments, Revisions, Pages and emit delete
-    const Bookmark = this.crowi.model('Bookmark');
+    // Delete Attachments, Revisions, Pages and emit delete
     const Page = this.crowi.model('Page');
     const Revision = this.crowi.model('Revision');
 
@@ -1878,7 +1877,6 @@ class PageService implements IPageService {
     const attachments = await Attachment.find({ page: { $in: pageIds } });
 
     await Promise.all([
-      Bookmark.deleteMany({ page: { $in: pageIds } }),
       Comment.deleteMany({ page: { $in: pageIds } }),
       PageTagRelation.deleteMany({ relatedPage: { $in: pageIds } }),
       ShareLink.deleteMany({ relatedPage: { $in: pageIds } }),
@@ -1886,6 +1884,8 @@ class PageService implements IPageService {
       Page.deleteMany({ _id: { $in: pageIds } }),
       PageRedirect.deleteMany({ $or: [{ fromPath: { $in: pagePaths } }, { toPath: { $in: pagePaths } }] }),
       attachmentService.removeAllAttachments(attachments),
+
+      // Leave bookmarks without deleting -- 2024.05.17 Yuki Takei
     ]);
   }
 

apps/app/test/integration/service/page.test.js

@@ -870,7 +870,9 @@ describe('PageService', () => {
       test('deleteCompletelyOperation', async() => {
         await crowi.pageService.deleteCompletelyOperation([parentForDeleteCompletely._id], [parentForDeleteCompletely.path], { });
 
-        expect(deleteManyBookmarkSpy).toHaveBeenCalledWith({ page: { $in: [parentForDeleteCompletely._id] } });
+        // bookmarks should not be deleted
+        expect(deleteManyBookmarkSpy).not.toHaveBeenCalled();
+
         expect(deleteManyCommentSpy).toHaveBeenCalledWith({ page: { $in: [parentForDeleteCompletely._id] } });
         expect(deleteManyPageTagRelationSpy).toHaveBeenCalledWith({ relatedPage: { $in: [parentForDeleteCompletely._id] } });
         expect(deleteManyShareLinkSpy).toHaveBeenCalledWith({ relatedPage: { $in: [parentForDeleteCompletely._id] } });

apps/app/test/integration/service/v5.public-page.test.ts

@@ -2103,7 +2103,7 @@ describe('PageService page operations with only public pages', () => {
       const deletedRevisions = await Revision.find({ pageId: { $in: [parentPage._id, grandchildPage._id] } });
       const tags = await Tag.find({ _id: { $in: [tag1?._id, tag2?._id] } });
       const deletedPageTagRelations = await PageTagRelation.find({ _id: { $in: [pageTagRelation1?._id, pageTagRelation2?._id] } });
-      const deletedBookmarks = await Bookmark.find({ _id: bookmark._id });
+      const remainingBookmarks = await Bookmark.find({ _id: bookmark._id });
       const deletedComments = await Comment.find({ _id: comment._id });
       const deletedPageRedirects = await PageRedirect.find({ _id: { $in: [pageRedirect1._id, pageRedirect2._id] } });
       const deletedShareLinks = await ShareLink.find({ _id: { $in: [shareLink1._id, shareLink2._id] } });
@@ -2117,7 +2117,7 @@ describe('PageService page operations with only public pages', () => {
       // PageTagRelation should be null
       expect(deletedPageTagRelations.length).toBe(0);
       // bookmark should be null
-      expect(deletedBookmarks.length).toBe(0);
+      expect(remainingBookmarks.length).toBe(1);
       // comment should be null
       expect(deletedComments.length).toBe(0);
       // pageRedirect should be null