Просмотр исходного кода

Added feature on the revert activity.

Shunm634-source 3 лет назад
Родитель
Сommit
850ff19317

+ 6 - 11
packages/app/src/server/routes/page.js

@@ -1360,6 +1360,11 @@ module.exports = function(crowi, app) {
     // get recursively flag
     const isRecursively = req.body.recursively;
 
+    const activityParameters = {
+      ip: req.ip,
+      endpoint: req.originalUrl,
+    };
+
     let page;
     let descendantPages;
     try {
@@ -1367,10 +1372,7 @@ module.exports = function(crowi, app) {
       if (page == null) {
         throw new Error(`Page '${pageId}' is not found or forbidden`, 'notfound_or_forbidden');
       }
-      page = await crowi.pageService.revertDeletedPage(page, req.user, {}, isRecursively);
-      const pages = await Page.findListWithDescendants(page.path, req.user);
-      descendantPages = pages.pages;
-      descendantPages.pop();
+      page = await crowi.pageService.revertDeletedPage(page, req.user, {}, isRecursively, activityParameters);
     }
     catch (err) {
       if (err instanceof PathAlreadyExistsError) {
@@ -1384,13 +1386,6 @@ module.exports = function(crowi, app) {
     const result = {};
     result.page = page; // TODO consider to use serializePageSecurely method -- 2018.08.06 Yuki Takei
 
-    const parameters = {
-      targetModel: SupportedTargetModel.MODEL_PAGE,
-      target: page,
-      action: isRecursively ? SupportedAction.ACTION_PAGE_RECURSIVELY_REVERT : SupportedAction.ACTION_PAGE_REVERT,
-    };
-    activityEvent.emit('update', res.locals.activity._id, parameters, page, descendantPages);
-
     return res.json(ApiResponse.success(result));
   };
 

+ 25 - 5
packages/app/src/server/service/page.ts

@@ -1969,13 +1969,25 @@ class PageService {
     }
   }
 
-  async revertDeletedPage(page, user, options = {}, isRecursively = false) {
+  async revertDeletedPage(page, user, options = {}, isRecursively = false, activityParameters?) {
     /*
      * Common Operation
      */
     const Page = this.crowi.model('Page');
     const PageTagRelation = this.crowi.model('PageTagRelation');
 
+    const parameters = {
+      ip: activityParameters.ip,
+      endpoint: activityParameters.endpoint,
+      action: page.descendantCount > 0 ? SupportedAction.ACTION_PAGE_RECURSIVELY_REVERT : SupportedAction.ACTION_PAGE_REVERT,
+      user,
+      snapshot: {
+        username: user.username,
+      },
+    };
+
+    const activity = await this.crowi.activityService.createActivity(parameters);
+
     // 1. Separate v4 & v5 process
     const shouldUseV4Process = this.shouldUseV4ProcessForRevert(page);
     if (shouldUseV4Process) {
@@ -2010,6 +2022,7 @@ class PageService {
 
     if (!isRecursively) {
       await this.updateDescendantCountOfAncestors(parent._id, 1, true);
+      this.activityEvent.emit('updated', activity, page);
     }
     else {
       let pageOp;
@@ -2033,7 +2046,7 @@ class PageService {
        */
       (async() => {
         try {
-          await this.revertRecursivelyMainOperation(page, user, options, pageOp._id);
+          await this.revertRecursivelyMainOperation(page, user, options, pageOp._id, activity);
         }
         catch (err) {
           logger.error('Error occurred while running revertRecursivelyMainOperation.', err);
@@ -2049,10 +2062,13 @@ class PageService {
     return updatedPage;
   }
 
-  async revertRecursivelyMainOperation(page, user, options, pageOpId: ObjectIdLike): Promise<void> {
+  async revertRecursivelyMainOperation(page, user, options, pageOpId: ObjectIdLike, activity?): Promise<void> {
     const Page = mongoose.model('Page') as unknown as PageModel;
 
-    await this.revertDeletedDescendantsWithStream(page, user, options, false);
+    const descendantsSubscribedSets = new Set();
+    await this.revertDeletedDescendantsWithStream(page, user, options, false, descendantsSubscribedSets);
+    const descendantsSubscribedUsers = Array.from(descendantsSubscribedSets);
+    this.activityEvent.emit('updated', activity, page, descendantsSubscribedUsers);
 
     const newPath = Page.getRevertDeletedPageName(page.path);
     // normalize parent of descendant pages
@@ -2127,7 +2143,7 @@ class PageService {
   /**
    * Create revert stream
    */
-  private async revertDeletedDescendantsWithStream(targetPage, user, options = {}, shouldUseV4Process = true): Promise<number> {
+  private async revertDeletedDescendantsWithStream(targetPage, user, options = {}, shouldUseV4Process = true, descendantsSubscribedSets?): Promise<number> {
     if (shouldUseV4Process) {
       return this.revertDeletedDescendantsWithStreamV4(targetPage, user, options);
     }
@@ -2142,6 +2158,10 @@ class PageService {
         try {
           count += batch.length;
           await revertDeletedDescendants(batch, user);
+          const subscribedUsers = await Subscription.getSubscriptions(batch);
+          subscribedUsers.forEach((eachUser) => {
+            descendantsSubscribedSets.add(eachUser);
+          });
           logger.debug(`Reverting pages progressing: (count=${count})`);
         }
         catch (err) {

+ 10 - 4
packages/app/test/integration/service/page.test.js

@@ -746,7 +746,7 @@ describe('PageService', () => {
     test('delete completely without options', async() => {
       await crowi.pageService.deleteCompletely(parentForDeleteCompletely, testUser2, { }, false, false, {
         ip: '::ffff:127.0.0.1',
-        endpoint: '/_api/v3/pages/delete',
+        endpoint: '/_api/v3/pages/deletecompletely',
       });
 
       expect(deleteCompletelyOperationSpy).toHaveBeenCalled();
@@ -759,7 +759,7 @@ describe('PageService', () => {
     test('delete completely with isRecursively', async() => {
       await crowi.pageService.deleteCompletely(parentForDeleteCompletely, testUser2, { }, true, false, {
         ip: '::ffff:127.0.0.1',
-        endpoint: '/_api/v3/pages/delete',
+        endpoint: '/_api/v3/pages/deletecompletely',
       });
 
       expect(deleteCompletelyOperationSpy).toHaveBeenCalled();
@@ -783,7 +783,10 @@ describe('PageService', () => {
     });
 
     test('revert deleted page when the redirect from page exists', async() => {
-      const resultPage = await crowi.pageService.revertDeletedPage(parentForRevert1, testUser2);
+      const resultPage = await crowi.pageService.revertDeletedPage(parentForRevert1, testUser2, {}, false, {
+        ip: '::ffff:127.0.0.1',
+        endpoint: '/_api/v3/pages/revert',
+      });
 
       expect(getRevertDeletedPageNameSpy).toHaveBeenCalledWith(parentForRevert1.path);
       expect(revertDeletedDescendantsWithStreamSpy).not.toHaveBeenCalled();
@@ -801,7 +804,10 @@ describe('PageService', () => {
         return null;
       });
 
-      const resultPage = await crowi.pageService.revertDeletedPage(parentForRevert2, testUser2, {}, true);
+      const resultPage = await crowi.pageService.revertDeletedPage(parentForRevert2, testUser2, {}, true, {
+        ip: '::ffff:127.0.0.1',
+        endpoint: '/_api/v3/pages/revert',
+      });
 
       expect(getRevertDeletedPageNameSpy).toHaveBeenCalledWith(parentForRevert2.path);
       expect(findByPathSpy).toHaveBeenCalledWith('/parentForRevert2');

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

@@ -1286,10 +1286,10 @@ describe('PageService page operations with non-public pages', () => {
     });
   });
   describe('revert', () => {
-    const revertDeletedPage = async(page, user, options = {}, isRecursively = false) => {
+    const revertDeletedPage = async(page, user, options = {}, isRecursively = false, activityParameters?) => {
       // mock return value
       const mockedRevertRecursivelyMainOperation = jest.spyOn(crowi.pageService, 'revertRecursivelyMainOperation').mockReturnValue(null);
-      const revertedPage = await crowi.pageService.revertDeletedPage(page, user, options, isRecursively);
+      const revertedPage = await crowi.pageService.revertDeletedPage(page, user, options, isRecursively, activityParameters);
 
       const argsForRecursivelyMainOperation = mockedRevertRecursivelyMainOperation.mock.calls[0];
 
@@ -1312,7 +1312,10 @@ describe('PageService page operations with non-public pages', () => {
       expect(tag).toBeTruthy();
       expect(deletedPageTagRelation).toBeTruthy();
 
-      await revertDeletedPage(trashedPage, dummyUser1, {}, false);
+      await revertDeletedPage(trashedPage, dummyUser1, {}, false, {
+        ip: '::ffff:127.0.0.1',
+        endpoint: '/_api/v3/pages/rename',
+      });
 
       const revertedPage = await Page.findOne({ path: '/np_revert1' });
       const deltedPageBeforeRevert = await Page.findOne({ path: '/trash/np_revert1' });
@@ -1339,7 +1342,10 @@ describe('PageService page operations with non-public pages', () => {
       expect(tag).toBeTruthy();
       expect(deletedPageTagRelation).toBeTruthy();
 
-      await revertDeletedPage(trashedPage, user1, {}, false);
+      await revertDeletedPage(trashedPage, user1, {}, false, {
+        ip: '::ffff:127.0.0.1',
+        endpoint: '/_api/v3/pages/revert',
+      });
 
       const revertedPage = await Page.findOne({ path: '/np_revert2' });
       const trashedPageBR = await Page.findOne({ path: beforeRevertPath });
@@ -1367,7 +1373,10 @@ describe('PageService page operations with non-public pages', () => {
       expect(revision1).toBeTruthy();
       expect(revision2).toBeTruthy();
 
-      await revertDeletedPage(trashedPage1, npDummyUser2, {}, true);
+      await revertDeletedPage(trashedPage1, npDummyUser2, {}, true, {
+        ip: '::ffff:127.0.0.1',
+        endpoint: '/_api/v3/pages/revert',
+      });
 
       const revertedPage = await Page.findOne({ path: '/np_revert3' });
       const middlePage = await Page.findOne({ path: '/np_revert3/middle' });
@@ -1406,7 +1415,10 @@ describe('PageService page operations with non-public pages', () => {
       expect(user).toBeTruthy();
       expect(nonExistantPage3).toBeNull();
 
-      await revertDeletedPage(trashedPage1, user, {}, true);
+      await revertDeletedPage(trashedPage1, user, {}, true, {
+        ip: '::ffff:127.0.0.1',
+        endpoint: '/_api/v3/pages/revert',
+      });
       const revertedPage1 = await Page.findOne({ path: '/np_revert5' });
       const newlyCreatedPage = await Page.findOne({ path: '/np_revert5/middle' });
       const revertedPage2 = await Page.findOne({ path: '/np_revert5/middle/np_revert6' });

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

@@ -1834,7 +1834,7 @@ describe('PageService page operations with only public pages', () => {
       try {
         await deletePage(rootPage, dummyUser1, {}, false, {
           ip: '::ffff:127.0.0.1',
-          endpoint: '/_api/v3/pages/rename',
+          endpoint: '/_api/v3/pages/delete',
         });
       }
       catch (err) { isThrown = true }
@@ -1853,7 +1853,7 @@ describe('PageService page operations with only public pages', () => {
       try {
         await deletePage(trashedPage, dummyUser1, {}, false, {
           ip: '::ffff:127.0.0.1',
-          endpoint: '/_api/v3/pages/rename',
+          endpoint: '/_api/v3/pages/delete',
         });
       }
       catch (err) { isThrown = true }
@@ -1871,7 +1871,7 @@ describe('PageService page operations with only public pages', () => {
       try {
         await deletePage(dummyUser1Page, dummyUser1, {}, false, {
           ip: '::ffff:127.0.0.1',
-          endpoint: '/_api/v3/pages/rename',
+          endpoint: '/_api/v3/pages/delete',
         });
       }
       catch (err) { isThrown = true }
@@ -1887,7 +1887,7 @@ describe('PageService page operations with only public pages', () => {
       expect(pageToDelete).toBeTruthy();
       const deletedPage = await deletePage(pageToDelete, dummyUser1, {}, false, {
         ip: '::ffff:127.0.0.1',
-        endpoint: '/_api/v3/pages/rename',
+        endpoint: '/_api/v3/pages/delete',
       });
       const page = await Page.findOne({ path: '/v5_PageForDelete2' });
 
@@ -1906,7 +1906,7 @@ describe('PageService page operations with only public pages', () => {
       expect(grandchildPage).toBeTruthy();
       const deletedParentPage = await deletePage(parentPage, dummyUser1, {}, true, {
         ip: '::ffff:127.0.0.1',
-        endpoint: '/_api/v3/pages/rename',
+        endpoint: '/_api/v3/pages/delete',
       });
       const deletedChildPage = await Page.findOne({ path: '/trash/v5_PageForDelete3/v5_PageForDelete4' });
       const deletedGrandchildPage = await Page.findOne({ path: '/trash/v5_PageForDelete3/v5_PageForDelete4/v5_PageForDelete5' });
@@ -1936,7 +1936,7 @@ describe('PageService page operations with only public pages', () => {
       expect(pageRelation2).toBeTruthy();
       const deletedPage = await deletePage(pageToDelete, dummyUser1, {}, false, {
         ip: '::ffff:127.0.0.1',
-        endpoint: '/_api/v3/pages/rename',
+        endpoint: '/_api/v3/pages/delete',
       });
       const page = await Page.findOne({ path: '/v5_PageForDelete6' });
       const deletedTagRelation1 = await PageTagRelation.findOne({ _id: pageRelation1._id });
@@ -1971,7 +1971,7 @@ describe('PageService page operations with only public pages', () => {
       try {
         await deleteCompletely(rootPage, dummyUser1, {}, false, false, {
           ip: '::ffff:127.0.0.1',
-          endpoint: '/_api/v3/pages/rename',
+          endpoint: '/_api/v3/pages/deletecompletely',
         });
       }
       catch (err) { isThrown = true }
@@ -1985,7 +1985,7 @@ describe('PageService page operations with only public pages', () => {
 
       await deleteCompletely(page, dummyUser1, {}, false, false, {
         ip: '::ffff:127.0.0.1',
-        endpoint: '/_api/v3/pages/rename',
+        endpoint: '/_api/v3/pages/deletecompletely',
       });
       const deletedPage = await Page.findOne({ _id: page._id, path: '/v5_PageForDeleteCompletely1' });
 
@@ -2021,7 +2021,7 @@ describe('PageService page operations with only public pages', () => {
 
       await deleteCompletely(parentPage, dummyUser1, {}, true, false, {
         ip: '::ffff:127.0.0.1',
-        endpoint: '/_api/v3/pages/rename',
+        endpoint: '/_api/v3/pages/deletecompletely',
       });
       const deletedPages = await Page.find({ _id: { $in: [parentPage._id, childPage._id, grandchildPage._id] } });
       const deletedRevisions = await Revision.find({ pageId: { $in: [parentPage._id, grandchildPage._id] } });
@@ -2056,7 +2056,7 @@ describe('PageService page operations with only public pages', () => {
       expect(revision).toBeTruthy();
       await deleteCompletely(page, dummyUser1, {}, false, false, {
         ip: '::ffff:127.0.0.1',
-        endpoint: '/_api/v3/pages/rename',
+        endpoint: '/_api/v3/pages/deletecompletely',
       });
       const deltedPage = await Page.findOne({ _id: page._id });
       const deltedRevision = await Revision.findOne({ _id: revision._id });
@@ -2074,7 +2074,7 @@ describe('PageService page operations with only public pages', () => {
 
       await deleteCompletely(childPage, dummyUser1, {}, false, false, {
         ip: '::ffff:127.0.0.1',
-        endpoint: '/_api/v3/pages/rename',
+        endpoint: '/_api/v3/pages/deletecompletely',
       });
       const parentPageAfterDelete = await Page.findOne({ path: '/v5_PageForDeleteCompletely6' });
       const childPageAfterDelete = await Page.findOne({ path: '/v5_PageForDeleteCompletely6/v5_PageForDeleteCompletely7' });
@@ -2093,10 +2093,10 @@ describe('PageService page operations with only public pages', () => {
     });
   });
   describe('revert', () => {
-    const revertDeletedPage = async(page, user, options = {}, isRecursively = false) => {
+    const revertDeletedPage = async(page, user, options = {}, isRecursively = false, activityParameters?) => {
       // mock return value
       const mockedRevertRecursivelyMainOperation = jest.spyOn(crowi.pageService, 'revertRecursivelyMainOperation').mockReturnValue(null);
-      const revertedPage = await crowi.pageService.revertDeletedPage(page, user, options, isRecursively);
+      const revertedPage = await crowi.pageService.revertDeletedPage(page, user, options, isRecursively, activityParameters);
 
       const argsForRecursivelyMainOperation = mockedRevertRecursivelyMainOperation.mock.calls[0];
 
@@ -2120,7 +2120,10 @@ describe('PageService page operations with only public pages', () => {
       expect(tag).toBeTruthy();
       expect(deletedPageTagRelation).toBeTruthy();
 
-      const revertedPage = await revertDeletedPage(deletedPage, dummyUser1, {}, false);
+      const revertedPage = await revertDeletedPage(deletedPage, dummyUser1, {}, false, {
+        ip: '::ffff:127.0.0.1',
+        endpoint: '/_api/v3/pages/revert',
+      });
       const pageTagRelation = await PageTagRelation.findOne({ relatedPage: deletedPage._id, relatedTag: tag?._id });
 
       expect(revertedPage.parent).toStrictEqual(rootPage._id);
@@ -2140,7 +2143,10 @@ describe('PageService page operations with only public pages', () => {
       expect(revision1).toBeTruthy();
       expect(revision2).toBeTruthy();
 
-      const revertedPage1 = await revertDeletedPage(deletedPage1, dummyUser1, {}, true);
+      const revertedPage1 = await revertDeletedPage(deletedPage1, dummyUser1, {}, true, {
+        ip: '::ffff:127.0.0.1',
+        endpoint: '/_api/v3/pages/revert',
+      });
       const revertedPage2 = await Page.findOne({ _id: deletedPage2._id });
       const newlyCreatedPage = await Page.findOne({ path: '/v5_revert2/v5_revert3' });