Merge pull request #5980 from weseek/imprv/96622-use-add-activity-middleware-with-page-event

imprv: Use add activity middleware with page event

packages/app/src/interfaces/activity.ts

@@ -3,6 +3,7 @@ import { IUser } from './user';
 
 // Model
 const MODEL_PAGE = 'Page';
+const MODEL_COMMENT = 'Comment';
 
 // Action
 const ACTION_UNSETTLED = 'UNSETTLED';
@@ -10,7 +11,9 @@ const ACTION_LOGIN_SUCCESS = 'LOGIN_SUCCESS';
 const ACTION_LOGIN_FAILURE = 'LOGIN_FAILURE';
 const ACTION_LOGOUT = 'LOGOUT';
 const ACTION_PAGE_LIKE = 'PAGE_LIKE';
+const ACTION_PAGE_UNLIKE = 'PAGE_UNLIKE';
 const ACTION_PAGE_BOOKMARK = 'PAGE_BOOKMARK';
+const ACTION_PAGE_UNBOOKMARK = 'PAGE_UNBOOKMARK';
 const ACTION_PAGE_CREATE = 'PAGE_CREATE';
 const ACTION_PAGE_UPDATE = 'PAGE_UPDATE';
 const ACTION_PAGE_RENAME = 'PAGE_RENAME';
@@ -26,13 +29,19 @@ export const SUPPORTED_TARGET_MODEL_TYPE = {
   MODEL_PAGE,
 } as const;
 
