Merge pull request #7673 from weseek/fix/gw7955-fix-user-icon-on-bookmarks

fix:  User picture of bookmark not showing inside bookmark folder

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

@@ -65,14 +65,15 @@ export const BookmarkFolderItem: FC<BookmarkFolderItemProps> = (props: BookmarkF
   // Rename for bookmark folder handler
   const onPressEnterHandlerForRename = useCallback(async(folderName: string) => {
     try {
-      await updateBookmarkFolder(folderId, folderName, parent);
+      // TODO: do not use any type
+      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) => {
@@ -99,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();
         }
       }
@@ -200,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/interfaces/bookmark-info.ts

@@ -3,13 +3,13 @@ import { Ref } from '@growi/core';
 import { IPageHasId } from '~/interfaces/page';
 import { IUser } from '~/interfaces/user';
 
-export type IBookmarkInfo = {
+export interface IBookmarkInfo {
   sumOfBookmarks: number;
   isBookmarked: boolean,
   bookmarkedUsers: IUser[]
-};
+}
 
-type BookmarkedPage = {
+export interface BookmarkedPage {
   _id: string,
   page: IPageHasId,
   user: Ref<IUser>,
@@ -24,12 +24,10 @@ export interface IBookmarkFolder {
   parent?: Ref<this>
 }
 
-export interface BookmarkFolderItems {
-  _id: string
-  name: string
-  parent: string
-  children: this[]
-  bookmarks: BookmarkedPage[]
+export interface BookmarkFolderItems extends IBookmarkFolder {
+  _id: string;
+  children: BookmarkFolderItems[];
+  bookmarks: BookmarkedPage[];
 }
 
 export const DRAG_ITEM_TYPE = {
@@ -37,15 +35,14 @@ export const DRAG_ITEM_TYPE = {
   BOOKMARK: 'BOOKMARK',
 } as const;
 
-type BookmarkDragItem = {
+interface BookmarkDragItem {
   bookmarkFolder: BookmarkFolderItems
   level: number
   root: string
 }
 
-export type DragItemDataType = BookmarkDragItem & {
+export interface DragItemDataType extends BookmarkDragItem, IPageHasId {
   parentFolder: BookmarkFolderItems | null
-} & IPageHasId
-
+}
 
 export type DragItemType = typeof DRAG_ITEM_TYPE[keyof typeof DRAG_ITEM_TYPE];

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

@@ -3,7 +3,7 @@ import monggoose, {
   Types, Document, Model, Schema,
 } from 'mongoose';
 
-import { IBookmarkFolder, BookmarkFolderItems, MyBookmarkList } from '~/interfaces/bookmark-info';
+import { BookmarkFolderItems, IBookmarkFolder } from '~/interfaces/bookmark-info';
 import { IPageHasId } from '~/interfaces/page';
 
 import loggerFactory from '../../utils/logger';
@@ -26,11 +26,9 @@ export interface BookmarkFolderDocument extends Document {
 
 export interface BookmarkFolderModel extends Model<BookmarkFolderDocument>{
   createByParameters(params: IBookmarkFolder): Promise<BookmarkFolderDocument>
-  findFolderAndChildren(user: Types.ObjectId | string, parentId?: Types.ObjectId | string): Promise<BookmarkFolderItems[]>
   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>
-  findUserRootBookmarksItem(userId: Types.ObjectId| string): Promise<MyBookmarkList>
   updateBookmark(pageId: Types.ObjectId | string, status: boolean, userId: Types.ObjectId| string): Promise<BookmarkFolderDocument | null>
 }
 
@@ -83,45 +81,6 @@ bookmarkFolderSchema.statics.createByParameters = async function(params: IBookma
   return bookmarkFolder;
 };
 
-bookmarkFolderSchema.statics.findFolderAndChildren = async function(
-    userId: Types.ObjectId | string,
-    parentId?: Types.ObjectId | string,
-): Promise<BookmarkFolderItems[]> {
-  const folderItems: BookmarkFolderItems[] = [];
-
-  const folders = await this.find({ owner: userId, parent: parentId })
-    .populate('children')
-    .populate({
-      path: 'bookmarks',
-      model: 'Bookmark',
-      populate: {
-        path: 'page',
-        model: 'Page',
-      },
-    });
-
-  const promises = folders.map(async(folder) => {
-    const children = await this.findFolderAndChildren(userId, folder._id);
-    const {
-      _id, name, owner, bookmarks, parent,
-    } = folder;
-
-    const res = {
-      _id: _id.toString(),
-      name,
-      owner,
-      bookmarks,
-      children,
-      parent,
-    };
-    return res;
-  });
-
-  const results = await Promise.all(promises) as unknown as BookmarkFolderItems[];
-  folderItems.push(...results);
-  return folderItems;
-};
-
 bookmarkFolderSchema.statics.deleteFolderAndChildren = async function(bookmarkFolderId: Types.ObjectId | string): Promise<{deletedCount: number}> {
   const bookmarkFolder = await this.findById(bookmarkFolderId);
   // Delete parent and all children folder
@@ -145,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: '',
@@ -163,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).populate('children');
-    if (bookmarkFolder?.children?.length !== 0) {
+    if (children.length !== 0) {
       throw new Error('Update bookmark folder failed');
     }
   }
@@ -198,22 +161,6 @@ Promise<BookmarkFolderDocument | null> {
   return bookmarkFolder;
 };
 
-bookmarkFolderSchema.statics.findUserRootBookmarksItem = async function(userId: Types.ObjectId | string): Promise<MyBookmarkList> {
-  const bookmarkIdsInFolders = await this.distinct('bookmarks', { owner: userId });
-  const userRootBookmarks: MyBookmarkList = await Bookmark.find({
-    _id: { $nin: bookmarkIdsInFolders },
-    user: userId,
-  }).populate({
-    path: 'page',
-    model: 'Page',
-    populate: {
-      path: 'lastUpdateUser',
-      model: 'User',
-    },
-  });
-  return userRootBookmarks;
-};
-
 bookmarkFolderSchema.statics.updateBookmark = async function(pageId: Types.ObjectId | string, status: boolean, userId: Types.ObjectId | string):
 Promise<BookmarkFolderDocument | null> {
   // If isBookmarked

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

@@ -1,14 +1,16 @@
 import { ErrorV3 } from '@growi/core';
 import { body } from 'express-validator';
+import { Types } from 'mongoose';
 
+import { BookmarkFolderItems } from '~/interfaces/bookmark-info';
 import { apiV3FormValidator } from '~/server/middlewares/apiv3-form-validator';
 import { InvalidParentBookmarkFolderError } from '~/server/models/errors';
+import { serializeBookmarkSecurely } from '~/server/models/serializers/bookmark-serializer';
 import loggerFactory from '~/utils/logger';
 
 import BookmarkFolder from '../../models/bookmark-folder';
 
 const logger = loggerFactory('growi:routes:apiv3:bookmark-folder');
-
 const express = require('express');
 
 const router = express.Router();
@@ -23,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'),
@@ -52,6 +56,7 @@ module.exports = (crowi) => {
       return res.apiv3({ bookmarkFolder });
     }
     catch (err) {
+      logger.error(err);
       if (err instanceof InvalidParentBookmarkFolderError) {
         return res.apiv3Err(new ErrorV3(err.message, 'failed_to_create_bookmark_folder'));
       }
@@ -63,12 +68,57 @@ module.exports = (crowi) => {
   router.get('/list/:userId', accessTokenParser, loginRequiredStrictly, async(req, res) => {
     const { userId } = req.params;
 
+    const getBookmarkFolders = async(
+        userId: Types.ObjectId | string,
+        parentFolderId?: Types.ObjectId | string,
+    ) => {
+      const folders = await BookmarkFolder.find({ owner: userId, parent: parentFolderId })
+        .populate('children')
+        .populate({
+          path: 'bookmarks',
+          model: 'Bookmark',
+          populate: {
+            path: 'page',
+            model: 'Page',
+            populate: {
+              path: 'lastUpdateUser',
+              model: 'User',
+            },
+          },
+        }).exec();
+
+      const returnValue: BookmarkFolderItems[] = [];
+
+      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,
+          children,
+          parent: folder.parent,
+        };
+        return res;
+      });
+
+      const results = await Promise.all(promises) as unknown as BookmarkFolderItems[];
+      returnValue.push(...results);
+      return returnValue;
+    };
+
     try {
-      const bookmarkFolderItems = await BookmarkFolder.findFolderAndChildren(userId);
+      const bookmarkFolderItems = await getBookmarkFolders(userId, undefined);
 
       return res.apiv3({ bookmarkFolderItems });
     }
     catch (err) {
+      logger.error(err);
       return res.apiv3Err(err, 500);
     }
   });
@@ -88,12 +138,15 @@ 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) {
+      logger.error(err);
       return res.apiv3Err(err, 500);
     }
   });
@@ -108,6 +161,7 @@ module.exports = (crowi) => {
       return res.apiv3({ bookmarkFolder });
     }
     catch (err) {
+      logger.error(err);
       return res.apiv3Err(err, 500);
     }
   });
@@ -120,6 +174,7 @@ module.exports = (crowi) => {
       return res.apiv3({ bookmarkFolder });
     }
     catch (err) {
+      logger.error(err);
       return res.apiv3Err(err, 500);
     }
   });

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

@@ -1,5 +1,6 @@
 import { SupportedAction, SupportedTargetModel } from '~/interfaces/activity';
 import { generateAddActivityMiddleware } from '~/server/middlewares/add-activity';
+import { serializeBookmarkSecurely } from '~/server/models/serializers/bookmark-serializer';
 import loggerFactory from '~/utils/logger';
 
 import { apiV3FormValidator } from '../../middlewares/apiv3-form-validator';
@@ -201,14 +202,23 @@ module.exports = (crowi) => {
       return res.apiv3Err('User id is not found or forbidden', 400);
     }
     try {
-      const userRootBookmarks = await BookmarkFolder.findUserRootBookmarksItem(userId);
-      userRootBookmarks.forEach((bookmark) => {
-        if (bookmark.page.lastUpdateUser != null && bookmark.page.lastUpdateUser instanceof User) {
-          bookmark.page.lastUpdateUser = serializeUserSecurely(bookmark.page.lastUpdateUser);
-        }
-      });
+      const bookmarkIdsInFolders = await BookmarkFolder.distinct('bookmarks', { owner: userId });
+      const userRootBookmarks = await Bookmark.find({
+        _id: { $nin: bookmarkIdsInFolders },
+        user: userId,
+      }).populate({
+        path: 'page',
+        model: 'Page',
+        populate: {
+          path: 'lastUpdateUser',
+          model: 'User',
+        },
+      }).exec();
+
+      // 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);