fix: update bookmark handling to support empty pages and improve type safety

apps/app/src/client/components/Common/Dropdown/PageItemControl.tsx

@@ -76,9 +76,12 @@ const PageItemControlDropdownMenu = React.memo((props: DropdownMenuProps): JSX.E
 
   // eslint-disable-next-line react-hooks/rules-of-hooks
   const bookmarkItemClickedHandler = useCallback(async() => {
-    if (!isIPageInfoForOperation(pageInfo) || onClickBookmarkMenuItem == null) {
+    if (onClickBookmarkMenuItem == null) return;
+
+    if (!isIPageInfoForEmpty(pageInfo) && !isIPageInfoForOperation(pageInfo)) {
       return;
     }
+
     await onClickBookmarkMenuItem(pageId, !pageInfo.isBookmarked);
   }, [onClickBookmarkMenuItem, pageId, pageInfo]);
 
@@ -172,7 +175,7 @@ const PageItemControlDropdownMenu = React.memo((props: DropdownMenuProps): JSX.E
         ) }
 
         {/* Bookmark */}
-        { !forceHideMenuItems?.includes(MenuItemType.BOOKMARK) && isEnableActions && isIPageInfoForOperation(pageInfo) && (
+        { !forceHideMenuItems?.includes(MenuItemType.BOOKMARK) && isEnableActions && (isIPageInfoForEmpty(pageInfo) || isIPageInfoForOperation(pageInfo)) && (
           <DropdownItem
             onClick={bookmarkItemClickedHandler}
             className="grw-page-control-dropdown-item"

apps/app/src/interfaces/bookmark-info.ts

@@ -1,9 +1,15 @@
-import type { IPageHasId, IUser, Ref } from '@growi/core';
+import type { IPageHasId, IUser, Ref } from '@growi/core/dist/interfaces';
+import type { IUserSerializedSecurely } from '@growi/core/dist/models/serializers';
+
+export interface IBookmark {
+  page: Ref<IPageHasId>;
+  user: Ref<IUser>;
+}
 
 export interface IBookmarkInfo {
   sumOfBookmarks: number;
   isBookmarked: boolean;
-  bookmarkedUsers: IUser[];
+  bookmarkedUsers: IUserSerializedSecurely<IUser>[];
   pageId: string;
 }
 

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

@@ -1,48 +1,80 @@
-/* eslint-disable no-return-await */
-
-import mongoose from 'mongoose';
+import type { Document, Model, Types } from 'mongoose';
+import { Schema } from 'mongoose';
 import mongoosePaginate from 'mongoose-paginate-v2';
 import uniqueValidator from 'mongoose-unique-validator';
 
+import type { IBookmark } from '~/interfaces/bookmark-info';
 import loggerFactory from '~/utils/logger';
 
-const logger = loggerFactory('growi:models:bookmark');
+import type Crowi from '../crowi';
+import { getOrCreateModel } from '../util/mongoose-utils';
 
-const ObjectId = mongoose.Schema.Types.ObjectId;
+const logger = loggerFactory('growi:models:bookmark');
 
-/** @param {import('~/server/crowi').default} crowi Crowi instance */
-const factory = (crowi) => {
+export interface BookmarkDocument extends IBookmark, Document {
+  _id: Types.ObjectId;
+  page: Types.ObjectId;
+  user: Types.ObjectId;
+  createdAt: Date;
+}
+
+export interface BookmarkModel extends Model<BookmarkDocument> {
+  countByPageId(pageId: Types.ObjectId | string): Promise<number>;
+  getPageIdToCountMap(
+    pageIds: Types.ObjectId[],
+  ): Promise<{ [key: string]: number }>;
+  findByPageIdAndUserId(
+    pageId: Types.ObjectId | string,
+    userId: Types.ObjectId | string,
+  ): Promise<BookmarkDocument | null>;
+  add(
+    page: Types.ObjectId | string,
+    user: Types.ObjectId | string,
+  ): Promise<BookmarkDocument>;
+  removeBookmarksByPageId(
+    pageId: Types.ObjectId | string,
+  ): Promise<{ deletedCount: number }>;
+  removeBookmark(
+    pageId: Types.ObjectId | string,
+    user: Types.ObjectId | string,
+  ): Promise<BookmarkDocument | null>;
+}
+
+const factory = (crowi: Crowi) => {
   const bookmarkEvent = crowi.event('bookmark');
 
-  let bookmarkSchema = null;
-
-  bookmarkSchema = new mongoose.Schema(
+  const bookmarkSchema = new Schema<BookmarkDocument, BookmarkModel>(
     {
-      page: { type: ObjectId, ref: 'Page', index: true },
-      user: { type: ObjectId, ref: 'User', index: true },
+      page: { type: Schema.Types.ObjectId, ref: 'Page', index: true },
+      user: { type: Schema.Types.ObjectId, ref: 'User', index: true },
     },
     {
       timestamps: { createdAt: true, updatedAt: false },
     },
   );
+
   bookmarkSchema.index({ page: 1, user: 1 }, { unique: true });
   bookmarkSchema.plugin(mongoosePaginate);
   bookmarkSchema.plugin(uniqueValidator);
 
-  bookmarkSchema.statics.countByPageId = async function (pageId) {
-    return await this.count({ page: pageId });
+  bookmarkSchema.statics.countByPageId = async function (
+    pageId: Types.ObjectId | string,
+  ): Promise<number> {
+    return await this.countDocuments({ page: pageId });
   };
 
   /**
    * @return {object} key: page._id, value: bookmark count
    */
-  bookmarkSchema.statics.getPageIdToCountMap = async function (pageIds) {
+  bookmarkSchema.statics.getPageIdToCountMap = async function (
+    pageIds: Types.ObjectId[],
+  ): Promise<{ [key: string]: number }> {
     const results = await this.aggregate()
       .match({ page: { $in: pageIds } })
       .group({ _id: '$page', count: { $sum: 1 } });
 
     // convert to map
-    const idToCountMap = {};
+    const idToCountMap: { [key: string]: number } = {};
     results.forEach((result) => {
       idToCountMap[result._id] = result.count;
     });
@@ -51,29 +83,24 @@ const factory = (crowi) => {
   };
 
   // bookmark チェック用
-  bookmarkSchema.statics.findByPageIdAndUserId = function (pageId, userId) {
-    return new Promise((resolve, reject) => {
-      return this.findOne({ page: pageId, user: userId }, (err, doc) => {
-        if (err) {
-          return reject(err);
-        }
-
-        return resolve(doc);
-      });
-    });
+  bookmarkSchema.statics.findByPageIdAndUserId = async function (
+    pageId: Types.ObjectId | string,
+    userId: Types.ObjectId | string,
+  ): Promise<BookmarkDocument | null> {
+    return await this.findOne({ page: pageId, user: userId });
   };
 
-  bookmarkSchema.statics.add = async function (page, user) {
-    // biome-ignore lint/complexity/noUselessThisAlias: ignore
-    const Bookmark = this;
-
-    const newBookmark = new Bookmark({ page, user });
+  bookmarkSchema.statics.add = async function (
+    page: Types.ObjectId | string,
+    user: Types.ObjectId | string,
+  ): Promise<BookmarkDocument> {
+    const newBookmark = new this({ page, user });
 
     try {
       const bookmark = await newBookmark.save();
-      bookmarkEvent.emit('create', page._id);
+      bookmarkEvent.emit('create', page);
       return bookmark;
-    } catch (err) {
+    } catch (err: any) {
       if (err.code === 11000) {
         // duplicate key (dummy response of new object)
         return newBookmark;
@@ -88,26 +115,25 @@ const factory = (crowi) => {
    * used only when removing the page
    * @param {string} pageId
    */
-  bookmarkSchema.statics.removeBookmarksByPageId = async function (pageId) {
-    // biome-ignore lint/complexity/noUselessThisAlias: ignore
-    const Bookmark = this;
-
+  bookmarkSchema.statics.removeBookmarksByPageId = async function (
+    pageId: Types.ObjectId | string,
+  ): Promise<{ deletedCount: number }> {
     try {
-      const data = await Bookmark.remove({ page: pageId });
+      const result = await this.deleteMany({ page: pageId });
       bookmarkEvent.emit('delete', pageId);
-      return data;
+      return { deletedCount: result.deletedCount ?? 0 };
     } catch (err) {
       logger.debug('Bookmark.remove failed (removeBookmarkByPage)', err);
       throw err;
     }
   };
 
-  bookmarkSchema.statics.removeBookmark = async function (pageId, user) {
-    // biome-ignore lint/complexity/noUselessThisAlias: ignore
-    const Bookmark = this;
-
+  bookmarkSchema.statics.removeBookmark = async function (
+    pageId: Types.ObjectId | string,
+    user: Types.ObjectId | string,
+  ): Promise<BookmarkDocument | null> {
     try {
-      const data = await Bookmark.findOneAndRemove({ page: pageId, user });
+      const data = await this.findOneAndDelete({ page: pageId, user });
       bookmarkEvent.emit('delete', pageId);
       return data;
     } catch (err) {
@@ -116,7 +142,10 @@ const factory = (crowi) => {
     }
   };
 
-  return mongoose.model('Bookmark', bookmarkSchema);
+  return getOrCreateModel<BookmarkDocument, BookmarkModel>(
+    'Bookmark',
+    bookmarkSchema,
+  );
 };
 
 export default factory;

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

@@ -1,9 +1,14 @@
+import type { IUserHasId } from '@growi/core';
 import { SCOPE } from '@growi/core/dist/interfaces';
 import { serializeUserSecurely } from '@growi/core/dist/models/serializers';
+import mongoose, { type HydratedDocument } from 'mongoose';
 
 import { SupportedAction, SupportedTargetModel } from '~/interfaces/activity';
+import type { IBookmarkInfo } from '~/interfaces/bookmark-info';
 import { accessTokenParser } from '~/server/middlewares/access-token-parser';
 import { generateAddActivityMiddleware } from '~/server/middlewares/add-activity';
+import type { BookmarkDocument, BookmarkModel } from '~/server/models/bookmark';
+import type { PageDocument, PageModel } from '~/server/models/page';
 import { serializeBookmarkSecurely } from '~/server/models/serializers/bookmark-serializer';
 import { preNotifyService } from '~/server/service/pre-notify';
 import loggerFactory from '~/utils/logger';
@@ -91,15 +96,16 @@ module.exports = (crowi) => {
     crowi,
     true,
   );
-  const addActivity = generateAddActivityMiddleware(crowi);
+  const addActivity = generateAddActivityMiddleware();
 
   const activityEvent = crowi.event('activity');
 
-  const { Page, Bookmark } = crowi.models;
-
   const validator = {
     bookmarks: [body('pageId').isString(), body('bool').isBoolean()],
     bookmarkInfo: [query('pageId').isMongoId()],
+    userBookmarkList: [
+      param('userId').isMongoId().withMessage('userId is required'),
+    ],
   };
 
   /**
@@ -134,18 +140,25 @@ module.exports = (crowi) => {
       const { user } = req;
       const { pageId } = req.query;
 
-      const responsesParams = {};
+      const responsesParams: IBookmarkInfo = {
+        sumOfBookmarks: 0,
+        isBookmarked: false,
+        bookmarkedUsers: [],
+        pageId: '',
+      };
+
+      const Bookmark: BookmarkModel = mongoose.model<
+        HydratedDocument<BookmarkDocument>,
+        BookmarkModel
+      >('Bookmark');
 
       try {
-        const bookmarks = await Bookmark.find({ page: pageId }).populate(
-          'user',
+        const bookmarks = await Bookmark.find({ page: pageId }).populate<{
+          user: IUserHasId;
+        }>('user');
+        const users = bookmarks.map((bookmark) =>
+          serializeUserSecurely(bookmark.user),
         );
-        let users = [];
-        if (bookmarks.length > 0) {
-          users = bookmarks.map((bookmark) =>
-            serializeUserSecurely(bookmark.user),
-          );
-        }
         responsesParams.sumOfBookmarks = bookmarks.length;
         responsesParams.bookmarkedUsers = users;
         responsesParams.pageId = pageId;
@@ -194,10 +207,6 @@ module.exports = (crowi) => {
    *                schema:
    *                  $ref: '#/components/schemas/Bookmarks'
    */
-  validator.userBookmarkList = [
-    param('userId').isMongoId().withMessage('userId is required'),
-  ];
-
   router.get(
     '/:userId',
     accessTokenParser([SCOPE.READ.FEATURES.BOOKMARK], { acceptLegacy: true }),
@@ -210,6 +219,12 @@ module.exports = (crowi) => {
       if (userId == null) {
         return res.apiv3Err('User id is not found or forbidden', 400);
       }
+
+      const Bookmark: BookmarkModel = mongoose.model<
+        HydratedDocument<BookmarkDocument>,
+        BookmarkModel
+      >('Bookmark');
+
       try {
         const bookmarkIdsInFolders = await BookmarkFolder.distinct(
           'bookmarks',
@@ -281,8 +296,17 @@ module.exports = (crowi) => {
         return res.apiv3Err('A logged in user is required.');
       }
 
-      let page;
-      let bookmark;
+      const Page: PageModel = mongoose.model<
+        HydratedDocument<PageDocument>,
+        PageModel
+      >('Page');
+      const Bookmark: BookmarkModel = mongoose.model<
+        HydratedDocument<BookmarkDocument>,
+        BookmarkModel
+      >('Bookmark');
+
+      let page: HydratedDocument<PageDocument> | null;
+      let bookmark: HydratedDocument<BookmarkDocument> | null;
       try {
         page = await Page.findByIdAndViewer(pageId, req.user);
         if (page == null) {
@@ -293,7 +317,7 @@ module.exports = (crowi) => {
 
         if (bookmark == null) {
           if (bool) {
-            bookmark = await Bookmark.add(page, req.user);
+            bookmark = await Bookmark.add(page._id, req.user);
           } else {
             logger.warn(
               `Removing the bookmark for ${page._id} by ${req.user._id} failed because the bookmark does not exist.`,
@@ -306,7 +330,7 @@ module.exports = (crowi) => {
               `Adding the bookmark for ${page._id} by ${req.user._id} failed because the bookmark has already exist.`,
             );
           } else {
-            bookmark = await Bookmark.removeBookmark(page, req.user);
+            bookmark = await Bookmark.removeBookmark(page._id, req.user);
           }
         }
       } catch (err) {

apps/app/src/stores/bookmark.ts

@@ -1,4 +1,5 @@
-import type { IPageHasId, IUser } from '@growi/core';
+import type { IPageHasId, IUser } from '@growi/core/dist/interfaces';
+import type { IUserSerializedSecurely } from '@growi/core/dist/models/serializers';
 import type { SWRResponse } from 'swr';
 import useSWR from 'swr';
 import useSWRImmutable from 'swr/immutable';
@@ -11,7 +12,7 @@ import type { IBookmarkInfo } from '../interfaces/bookmark-info';
 
 export const useSWRxBookmarkedUsers = (
   pageId: string | null,
-): SWRResponse<IUser[], Error> => {
+): SWRResponse<IUserSerializedSecurely<IUser>[], Error> => {
   return useSWR(
     pageId != null ? `/bookmarks/info?pageId=${pageId}` : null,
     (endpoint) =>