Merge pull request #7685 from weseek/fix/122556-122841-do-not-show-user-icon-extra

fix: Update bookmark find serialize process

apps/app/src/client/util/bookmark-utils.ts

@@ -41,6 +41,8 @@ export const toggleBookmark = async(pageId: string, status: boolean): Promise<vo
 };
 
 // Update Bookmark folder
-export const updateBookmarkFolder = async(bookmarkFolderId: string, name: string, parent: string | null): Promise<void> => {
-  await apiv3Put('/bookmark-folder', { bookmarkFolderId, name, parent });
+export const updateBookmarkFolder = async(bookmarkFolderId: string, name: string, parent: string | null, children: BookmarkFolderItems[]): Promise<void> => {
+  await apiv3Put('/bookmark-folder', {
+    bookmarkFolderId, name, parent, children,
+  });
 };

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

@@ -66,14 +66,14 @@ export const BookmarkFolderItem: FC<BookmarkFolderItemProps> = (props: BookmarkF
   const onPressEnterHandlerForRename = useCallback(async(folderName: string) => {
     try {
       // TODO: do not use any type
-      await updateBookmarkFolder(folderId, folderName, parent as any);
+      await updateBookmarkFolder(folderId, folderName, parent as any, children);
       bookmarkFolderTreeMutation();
       setIsRenameAction(false);
     }
     catch (err) {
       toastError(err);
     }
-  }, [bookmarkFolderTreeMutation, folderId, parent]);
+  }, [bookmarkFolderTreeMutation, children, folderId, parent]);
 
   // Create new folder / subfolder handler
   const onPressEnterHandlerForCreate = useCallback(async(folderName: string) => {
@@ -100,7 +100,7 @@ export const BookmarkFolderItem: FC<BookmarkFolderItemProps> = (props: BookmarkF
     if (dragItemType === DRAG_ITEM_TYPE.FOLDER) {
       try {
         if (item.bookmarkFolder != null) {
-          await updateBookmarkFolder(item.bookmarkFolder._id, item.bookmarkFolder.name, bookmarkFolder._id);
+          await updateBookmarkFolder(item.bookmarkFolder._id, item.bookmarkFolder.name, bookmarkFolder._id, item.bookmarkFolder.children);
           bookmarkFolderTreeMutation();
         }
       }
@@ -201,13 +201,13 @@ export const BookmarkFolderItem: FC<BookmarkFolderItemProps> = (props: BookmarkF
 
   const onClickMoveToRootHandlerForBookmarkFolderItemControl = useCallback(async() => {
     try {
-      await updateBookmarkFolder(bookmarkFolder._id, bookmarkFolder.name, null);
+      await updateBookmarkFolder(bookmarkFolder._id, bookmarkFolder.name, null, bookmarkFolder.children);
       bookmarkFolderTreeMutation();
     }
     catch (err) {
       toastError(err);
     }
-  }, [bookmarkFolder._id, bookmarkFolder.name, bookmarkFolderTreeMutation]);
+  }, [bookmarkFolder._id, bookmarkFolder.children, bookmarkFolder.name, bookmarkFolderTreeMutation]);
 
   return (
     <div id={`grw-bookmark-folder-item-${folderId}`} className="grw-foldertree-item-container">

apps/app/src/server/models/bookmark-folder.ts

@@ -3,7 +3,7 @@ import monggoose, {
   Types, Document, Model, Schema,
 } from 'mongoose';
 
-import { IBookmarkFolder } from '~/interfaces/bookmark-info';
+import { BookmarkFolderItems, IBookmarkFolder } from '~/interfaces/bookmark-info';
 import { IPageHasId } from '~/interfaces/page';
 
 import loggerFactory from '../../utils/logger';
@@ -27,7 +27,7 @@ export interface BookmarkFolderDocument extends Document {
 export interface BookmarkFolderModel extends Model<BookmarkFolderDocument>{
   createByParameters(params: IBookmarkFolder): Promise<BookmarkFolderDocument>
   deleteFolderAndChildren(bookmarkFolderId: Types.ObjectId | string): Promise<{deletedCount: number}>
-  updateBookmarkFolder(bookmarkFolderId: string, name: string, parent: string | null): Promise<BookmarkFolderDocument>
+  updateBookmarkFolder(bookmarkFolderId: string, name: string, parent: string | null, children: BookmarkFolderItems[]): Promise<BookmarkFolderDocument>
   insertOrUpdateBookmarkedPage(pageId: IPageHasId, userId: Types.ObjectId | string, folderId: string | null): Promise<BookmarkFolderDocument | null>
   updateBookmark(pageId: Types.ObjectId | string, status: boolean, userId: Types.ObjectId| string): Promise<BookmarkFolderDocument | null>
 }
@@ -104,7 +104,12 @@ bookmarkFolderSchema.statics.deleteFolderAndChildren = async function(bookmarkFo
   return { deletedCount };
 };
 
-bookmarkFolderSchema.statics.updateBookmarkFolder = async function(bookmarkFolderId: string, name: string, parentId: string | null):
+bookmarkFolderSchema.statics.updateBookmarkFolder = async function(
+    bookmarkFolderId: string,
+    name: string,
+    parentId: string | null,
+    children: BookmarkFolderItems[],
+):
  Promise<BookmarkFolderDocument> {
   const updateFields: {name: string, parent: Types.ObjectId | null} = {
     name: '',
@@ -122,8 +127,7 @@ bookmarkFolderSchema.statics.updateBookmarkFolder = async function(bookmarkFolde
     if (parentFolder?.parent != null) {
       throw new Error('Update bookmark folder failed');
     }
-    const bookmarkFolder = await this.findById(bookmarkFolderId);
-    if (bookmarkFolder?.children?.length !== 0) {
+    if (children.length !== 0) {
       throw new Error('Update bookmark folder failed');
     }
   }

apps/app/src/server/models/serializers/bookmark-serializer.js

@@ -0,0 +1,25 @@
+const { serializePageSecurely } = require('./page-serializer');
+
+function serializeInsecurePageAttributes(bookmark) {
+  if (bookmark.page != null && bookmark.page._id != null) {
+    bookmark.page = serializePageSecurely(bookmark.page);
+  }
+  return bookmark;
+}
+
+function serializeBookmarkSecurely(bookmark) {
+  let serialized = bookmark;
+
+  // invoke toObject if bookmark is a model instance
+  if (bookmark.toObject != null) {
+    serialized = bookmark.toObject();
+  }
+
+  serializeInsecurePageAttributes(serialized);
+
+  return serialized;
+}
+
+module.exports = {
+  serializeBookmarkSecurely,
+};

apps/app/src/server/routes/apiv3/bookmark-folder.ts

@@ -2,10 +2,10 @@ import { ErrorV3 } from '@growi/core';
 import { body } from 'express-validator';
 import { Types } from 'mongoose';
 
-import { BookmarkFolderItems, BookmarkedPage } from '~/interfaces/bookmark-info';
+import { BookmarkFolderItems } from '~/interfaces/bookmark-info';
 import { apiV3FormValidator } from '~/server/middlewares/apiv3-form-validator';
 import { InvalidParentBookmarkFolderError } from '~/server/models/errors';
-import { serializePageSecurely } from '~/server/models/serializers/page-serializer';
+import { serializeBookmarkSecurely } from '~/server/models/serializers/bookmark-serializer';
 import loggerFactory from '~/utils/logger';
 
 import BookmarkFolder from '../../models/bookmark-folder';
@@ -13,8 +13,6 @@ import BookmarkFolder from '../../models/bookmark-folder';
 const logger = loggerFactory('growi:routes:apiv3:bookmark-folder');
 const express = require('express');
 
-const { serializeUserSecurely } = require('../../models/serializers/user-serializer');
-
 const router = express.Router();
 
 const validator = {
@@ -27,6 +25,8 @@ const validator = {
           throw new Error('Maximum folder hierarchy of 2 levels');
         }
       }),
+    body('children').optional().isArray().withMessage('Children must be an array'),
+    body('bookmarkFolderId').optional().isMongoId().withMessage('Bookark Folder ID must be a valid mongo ID'),
   ],
   bookmarkPage: [
     body('pageId').isMongoId().withMessage('Page ID must be a valid mongo ID'),
@@ -72,9 +72,6 @@ module.exports = (crowi) => {
         userId: Types.ObjectId | string,
         parentFolderId?: Types.ObjectId | string,
     ) => {
-      const Page = crowi.model('Page');
-      const User = crowi.model('User');
-
       const folders = await BookmarkFolder.find({ owner: userId, parent: parentFolderId })
         .populate('children')
         .populate({
@@ -88,30 +85,22 @@ module.exports = (crowi) => {
               model: 'User',
             },
           },
-        });
+        }).exec();
 
       const returnValue: BookmarkFolderItems[] = [];
 
-      // serialize page and user
-      folders.forEach((folder: BookmarkFolderItems) => {
-        folder.bookmarks.forEach((bookmark: BookmarkedPage) => {
-          if (bookmark.page != null && bookmark.page instanceof Page) {
-            bookmark.page = serializePageSecurely(bookmark.page);
-          }
-          if (bookmark.page.lastUpdateUser != null && bookmark.page.lastUpdateUser instanceof User) {
-            bookmark.page.lastUpdateUser = serializeUserSecurely(bookmark.page.lastUpdateUser);
-          }
-        });
-      });
-
       const promises = folders.map(async(folder: BookmarkFolderItems) => {
         const children = await getBookmarkFolders(userId, folder._id);
 
+        // !! DO NOT THIS SERIALIZING OUTSIDE OF PROMISES !! -- 05.23.2023 ryoji-s
+        // Serializing outside of promises will cause not populated.
+        const bookmarks = folder.bookmarks.map(bookmark => serializeBookmarkSecurely(bookmark));
+
         const res = {
           _id: folder._id.toString(),
           name: folder.name,
           owner: folder.owner,
-          bookmarks: folder.bookmarks,
+          bookmarks,
           children,
           parent: folder.parent,
         };
@@ -149,9 +138,11 @@ module.exports = (crowi) => {
   });
 
   router.put('/', accessTokenParser, loginRequiredStrictly, validator.bookmarkFolder, async(req, res) => {
-    const { bookmarkFolderId, name, parent } = req.body;
+    const {
+      bookmarkFolderId, name, parent, children,
+    } = req.body;
     try {
-      const bookmarkFolder = await BookmarkFolder.updateBookmarkFolder(bookmarkFolderId, name, parent);
+      const bookmarkFolder = await BookmarkFolder.updateBookmarkFolder(bookmarkFolderId, name, parent, children);
       return res.apiv3({ bookmarkFolder });
     }
     catch (err) {

apps/app/src/server/routes/apiv3/bookmarks.js

@@ -1,6 +1,6 @@
 import { SupportedAction, SupportedTargetModel } from '~/interfaces/activity';
 import { generateAddActivityMiddleware } from '~/server/middlewares/add-activity';
-import { serializePageSecurely } from '~/server/models/serializers/page-serializer';
+import { serializeBookmarkSecurely } from '~/server/models/serializers/bookmark-serializer';
 import loggerFactory from '~/utils/logger';
 
 import { apiV3FormValidator } from '../../middlewares/apiv3-form-validator';
@@ -213,18 +213,12 @@ module.exports = (crowi) => {
           path: 'lastUpdateUser',
           model: 'User',
         },
-      });
+      }).exec();
 
-      userRootBookmarks.forEach((bookmark) => {
-        if (bookmark.page != null && bookmark.page instanceof Page) {
-          bookmark.page = serializePageSecurely(bookmark.page);
-        }
-        if (bookmark.page.lastUpdateUser != null && bookmark.page.lastUpdateUser instanceof User) {
-          bookmark.page.lastUpdateUser = serializeUserSecurely(bookmark.page.lastUpdateUser);
-        }
-      });
+      // serialize Bookmark
+      const serializedUserRootBookmarks = userRootBookmarks.map(bookmark => serializeBookmarkSecurely(bookmark));
 
-      return res.apiv3({ userRootBookmarks });
+      return res.apiv3({ userRootBookmarks: serializedUserRootBookmarks });
     }
     catch (err) {
       logger.error('get-bookmark-failed', err);