Merge pull request #8841 from weseek/fix/146980-slack-notification-not-sent-on-page-update

fix: Slack notification not sent on page update

apps/app/src/components/PageEditor/PageEditor.tsx

@@ -68,6 +68,7 @@ declare global {
 export type SaveOptions = {
   wip: boolean,
   slackChannels: string,
+  isSlackEnabled: boolean,
   overwriteScopesOfDescendants?: boolean
 }
 export type Save = (

apps/app/src/components/SavePageControls.tsx

@@ -35,30 +35,32 @@ declare global {
 const logger = loggerFactory('growi:SavePageControls');
 
 
-const SavePageButton = (props: {slackChannels: string, isDeviceLargerThanMd?: boolean}) => {
+const SavePageButton = (props: {slackChannels: string, isSlackEnabled?: boolean, isDeviceLargerThanMd?: boolean}) => {
 
   const { t } = useTranslation();
   const { data: _isWaitingSaveProcessing } = useWaitingSaveProcessing();
   const [isSavePageModalShown, setIsSavePageModalShown] = useState<boolean>(false);
 
-  const { slackChannels, isDeviceLargerThanMd } = props;
+  const { slackChannels, isSlackEnabled, isDeviceLargerThanMd } = props;
 
   const isWaitingSaveProcessing = _isWaitingSaveProcessing === true; // ignore undefined
 
   const save = useCallback(async(): Promise<void> => {
     // save
-    globalEmitter.emit('saveAndReturnToView', { wip: false, slackChannels });
-  }, [slackChannels]);
+    globalEmitter.emit('saveAndReturnToView', { wip: false, slackChannels, isSlackEnabled });
+  }, [isSlackEnabled, slackChannels]);
 
   const saveAndOverwriteScopesOfDescendants = useCallback(() => {
     // save
-    globalEmitter.emit('saveAndReturnToView', { wip: false, overwriteScopesOfDescendants: true, slackChannels });
-  }, [slackChannels]);
+    globalEmitter.emit('saveAndReturnToView', {
+      wip: false, overwriteScopesOfDescendants: true, slackChannels, isSlackEnabled,
+    });
+  }, [isSlackEnabled, slackChannels]);
 
   const saveAndMakeWip = useCallback(() => {
     // save
-    globalEmitter.emit('saveAndReturnToView', { wip: true, slackChannels });
-  }, [slackChannels]);
+    globalEmitter.emit('saveAndReturnToView', { wip: true, slackChannels, isSlackEnabled });
+  }, [isSlackEnabled, slackChannels]);
 
   const labelSubmitButton = t('Update');
   const labelOverwriteScopes = t('page_edit.overwrite_scopes', { operation: labelSubmitButton });
@@ -197,11 +199,11 @@ export const SavePageControls = (): JSX.Element | null => {
               )
             }
 