+export const SUPPORTED_EVENT_MODEL_TYPE = {
+  MODEL_COMMENT,
+} as const;
+
 export const SUPPORTED_ACTION_TYPE = {
   ACTION_LOGIN_SUCCESS,
   ACTION_LOGIN_FAILURE,
   ACTION_LOGOUT,
   ACTION_UNSETTLED,
   ACTION_PAGE_LIKE,
+  ACTION_PAGE_UNLIKE,
   ACTION_PAGE_BOOKMARK,
+  ACTION_PAGE_UNBOOKMARK,
   ACTION_PAGE_CREATE,
   ACTION_PAGE_UPDATE,
   ACTION_PAGE_RENAME,
@@ -54,11 +63,11 @@ export const SUPPORTED_ACTION_TO_NOTIFIED_TYPE = {
   ACTION_PAGE_DELETE_COMPLETELY,
   ACTION_PAGE_REVERT,
   ACTION_COMMENT_CREATE,
-  ACTION_COMMENT_UPDATE,
 } as const;
 
 
 export const AllSupportedTargetModelType = Object.values(SUPPORTED_TARGET_MODEL_TYPE);
+export const AllSupportedEventModelType = Object.values(SUPPORTED_EVENT_MODEL_TYPE);
 export const AllSupportedActionType = Object.values(SUPPORTED_ACTION_TYPE);
 export const AllSupportedActionToNotifiedType = Object.values(SUPPORTED_ACTION_TO_NOTIFIED_TYPE);
 
@@ -85,17 +94,20 @@ export const CommentActions = Object.values({
 
 
 export type SupportedTargetModelType = typeof SUPPORTED_TARGET_MODEL_TYPE[keyof typeof SUPPORTED_TARGET_MODEL_TYPE];
+export type SupportedEventModelType = typeof SUPPORTED_EVENT_MODEL_TYPE[keyof typeof SUPPORTED_EVENT_MODEL_TYPE];
 export type SupportedActionType = typeof SUPPORTED_ACTION_TYPE[keyof typeof SUPPORTED_ACTION_TYPE];
 
 
 export type ISnapshot = Partial<Pick<IUser, 'username'>>
 
 export type IActivity = {
-  user?: IUser
+  user?: string
   ip?: string
   endpoint?: string
   targetModel?: SupportedTargetModelType
   target?: string
+  eventModel?: SupportedEventModelType
+  event?: string
   action: SupportedActionType
   createdAt: Date
   snapshot?: ISnapshot

packages/app/src/server/models/activity.ts

@@ -5,7 +5,9 @@ import {
 import mongoosePaginate from 'mongoose-paginate-v2';
 
 import {
-  AllSupportedTargetModelType, AllSupportedActionType, SupportedActionType, ISnapshot,
+  ISnapshot, AllSupportedActionType, SupportedActionType,
+  AllSupportedTargetModelType, SupportedTargetModelType,
+  AllSupportedEventModelType, SupportedEventModelType,
 } from '~/interfaces/activity';
 
 import loggerFactory from '../../utils/logger';
@@ -16,11 +18,13 @@ const logger = loggerFactory('growi:models:activity');
 
 export interface ActivityDocument extends Document {
   _id: Types.ObjectId
-  user: Types.ObjectId | any
+  user: Types.ObjectId
   ip: string
   endpoint: string
-  targetModel: string
+  targetModel: SupportedTargetModelType
   target: Types.ObjectId
+  eventModel: SupportedEventModelType
+  event: Types.ObjectId
   action: SupportedActionType
   snapshot: ISnapshot
 
@@ -57,6 +61,13 @@ const activitySchema = new Schema<ActivityDocument, ActivityModel>({
     type: Schema.Types.ObjectId,
     refPath: 'targetModel',
   },
+  eventModel: {
+    type: String,
+    enum: AllSupportedEventModelType,
+  },
+  event: {
+    type: Schema.Types.ObjectId,
+  },
   action: {
     type: String,
     enum: AllSupportedActionType,

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

@@ -1,11 +1,15 @@
+import { SUPPORTED_ACTION_TYPE, SUPPORTED_TARGET_MODEL_TYPE } from '~/interfaces/activity';
+import { generateAddActivityMiddleware } from '~/server/middlewares/add-activity';
 import loggerFactory from '~/utils/logger';
 
 import { apiV3FormValidator } from '../../middlewares/apiv3-form-validator';
 
+
 const logger = loggerFactory('growi:routes:apiv3:bookmarks'); // eslint-disable-line no-unused-vars
 
 const express = require('express');
 const { body, query, param } = require('express-validator');
+
 const { serializeUserSecurely } = require('../../models/serializers/user-serializer');
 
 const router = express.Router();
@@ -71,6 +75,9 @@ module.exports = (crowi) => {
   const loginRequiredStrictly = require('../../middlewares/login-required')(crowi);
   const loginRequired = require('../../middlewares/login-required')(crowi, true);
   const csrf = require('../../middlewares/csrf')(crowi);
+  const addActivity = generateAddActivityMiddleware(crowi);
+
+  const activityEvent = crowi.event('activity');
 
   const { Page, Bookmark, User } = crowi.models;
 
@@ -257,7 +264,7 @@ module.exports = (crowi) => {
    *                schema:
    *                  $ref: '#/components/schemas/Bookmark'
    */
-  router.put('/', accessTokenParser, loginRequiredStrictly, csrf, validator.bookmarks, apiV3FormValidator, async(req, res) => {
+  router.put('/', accessTokenParser, loginRequiredStrictly, csrf, addActivity, validator.bookmarks, apiV3FormValidator, async(req, res) => {
     const { pageId, bool } = req.body;
     const userId = req.user?._id;
 
@@ -265,9 +272,10 @@ module.exports = (crowi) => {
       return res.apiv3Err('A logged in user is required.');
     }
 
+    let page;
     let bookmark;
     try {
-      const page = await Page.findByIdAndViewer(pageId, req.user);
+      page = await Page.findByIdAndViewer(pageId, req.user);
       if (page == null) {
         return res.apiv3Err(`Page '${pageId}' is not found or forbidden`);
       }
@@ -277,10 +285,6 @@ module.exports = (crowi) => {
       if (bookmark == null) {
         if (bool) {
           bookmark = await Bookmark.add(page, req.user);
-
-          const pageEvent = crowi.event('page');
-          // in-app notification
-          pageEvent.emit('bookmark', page, req.user);
         }
         else {
           logger.warn(`Removing the bookmark for ${page._id} by ${req.user._id} failed because the bookmark does not exist.`);
@@ -306,6 +310,13 @@ module.exports = (crowi) => {
       bookmark.depopulate('user');
     }
 
+    const parameters = {
+      targetModel: SUPPORTED_TARGET_MODEL_TYPE.MODEL_PAGE,
+      target: page,
+      action: bool ? SUPPORTED_ACTION_TYPE.ACTION_PAGE_BOOKMARK : SUPPORTED_ACTION_TYPE.ACTION_PAGE_UNBOOKMARK,
+    };
+    activityEvent.emit('update', res.locals.activity._id, parameters, page);
+
     return res.apiv3({ bookmark });
   });
 

packages/app/src/server/routes/apiv3/page.js

@@ -1,6 +1,8 @@
 import { pagePathUtils } from '@growi/core';
 
+import { SUPPORTED_ACTION_TYPE, SUPPORTED_TARGET_MODEL_TYPE } from '~/interfaces/activity';
 import { AllSubscriptionStatusType } from '~/interfaces/subscription';
+import { generateAddActivityMiddleware } from '~/server/middlewares/add-activity';
 import Subscription from '~/server/models/subscription';
 import UserGroup from '~/server/models/user-group';
 import loggerFactory from '~/utils/logger';
@@ -162,12 +164,15 @@ module.exports = (crowi) => {
   const loginRequiredStrictly = require('../../middlewares/login-required')(crowi);
   const csrf = require('../../middlewares/csrf')(crowi);
   const certifySharedPage = require('../../middlewares/certify-shared-page')(crowi);
+  const addActivity = generateAddActivityMiddleware(crowi);
 
   const globalNotificationService = crowi.getGlobalNotificationService();
   const socketIoService = crowi.socketIoService;
   const { Page, GlobalNotificationSetting, Bookmark } = crowi.models;
   const { pageService, exportService } = crowi;
 
+  const activityEvent = crowi.event('activity');
+
   const validator = {
     getPage: [
       query('id').if(value => value != null).isMongoId(),
@@ -306,7 +311,7 @@ module.exports = (crowi) => {
    *                schema:
    *                  $ref: '#/components/schemas/Page'
    */
-  router.put('/likes', accessTokenParser, loginRequiredStrictly, csrf, validator.likes, apiV3FormValidator, async(req, res) => {
+  router.put('/likes', accessTokenParser, loginRequiredStrictly, csrf, addActivity, validator.likes, apiV3FormValidator, async(req, res) => {
     const { pageId, bool: isLiked } = req.body;
 
     let page;
@@ -330,13 +335,17 @@ module.exports = (crowi) => {
 
     const result = { page };
     result.seenUser = page.seenUsers;
+
+    const parameters = {
+      targetModel: SUPPORTED_TARGET_MODEL_TYPE.MODEL_PAGE,
+      target: page,
+      action: isLiked ? SUPPORTED_ACTION_TYPE.ACTION_PAGE_LIKE : SUPPORTED_ACTION_TYPE.ACTION_PAGE_UNLIKE,
+    };
+    activityEvent.emit('update', res.locals.activity._id, parameters, page);
+
     res.apiv3({ result });
 
     if (isLiked) {
-      const pageEvent = crowi.event('page');
-      // in-app notification
-      pageEvent.emit('like', page, req.user);
-
       try {
         // global notification
         await globalNotificationService.fire(GlobalNotificationSetting.EVENT.PAGE_LIKE, page, req.user);

packages/app/src/server/routes/apiv3/pages.js

@@ -286,7 +286,7 @@ module.exports = (crowi) => {
    *          409:
    *            description: page path is already existed
    */
-  router.post('/', accessTokenParser, loginRequiredStrictly, csrf, validator.createPage, apiV3FormValidator, async(req, res) => {
+  router.post('/', accessTokenParser, loginRequiredStrictly, csrf, addActivity, validator.createPage, apiV3FormValidator, async(req, res) => {
     const {
       body, grant, grantUserGroupId, overwriteScopesOfDescendants, isSlackEnabled, slackChannels, pageTags,
     } = req.body;
@@ -326,6 +326,13 @@ module.exports = (crowi) => {
       Page.applyScopesToDescendantsAsyncronously(createdPage, req.user);
     }
 
+    const parameters = {
+      targetModel: SUPPORTED_TARGET_MODEL_TYPE.MODEL_PAGE,
+      target: createdPage,
+      action: SUPPORTED_ACTION_TYPE.ACTION_PAGE_CREATE,
+    };
+    activityEvent.emit('update', res.locals.activity._id, parameters);
+
     res.apiv3(result, 201);
 
     try {
@@ -351,21 +358,6 @@ module.exports = (crowi) => {
       }
     }
 
-    // create activity
-    try {
-      const parameters = {
-        user: req.user._id,
-        targetModel: SUPPORTED_TARGET_MODEL_TYPE.MODEL_PAGE,
-        target: createdPage,
-        action: SUPPORTED_ACTION_TYPE.ACTION_PAGE_CREATE,
-        snapshot: { username: req.user.username },
-      };
-      await crowi.activityService.createByParameters(parameters);
-    }
-    catch (err) {
-      logger.error('Failed to create activity', err);
-    }
-
     // create subscription
     try {
       await crowi.inAppNotificationService.createSubscription(req.user.id, createdPage._id, subscribeRuleNames.PAGE_CREATE);
@@ -694,7 +686,7 @@ module.exports = (crowi) => {
    *          500:
    *            description: Internal server error.
    */
-  router.post('/duplicate', accessTokenParser, loginRequiredStrictly, csrf, validator.duplicatePage, apiV3FormValidator, async(req, res) => {
+  router.post('/duplicate', accessTokenParser, loginRequiredStrictly, csrf, addActivity, validator.duplicatePage, apiV3FormValidator, async(req, res) => {
     const { pageId, isRecursively } = req.body;
 
     const newPagePath = pathUtils.normalizePath(req.body.pageNameInput);
@@ -740,6 +732,13 @@ module.exports = (crowi) => {
       logger.error('Failed to create subscription document', err);
     }
 
+    const parameters = {
+      targetModel: SUPPORTED_TARGET_MODEL_TYPE.MODEL_PAGE,
+      target: page,
+      action: SUPPORTED_ACTION_TYPE.ACTION_PAGE_DUPLICATE,
+    };
+    activityEvent.emit('update', res.locals.activity._id, parameters, page);
+
     return res.apiv3(result);
   });
 

packages/app/src/server/routes/comment.js

@@ -1,3 +1,4 @@
+import { SUPPORTED_ACTION_TYPE, SUPPORTED_TARGET_MODEL_TYPE, SUPPORTED_EVENT_MODEL_TYPE } from '~/interfaces/activity';
 import loggerFactory from '~/utils/logger';
 
 /**
@@ -60,6 +61,8 @@ module.exports = function(crowi, app) {
   const mongoose = require('mongoose');
   const ObjectId = mongoose.Types.ObjectId;
 
+  const activityEvent = crowi.event('activity');
+
   const actions = {};
   const api = {};
 
@@ -242,13 +245,12 @@ module.exports = function(crowi, app) {
     let createdComment;
     try {
       createdComment = await Comment.create(pageId, req.user._id, revisionId, comment, position, isMarkdown, replyTo);
-      commentEvent.emit('create', req.user, createdComment);
+      commentEvent.emit('create', createdComment);
     }
     catch (err) {
       logger.error(err);
       return res.json(ApiResponse.error(err));
     }
-
     // update page
     const page = await Page.findOneAndUpdate(
       { _id: pageId },
@@ -258,6 +260,15 @@ module.exports = function(crowi, app) {
       },
     );
 
+    const parameters = {
+      targetModel: SUPPORTED_TARGET_MODEL_TYPE.MODEL_PAGE,
+      target: page,
+      eventModel: SUPPORTED_EVENT_MODEL_TYPE.MODEL_COMMENT,
+      event: createdComment,
+      action: SUPPORTED_ACTION_TYPE.ACTION_COMMENT_CREATE,
+    };
+    activityEvent.emit('update', res.locals.activity._id, parameters, page);
+
     res.json(ApiResponse.success({ comment: createdComment }));
 
     // global notification
@@ -379,13 +390,16 @@ module.exports = function(crowi, app) {
         { _id: commentId },
         { $set: { comment: commentStr, isMarkdown, revision } },
       );
-      commentEvent.emit('update', req.user, updatedComment);
+      commentEvent.emit('update', updatedComment);
     }
     catch (err) {
       logger.error(err);
       return res.json(ApiResponse.error(err));
     }
 
+    const parameters = { action: SUPPORTED_ACTION_TYPE.ACTION_COMMENT_UPDATE };
+    activityEvent.emit('update', res.locals.activity._id, parameters);
+
     res.json(ApiResponse.success({ comment: updatedComment }));
 
     // process notification if needed

packages/app/src/server/routes/index.js

@@ -173,21 +173,21 @@ module.exports = function(crowi, app) {
 
   // HTTP RPC Styled API (に徐々に移行していいこうと思う)
   apiV1Router.get('/pages.list'          , accessTokenParser , loginRequired , page.api.list);
-  apiV1Router.post('/pages.update'       , accessTokenParser , loginRequiredStrictly , csrf, page.api.update);
+  apiV1Router.post('/pages.update'       , accessTokenParser , loginRequiredStrictly , csrf, addActivity, page.api.update);
   apiV1Router.get('/pages.exist'         , accessTokenParser , loginRequired , page.api.exist);
   apiV1Router.get('/pages.updatePost'    , accessTokenParser, loginRequired, page.api.getUpdatePost);
   apiV1Router.get('/pages.getPageTag'    , accessTokenParser , loginRequired , page.api.getPageTag);
   // allow posting to guests because the client doesn't know whether the user logged in
-  apiV1Router.post('/pages.remove'       , loginRequiredStrictly , csrf, page.validator.remove, apiV1FormValidator, page.api.remove); // (Avoid from API Token)
-  apiV1Router.post('/pages.revertRemove' , loginRequiredStrictly , csrf, page.validator.revertRemove, apiV1FormValidator, page.api.revertRemove); // (Avoid from API Token)
+  apiV1Router.post('/pages.remove'       , loginRequiredStrictly , csrf, addActivity, page.validator.remove, apiV1FormValidator, page.api.remove); // (Avoid from API Token)
+  apiV1Router.post('/pages.revertRemove' , loginRequiredStrictly , csrf, addActivity, page.validator.revertRemove, apiV1FormValidator, page.api.revertRemove); // (Avoid from API Token)
   apiV1Router.post('/pages.unlink'       , loginRequiredStrictly , csrf, page.api.unlink); // (Avoid from API Token)
   apiV1Router.post('/pages.duplicate'    , accessTokenParser, loginRequiredStrictly, csrf, page.api.duplicate);
   apiV1Router.get('/tags.list'           , accessTokenParser, loginRequired, tag.api.list);
   apiV1Router.get('/tags.search'         , accessTokenParser, loginRequired, tag.api.search);
   apiV1Router.post('/tags.update'        , accessTokenParser, loginRequiredStrictly, tag.api.update);
   apiV1Router.get('/comments.get'        , accessTokenParser , loginRequired , comment.api.get);
-  apiV1Router.post('/comments.add'       , comment.api.validators.add(), accessTokenParser , loginRequiredStrictly , csrf, comment.api.add);
-  apiV1Router.post('/comments.update'    , comment.api.validators.add(), accessTokenParser , loginRequiredStrictly , csrf, comment.api.update);
+  apiV1Router.post('/comments.add'       , comment.api.validators.add(), accessTokenParser , loginRequiredStrictly , csrf, addActivity, comment.api.add);
+  apiV1Router.post('/comments.update'    , comment.api.validators.add(), accessTokenParser , loginRequiredStrictly , csrf, addActivity, comment.api.update);
   apiV1Router.post('/comments.remove'    , accessTokenParser , loginRequiredStrictly , csrf, comment.api.remove);
   apiV1Router.post('/attachments.add'                  , uploads.single('file'), autoReap, accessTokenParser, loginRequiredStrictly ,csrf, attachment.api.add);
   apiV1Router.post('/attachments.uploadProfileImage'   , uploads.single('file'), autoReap, accessTokenParser, loginRequiredStrictly ,csrf, attachment.api.uploadProfileImage);

packages/app/src/server/routes/page.js

@@ -3,6 +3,7 @@ import { body } from 'express-validator';
 import mongoose from 'mongoose';
 import urljoin from 'url-join';
 
+import { SUPPORTED_TARGET_MODEL_TYPE, SUPPORTED_ACTION_TYPE } from '~/interfaces/activity';
 import loggerFactory from '~/utils/logger';
 
 import { PathAlreadyExistsError } from '../models/errors';
@@ -157,6 +158,8 @@ module.exports = function(crowi, app) {
   const globalNotificationService = crowi.getGlobalNotificationService();
   const userNotificationService = crowi.getUserNotificationService();
 
+  const activityEvent = crowi.event('activity');
+
   const XssOption = require('~/services/xss/xssOption');
   const Xss = require('~/services/xss/index');
   const initializedConfig = {
@@ -987,6 +990,13 @@ module.exports = function(crowi, app) {
         logger.error('Create user notification failed', err);
       }
     }
+
+    const parameters = {
+      targetModel: SUPPORTED_TARGET_MODEL_TYPE.MODEL_PAGE,
+      target: page,
+      action: SUPPORTED_ACTION_TYPE.ACTION_PAGE_UPDATE,
+    };
+    activityEvent.emit('update', res.locals.activity._id, parameters, page);
   };
 
   /**
@@ -1223,6 +1233,13 @@ module.exports = function(crowi, app) {
     result.isRecursively = isRecursively;
     result.isCompletely = isCompletely;
 
+    const parameters = {
+      targetModel: SUPPORTED_TARGET_MODEL_TYPE.MODEL_PAGE,
+      target: page,
+      action: isCompletely ? SUPPORTED_ACTION_TYPE.ACTION_PAGE_DELETE_COMPLETELY : SUPPORTED_ACTION_TYPE.ACTION_PAGE_DELETE,
+    };
+    activityEvent.emit('update', res.locals.activity._id, parameters, page);
+
     res.json(ApiResponse.success(result));
 
     try {
@@ -1274,6 +1291,13 @@ module.exports = function(crowi, app) {
     const result = {};
     result.page = page; // TODO consider to use serializePageSecurely method -- 2018.08.06 Yuki Takei
 
+    const parameters = {
+      targetModel: SUPPORTED_TARGET_MODEL_TYPE.MODEL_PAGE,
+      target: page,
+      action: SUPPORTED_ACTION_TYPE.ACTION_PAGE_REVERT,
+    };
+    activityEvent.emit('update', res.locals.activity._id, parameters, page);
+
     return res.json(ApiResponse.success(result));
   };
 

packages/app/src/server/service/comment.ts

@@ -1,13 +1,6 @@
 import { getModelSafely } from '@growi/core';
 import { Types } from 'mongoose';
 
-import {
-  SUPPORTED_TARGET_MODEL_TYPE, SUPPORTED_ACTION_TYPE, SupportedActionType, ISnapshot,
-} from '~/interfaces/activity';
-import { IPage } from '~/interfaces/page';
-import { IUserHasId } from '~/interfaces/user';
-import { stringifySnapshot } from '~/models/serializers/in-app-notification-snapshot/page';
-
 import loggerFactory from '../../utils/logger';
 import Crowi from '../crowi';
 
@@ -39,20 +32,11 @@ class CommentService {
 
   initCommentEventListeners(): void {
     // create
-    this.commentEvent.on('create', async(user, savedComment) => {
+    this.commentEvent.on('create', async(savedComment) => {
 
       try {
         const Page = getModelSafely('Page') || require('../models/page')(this.crowi);
         await Page.updateCommentCount(savedComment.page);
-
-        const page = await Page.findById(savedComment.page);
-        if (page == null) {
-          logger.error('Page is not found');
-          return;
-        }
-
-        const activity = await this.createActivity(user, savedComment.page, SUPPORTED_ACTION_TYPE.ACTION_COMMENT_CREATE);
-        await this.createAndSendNotifications(activity, page);
       }
       catch (err) {
         logger.error('Error occurred while handling the comment create event:\n', err);
@@ -61,10 +45,9 @@ class CommentService {
     });
 
     // update
-    this.commentEvent.on('update', async(user, updatedComment) => {
+    this.commentEvent.on('update', async() => {
       try {
         this.commentEvent.onUpdate();
-        await this.createActivity(user, updatedComment.page, SUPPORTED_ACTION_TYPE.ACTION_COMMENT_UPDATE);
       }
       catch (err) {
         logger.error('Error occurred while handling the comment update event:\n', err);
@@ -85,35 +68,6 @@ class CommentService {
     });
   }
 
-  private createActivity = async function(user: IUserHasId, target: IPage, action: SupportedActionType) {
-    const snapshot: ISnapshot = { username: user.username };
-    const parameters = {
-      user: user._id,
-      targetModel: SUPPORTED_TARGET_MODEL_TYPE.MODEL_PAGE,
-      target,
-      action,
-      snapshot,
-    };
-    const activity = await this.activityService.createByParameters(parameters);
-    return activity;
-  };
-
-  private createAndSendNotifications = async function(activity, page: IPage) {
-
-    // Get user to be notified
-    let targetUsers: Types.ObjectId[] = [];
-    targetUsers = await activity.getNotificationTargetUsers();
-
-    // Create and send notifications
-    const snapshot = stringifySnapshot(page);
-    // Add mentioned users to targetUsers
-    const mentionedUsers = await this.getMentionedUsers(activity.event);
-    targetUsers = targetUsers.concat(mentionedUsers);
-
-    await this.inAppNotificationService.upsertByActivity(targetUsers, activity, snapshot);
-    await this.inAppNotificationService.emitSocketIo(targetUsers);
-  };
-
   getMentionedUsers = async(commentId: Types.ObjectId): Promise<Types.ObjectId[]> => {
     const Comment = getModelSafely('Comment') || require('../models/comment')(this.crowi);
     const User = getModelSafely('User') || require('../models/user')(this.crowi);

packages/app/src/server/service/in-app-notification.ts

@@ -1,7 +1,7 @@
 import { subDays } from 'date-fns';
 import { Types } from 'mongoose';
 
-import { AllSupportedActionToNotifiedType } from '~/interfaces/activity';
+import { AllSupportedActionToNotifiedType, SUPPORTED_ACTION_TYPE } from '~/interfaces/activity';
 import { HasObjectId } from '~/interfaces/has-object-id';
 import { InAppNotificationStatuses, PaginateResult } from '~/interfaces/in-app-notification';
 import { IPage } from '~/interfaces/page';
@@ -197,9 +197,13 @@ export default class InAppNotificationService {
   createInAppNotification = async function(activity: ActivityDocument, target: IPage): Promise<void> {
     const shouldNotification = activity != null && target != null && (AllSupportedActionToNotifiedType as ReadonlyArray<string>).includes(activity.action);
     if (shouldNotification) {
-      const snapshot = stringifySnapshot(target as IPage);
+      let mentionedUsers: IUser[] = [];
+      if (activity.action === SUPPORTED_ACTION_TYPE.ACTION_COMMENT_CREATE) {
+        mentionedUsers = await this.crowi.commentService.getMentionedUsers(activity.event);
+      }
       const notificationTargetUsers = await activity?.getNotificationTargetUsers();
-      await this.upsertByActivity(notificationTargetUsers, activity, snapshot);
+      const snapshot = stringifySnapshot(target as IPage);
+      await this.upsertByActivity([...notificationTargetUsers, ...mentionedUsers], activity, snapshot);
       await this.emitSocketIo(notificationTargetUsers);
     }
     else {

packages/app/src/server/service/page.ts

@@ -6,9 +6,6 @@ import escapeStringRegexp from 'escape-string-regexp';
 import mongoose, { ObjectId, QueryCursor } from 'mongoose';
 import streamToPromise from 'stream-to-promise';
 
-import {
-  SUPPORTED_TARGET_MODEL_TYPE, SUPPORTED_ACTION_TYPE, SupportedActionType, ISnapshot,
-} from '~/interfaces/activity';
 import { Ref } from '~/interfaces/common';
 import { V5ConversionErrCode } from '~/interfaces/errors/v5-conversion-error';
 import { HasObjectId } from '~/interfaces/has-object-id';
@@ -20,7 +17,6 @@ import {
 } from '~/interfaces/page-delete-config';
 import { IUserHasId } from '~/interfaces/user';
 import { PageMigrationErrorData, SocketEventName, UpdateDescCountRawData } from '~/interfaces/websocket';
-import { stringifySnapshot } from '~/models/serializers/in-app-notification-snapshot/page';
 import {
   CreateMethod, PageCreateOptions, PageModel, PageDocument, pushRevision,
 } from '~/server/models/page';
@@ -154,79 +150,6 @@ class PageService {
     // createMany
     this.pageEvent.on('createMany', this.pageEvent.onCreateMany);
     this.pageEvent.on('addSeenUsers', this.pageEvent.onAddSeenUsers);
-
-    // update
-    this.pageEvent.on('update', async(page, user) => {
-
-      this.pageEvent.onUpdate();
-
-      try {
-        await this.createAndSendNotifications(user, page, SUPPORTED_ACTION_TYPE.ACTION_PAGE_UPDATE);
-      }
-      catch (err) {
-        logger.error(err);
-      }
-    });
-
-    // duplicate
-    this.pageEvent.on('duplicate', async(page, user) => {
-      try {
-        await this.createAndSendNotifications(user, page, SUPPORTED_ACTION_TYPE.ACTION_PAGE_DUPLICATE);
-      }
-      catch (err) {
-        logger.error(err);
-      }
-    });
-
-    // delete
-    this.pageEvent.on('delete', async(page, user) => {
-      try {
-        await this.createAndSendNotifications(user, page, SUPPORTED_ACTION_TYPE.ACTION_PAGE_DELETE);
-      }
-      catch (err) {
-        logger.error(err);
-      }
-    });
-
-    // delete completely
-    this.pageEvent.on('deleteCompletely', async(page, user) => {
-      try {
-        await this.createAndSendNotifications(user, page, SUPPORTED_ACTION_TYPE.ACTION_PAGE_DELETE_COMPLETELY);
-      }
-      catch (err) {
-        logger.error(err);
-      }
-    });
-
-    // revert
-    this.pageEvent.on('revert', async(page, user) => {
-      try {
-        await this.createAndSendNotifications(user, page, SUPPORTED_ACTION_TYPE.ACTION_PAGE_REVERT);
-      }
-      catch (err) {
-        logger.error(err);
-      }
-    });
-
-    // likes
-    this.pageEvent.on('like', async(page, user) => {
-      try {
-        await this.createAndSendNotifications(user, page, SUPPORTED_ACTION_TYPE.ACTION_PAGE_LIKE);
-      }
-      catch (err) {
-        logger.error(err);
-      }
-    });
-
-    // bookmark
-    this.pageEvent.on('bookmark', async(page, user) => {
-      try {
-        await this.createAndSendNotifications(user, page, SUPPORTED_ACTION_TYPE.ACTION_PAGE_BOOKMARK);
-      }
-      catch (err) {
-        logger.error(err);
-      }
-    });
   }
 
   canDeleteCompletely(creatorId: ObjectIdLike, operator, isRecursively: boolean): boolean {
@@ -2222,29 +2145,6 @@ class PageService {
     return shortBodiesMap;
   }
 
-  private async createAndSendNotifications(user: IUserHasId, target: IPage, action: SupportedActionType) {
-    const { activityService, inAppNotificationService } = this.crowi;
-
-    // Create activity
-    const snapshotForActivity: ISnapshot = { username: user.username };
-    const parameters = {
-      user: user._id,
-      targetModel: SUPPORTED_TARGET_MODEL_TYPE.MODEL_PAGE,
-      target,
-      action,
-      snapshot: snapshotForActivity,
-    };
-    const activity = await activityService.createByParameters(parameters);
-
-    // Get user to be notified
-    const targetUsers = await activity.getNotificationTargetUsers();
-
-    // Create and send notifications
-    const snapshotForInAppNotification = stringifySnapshot(target);
-    await inAppNotificationService.upsertByActivity(targetUsers, activity, snapshotForInAppNotification);
-    await inAppNotificationService.emitSocketIo(targetUsers);
-  }
-
   async normalizeParentByPath(path: string, user): Promise<void> {
     const Page = mongoose.model('Page') as unknown as PageModel;
     const { PageQueryBuilder } = Page;

packages/app/test/integration/service/v5.non-public-page.test.ts

@@ -620,7 +620,6 @@ describe('PageService page operations with non-public pages', () => {
     const renamePage = async(page, newPagePath, user, options) => {
       // mock return value
       const mockedRenameSubOperation = jest.spyOn(crowi.pageService, 'renameSubOperation').mockReturnValue(null);
-      const mockedCreateAndSendNotifications = jest.spyOn(crowi.pageService, 'createAndSendNotifications').mockReturnValue(null);
       const renamedPage = await crowi.pageService.renamePage(page, newPagePath, user, options);
 
       // retrieve the arguments passed when calling method renameSubOperation inside renamePage method
@@ -628,7 +627,6 @@ describe('PageService page operations with non-public pages', () => {
 
       // restores the original implementation
       mockedRenameSubOperation.mockRestore();
-      mockedCreateAndSendNotifications.mockRestore();
 
       // rename descendants
       if (page.grant !== Page.GRANT_RESTRICTED) {
@@ -737,7 +735,6 @@ describe('PageService page operations with non-public pages', () => {
     const duplicate = async(page, newPagePath, user, isRecursively) => {
       // mock return value
       const mockedDuplicateRecursivelyMainOperation = jest.spyOn(crowi.pageService, 'duplicateRecursivelyMainOperation').mockReturnValue(null);
-      const mockedCreateAndSendNotifications = jest.spyOn(crowi.pageService, 'createAndSendNotifications').mockReturnValue(null);
       const duplicatedPage = await crowi.pageService.duplicate(page, newPagePath, user, isRecursively);
 
       // retrieve the arguments passed when calling method duplicateRecursivelyMainOperation inside duplicate method
@@ -745,7 +742,6 @@ describe('PageService page operations with non-public pages', () => {
 
       // restores the original implementation
       mockedDuplicateRecursivelyMainOperation.mockRestore();
-      mockedCreateAndSendNotifications.mockRestore();
 
       // duplicate descendants
       if (page.grant !== Page.GRANT_RESTRICTED && isRecursively) {
@@ -856,14 +852,12 @@ describe('PageService page operations with non-public pages', () => {
 
     const deletePage = async(page, user, options, isRecursively) => {
       const mockedDeleteRecursivelyMainOperation = jest.spyOn(crowi.pageService, 'deleteRecursivelyMainOperation').mockReturnValue(null);
-      const mockedCreateAndSendNotifications = jest.spyOn(crowi.pageService, 'createAndSendNotifications').mockReturnValue(null);
 
       const deletedPage = await crowi.pageService.deletePage(page, user, options, isRecursively);
 
       const argsForDeleteRecursivelyMainOperation = mockedDeleteRecursivelyMainOperation.mock.calls[0];
 
       mockedDeleteRecursivelyMainOperation.mockRestore();
-      mockedCreateAndSendNotifications.mockRestore();
 
       if (isRecursively) {
         await crowi.pageService.deleteRecursivelyMainOperation(...argsForDeleteRecursivelyMainOperation);
@@ -953,14 +947,12 @@ describe('PageService page operations with non-public pages', () => {
   describe('Delete completely', () => {
     const deleteCompletely = async(page, user, options = {}, isRecursively = false, preventEmitting = false) => {
       const mockedDeleteCompletelyRecursivelyMainOperation = jest.spyOn(crowi.pageService, 'deleteCompletelyRecursivelyMainOperation').mockReturnValue(null);
-      const mockedCreateAndSendNotifications = jest.spyOn(crowi.pageService, 'createAndSendNotifications').mockReturnValue(null);
 
       await crowi.pageService.deleteCompletely(page, user, options, isRecursively, preventEmitting);
 
       const argsForDeleteCompletelyRecursivelyMainOperation = mockedDeleteCompletelyRecursivelyMainOperation.mock.calls[0];
 
       mockedDeleteCompletelyRecursivelyMainOperation.mockRestore();
-      mockedCreateAndSendNotifications.mockRestore();
 
       if (isRecursively) {
         await crowi.pageService.deleteCompletelyRecursivelyMainOperation(...argsForDeleteCompletelyRecursivelyMainOperation);

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

@@ -890,7 +890,6 @@ describe('PageService page operations with only public pages', () => {
     const renamePage = async(page, newPagePath, user, options) => {
       // mock return value
       const mockedRenameSubOperation = jest.spyOn(crowi.pageService, 'renameSubOperation').mockReturnValue(null);
-      const mockedCreateAndSendNotifications = jest.spyOn(crowi.pageService, 'createAndSendNotifications').mockReturnValue(null);
       const renamedPage = await crowi.pageService.renamePage(page, newPagePath, user, options);
 
       // retrieve the arguments passed when calling method renameSubOperation inside renamePage method
@@ -898,7 +897,6 @@ describe('PageService page operations with only public pages', () => {
 
       // restores the original implementation
       mockedRenameSubOperation.mockRestore();
-      mockedCreateAndSendNotifications.mockRestore();
 
       // rename descendants
       await crowi.pageService.renameSubOperation(...argsForRenameSubOperation);
@@ -1181,7 +1179,6 @@ describe('PageService page operations with only public pages', () => {
     const duplicate = async(page, newPagePath, user, isRecursively) => {
       // mock return value
       const mockedDuplicateRecursivelyMainOperation = jest.spyOn(crowi.pageService, 'duplicateRecursivelyMainOperation').mockReturnValue(null);
-      const mockedCreateAndSendNotifications = jest.spyOn(crowi.pageService, 'createAndSendNotifications').mockReturnValue(null);
       const duplicatedPage = await crowi.pageService.duplicate(page, newPagePath, user, isRecursively);
 
       // retrieve the arguments passed when calling method duplicateRecursivelyMainOperation inside duplicate method
@@ -1189,7 +1186,6 @@ describe('PageService page operations with only public pages', () => {
 
       // restores the original implementation
       mockedDuplicateRecursivelyMainOperation.mockRestore();
-      mockedCreateAndSendNotifications.mockRestore();
 
       // duplicate descendants
       if (isRecursively) {
@@ -1367,14 +1363,12 @@ describe('PageService page operations with only public pages', () => {
   describe('Delete', () => {
     const deletePage = async(page, user, options, isRecursively) => {
       const mockedDeleteRecursivelyMainOperation = jest.spyOn(crowi.pageService, 'deleteRecursivelyMainOperation').mockReturnValue(null);
-      const mockedCreateAndSendNotifications = jest.spyOn(crowi.pageService, 'createAndSendNotifications').mockReturnValue(null);
 
       const deletedPage = await crowi.pageService.deletePage(page, user, options, isRecursively);
 
       const argsForDeleteRecursivelyMainOperation = mockedDeleteRecursivelyMainOperation.mock.calls[0];
 
       mockedDeleteRecursivelyMainOperation.mockRestore();
-      mockedCreateAndSendNotifications.mockRestore();
 
       if (isRecursively) {
         await crowi.pageService.deleteRecursivelyMainOperation(...argsForDeleteRecursivelyMainOperation);
@@ -1482,14 +1476,12 @@ describe('PageService page operations with only public pages', () => {
   describe('Delete completely', () => {
     const deleteCompletely = async(page, user, options = {}, isRecursively = false, preventEmitting = false) => {
       const mockedDeleteCompletelyRecursivelyMainOperation = jest.spyOn(crowi.pageService, 'deleteCompletelyRecursivelyMainOperation').mockReturnValue(null);
-      const mockedCreateAndSendNotifications = jest.spyOn(crowi.pageService, 'createAndSendNotifications').mockReturnValue(null);
 
       await crowi.pageService.deleteCompletely(page, user, options, isRecursively, preventEmitting);
 
       const argsForDeleteCompletelyRecursivelyMainOperation = mockedDeleteCompletelyRecursivelyMainOperation.mock.calls[0];
 
       mockedDeleteCompletelyRecursivelyMainOperation.mockRestore();
-      mockedCreateAndSendNotifications.mockRestore();
 
       if (isRecursively) {
         await crowi.pageService.deleteCompletelyRecursivelyMainOperation(...argsForDeleteCompletelyRecursivelyMainOperation);