-            <SavePageButton slackChannels={slackChannels} isDeviceLargerThanMd />
+            <SavePageButton isSlackEnabled={isSlackEnabled} slackChannels={slackChannels} isDeviceLargerThanMd />
           </>
         ) : (
           <>
-            <SavePageButton slackChannels={slackChannels} />
+            <SavePageButton isSlackEnabled={isSlackEnabled} slackChannels={slackChannels} />
             <button
               type="button"
               className="btn btn-outline-neutral-secondary border-0 fs-5 p-0 ms-1 text-muted"

apps/app/src/server/routes/apiv3/page/update-page.ts

@@ -80,7 +80,7 @@ export const updatePageHandlersFactory: UpdatePageHandlersFactory = (crowi) => {
   ];
 
 
-  async function postAction(req: UpdatePageRequest, res: ApiV3Response, updatedPage: PageDocument) {
+  async function postAction(req: UpdatePageRequest, res: ApiV3Response, updatedPage: PageDocument, previousRevision: IRevisionHasId | null) {
     // Reflect the updates in ydoc
     const origin = req.body.origin;
     if (origin === Origin.View || origin === undefined) {
@@ -111,10 +111,10 @@ export const updatePageHandlersFactory: UpdatePageHandlersFactory = (crowi) => {
     }
 
     // user notification
-    const { revisionId, isSlackEnabled, slackChannels } = req.body;
+    const { isSlackEnabled, slackChannels } = req.body;
     if (isSlackEnabled) {
       try {
-        const option = revisionId != null ? { previousRevision: revisionId } : undefined;
+        const option = previousRevision != null ? { previousRevision } : undefined;
         const results = await crowi.userNotificationService.fire(updatedPage, req.user, slackChannels, 'update', option);
         results.forEach((result) => {
           if (result.status === 'rejected') {
@@ -138,6 +138,8 @@ export const updatePageHandlersFactory: UpdatePageHandlersFactory = (crowi) => {
         pageId, revisionId, body, origin,
       } = req.body;
 
+      const sanitizeRevisionId = revisionId == null ? undefined : xss.process(revisionId);
+
       // check page existence
       const isExist = await Page.count({ _id: pageId }) > 0;
       if (!isExist) {
@@ -147,7 +149,7 @@ export const updatePageHandlersFactory: UpdatePageHandlersFactory = (crowi) => {
       // check revision
       const currentPage = await Page.findByIdAndViewer(pageId, req.user);
 
-      if (currentPage != null && !await currentPage.isUpdatable(revisionId, origin)) {
+      if (currentPage != null && !await currentPage.isUpdatable(sanitizeRevisionId, origin)) {
         const latestRevision = await Revision.findById(currentPage.revision).populate('author');
         const returnLatestRevision = {
           revisionId: latestRevision?._id.toString(),
@@ -159,6 +161,7 @@ export const updatePageHandlersFactory: UpdatePageHandlersFactory = (crowi) => {
       }
 
       let updatedPage: PageDocument;
+      let previousRevision: IRevisionHasId | null;
       try {
         const {
           grant, userRelatedGrantUserGroupIds, overwriteScopesOfDescendants, wip,
@@ -168,7 +171,7 @@ export const updatePageHandlersFactory: UpdatePageHandlersFactory = (crowi) => {
           options.grant = grant;
           options.userRelatedGrantUserGroupIds = userRelatedGrantUserGroupIds;
         }
-        const previousRevision = await Revision.findById(revisionId);
+        previousRevision = await Revision.findById(sanitizeRevisionId);
         updatedPage = await crowi.pageService.updatePage(currentPage, body, previousRevision?.body ?? null, req.user, options);
       }
       catch (err) {
@@ -183,7 +186,7 @@ export const updatePageHandlersFactory: UpdatePageHandlersFactory = (crowi) => {
 
       res.apiv3(result, 201);
 
-      postAction(req, res, updatedPage);
+      postAction(req, res, updatedPage, previousRevision);
     },
   ];
 };

apps/app/src/server/service/user-notification/index.ts

@@ -1,3 +1,5 @@
+import type { IRevisionHasId } from '@growi/core';
+
 import { toArrayFromCsv } from '~/utils/to-array-from-csv';
 
 
@@ -27,11 +29,11 @@ export class UserNotificationService {
    * @param {User} user
    * @param {string} slackChannelsStr comma separated string. e.g. 'general,channel1,channel2'
    * @param {string} mode 'create' or 'update' or 'comment'
-   * @param {string} previousRevision
+   * @param {IRevisionHasId} previousRevision
    * @param {Comment} comment
    */
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  async fire(page, user, slackChannelsStr, mode, option?: { previousRevision: string }, comment = {}): Promise<PromiseSettledResult<any>[]> {
+  async fire(page, user, slackChannelsStr, mode, option?: { previousRevision: IRevisionHasId }, comment = {}): Promise<PromiseSettledResult<any>[]> {
     const {
       appService, slackIntegrationService,
     } = this.crowi;

apps/app/src/server/util/slack.js

@@ -24,7 +24,17 @@ const prepareAttachmentTextForCreate = function(page, siteUrl) {
   return convertMarkdownToMarkdown(body, siteUrl);
 };
 
+/**
+ * Return diff with latest revisionBody
+ * @param {IPageHasId} page
+ * @param {string} siteUrl
+ * @param {IRevisionHasId} previousRevision
+ */
 const prepareAttachmentTextForUpdate = function(page, siteUrl, previousRevision) {
+  if (previousRevision == null) {
+    return;
+  }
+
   const diff = require('diff');
   let diffText = '';