Refactored deleteMultiple & normalizeParentMultiple

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

@@ -765,7 +765,8 @@ module.exports = (crowi) => {
     }
 
     // run delete
-    crowi.pageService.deleteMultiplePages(pagesCanBeDeleted, req.user, isCompletely, isRecursively);
+    const options = { isCompletely, isRecursively };
+    crowi.pageService.deleteMultiplePages(pagesCanBeDeleted, req.user, options);
 
     return res.apiv3({ paths: pagesCanBeDeleted.map(p => p.path), isRecursively, isCompletely });
   });
@@ -795,17 +796,11 @@ module.exports = (crowi) => {
       return res.apiv3Err(new ErrorV3(`The maximum number of pages you can select is ${LIMIT_FOR_MULTIPLE_PAGE_OP}.`, 'exceeded_maximum_number'), 400);
     }
 
-    if (isRecursively) {
-      // this method innerly uses socket to send message
-      crowi.pageService.normalizeParentRecursivelyByPageIds(pageIds, req.user);
+    try {
+      await crowi.pageService.normalizeParentByPageIds(pageIds, req.user, isRecursively);
     }
-    else {
-      try {
-        await crowi.pageService.normalizeParentByPageIds(pageIds, req.user);
-      }
-      catch (err) {
-        return res.apiv3Err(new ErrorV3(`Failed to migrate pages: ${err.message}`), 500);
-      }
+    catch (err) {
+      return res.apiv3Err(new ErrorV3(`Failed to migrate pages: ${err.message}`), 500);
     }
 
     return res.apiv3({});

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

@@ -352,24 +352,17 @@ class PageGrantService {
    * @param pageIds pageIds to be tested
    * @returns a tuple with the first element of normalizable pages and the second element of NOT normalizable pages
    */
-  async separateNormalizableAndNotNormalizablePages(pageIds: ObjectIdLike[]): Promise<[(PageDocument & { _id: any })[], (PageDocument & { _id: any })[]]> {
-    if (pageIds.length > LIMIT_FOR_MULTIPLE_PAGE_OP) {
+  async separateNormalizableAndNotNormalizablePages(pages): Promise<[(PageDocument & { _id: any })[], (PageDocument & { _id: any })[]]> {
+    if (pages.length > LIMIT_FOR_MULTIPLE_PAGE_OP) {
       throw Error(`The maximum number of pageIds allowed is ${LIMIT_FOR_MULTIPLE_PAGE_OP}.`);
     }
 
-    const Page = mongoose.model('Page') as unknown as PageModel;
-    const { PageQueryBuilder } = Page;
     const shouldCheckDescendants = true;
     const shouldIncludeNotMigratedPages = true;
 
     const normalizable: (PageDocument & { _id: any })[] = [];
     const nonNormalizable: (PageDocument & { _id: any })[] = []; // can be used to tell user which page failed to migrate
 
-    const builder = new PageQueryBuilder(Page.find());
-    builder.addConditionToListByPageIdsArray(pageIds);
-
-    const pages = await builder.query.exec();
-
     for await (const page of pages) {
       const {
         path, grant, grantedUsers: grantedUserIds, grantedGroup: grantedGroupId,

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

@@ -1439,7 +1439,9 @@ class PageService {
     return nDeletedNonEmptyPages;
   }
 
-  async deleteMultiplePages(pagesToDelete, user, isCompletely: boolean, isRecursively: boolean): Promise<void> {
+  async deleteMultiplePages(pagesToDelete, user, options): Promise<void> {
+    const { isRecursively, isCompletely } = options;
+
     if (pagesToDelete.length > LIMIT_FOR_MULTIPLE_PAGE_OP) {
       throw Error(`The maximum number of pages is ${LIMIT_FOR_MULTIPLE_PAGE_OP}.`);
     }
@@ -1447,8 +1449,6 @@ class PageService {
     // omit duplicate paths if isRecursively true, omit empty pages if isRecursively false
     const pages = isRecursively ? omitDuplicateAreaPageFromPages(pagesToDelete) : pagesToDelete.filter(p => !p.isEmpty);
 
-    // TODO: insertMany PageOperationBlock if isRecursively true
-
     if (isCompletely) {
       for await (const page of pages) {
         await this.deleteCompletely(page, user, {}, isRecursively);
@@ -1519,6 +1519,7 @@ class PageService {
       throw Error(`This page cannot be reverted since a page with path "${originPage.path}" already exists. Rename the existing pages first.`);
     }
 
+    // 2. Revert target
     const parent = await Page.getParentAndFillAncestors(newPath);
     const updatedPage = await Page.findByIdAndUpdate(page._id, {
       $set: {
@@ -1811,7 +1812,17 @@ class PageService {
     await inAppNotificationService.emitSocketIo(targetUsers);
   }
 
-  async normalizeParentByPageIds(pageIds: ObjectIdLike[], user): Promise<void> {
+  async normalizeParentByPageIds(pageIds: ObjectIdLike[], user, isRecursively: boolean): Promise<void> {
+    if (isRecursively) {
+      const Page = mongoose.model('Page') as unknown as PageModel;
+      const pages = await Page.findByPageIdsToEdit(pageIds, user, false);
+
+      // DO NOT await !!
+      this.normalizeParentRecursivelyByPageIds(pages, user);
+
+      return;
+    }
+
     for await (const pageId of pageIds) {
       try {
         const normalizedPage = await this.normalizeParentByPageId(pageId, user);
@@ -1830,16 +1841,12 @@ class PageService {
     }
   }
 
-  private async normalizeParentByPageId(pageId: ObjectIdLike, user) {
+  private async normalizeParentByPageId(page, user) {
     const Page = mongoose.model('Page') as unknown as PageModel;
-    const target = await Page.findByIdAndViewerToEdit(pageId, user);
-    if (target == null) {
-      throw Error('target does not exist or forbidden');
-    }
 
     const {
       path, grant, grantedUsers: grantedUserIds, grantedGroup: grantedGroupId,
-    } = target;
+    } = page;
 
     // check if any page exists at target path already
     const existingPage = await Page.findOne({ path });
@@ -1850,7 +1857,7 @@ class PageService {
     /*
      * UserGroup & Owner validation
      */
-    if (target.grant !== Page.GRANT_RESTRICTED) {
+    if (grant !== Page.GRANT_RESTRICTED) {
       let isGrantNormalized = false;
       try {
         const shouldCheckDescendants = true;
@@ -1873,35 +1880,35 @@ class PageService {
 
     // replace if empty page exists
     if (existingPage != null && existingPage.isEmpty) {
-      await Page.replaceTargetWithPage(existingPage, target, true);
-      updatedPage = await Page.findById(pageId);
+      await Page.replaceTargetWithPage(existingPage, page, true);
+      updatedPage = await Page.findById(page._id);
     }
     else {
       // getParentAndFillAncestors
-      const parent = await Page.getParentAndFillAncestors(target.path);
-      updatedPage = await Page.findOneAndUpdate({ _id: pageId }, { parent: parent._id }, { new: true });
+      const parent = await Page.getParentAndFillAncestors(page.path);
+      updatedPage = await Page.findOneAndUpdate({ _id: page._id }, { parent: parent._id }, { new: true });
     }
 
     return updatedPage;
   }
 
-  // TODO: this should be resumable
-  async normalizeParentRecursivelyByPageIds(pageIds, user) {
-    const Page = mongoose.model('Page') as unknown as PageModel;
-
-    if (pageIds == null || pageIds.length === 0) {
+  async normalizeParentRecursivelyByPageIds(pages, user): Promise<void> {
+    /*
+     * Main Operation
+     */
+    if (pages == null || pages.length === 0) {
       logger.error('pageIds is null or 0 length.');
       return;
     }
 
-    if (pageIds.length > LIMIT_FOR_MULTIPLE_PAGE_OP) {
+    if (pages.length > LIMIT_FOR_MULTIPLE_PAGE_OP) {
       throw Error(`The maximum number of pageIds allowed is ${LIMIT_FOR_MULTIPLE_PAGE_OP}.`);
     }
 
     let normalizablePages;
     let nonNormalizablePages;
     try {
-      [normalizablePages, nonNormalizablePages] = await this.crowi.pageGrantService.separateNormalizableAndNotNormalizablePages(pageIds);
+      [normalizablePages, nonNormalizablePages] = await this.crowi.pageGrantService.separateNormalizableAndNotNormalizablePages(pages);
     }
     catch (err) {
       throw err;
@@ -1917,7 +1924,16 @@ class PageService {
       // socket.emit('normalizeParentRecursivelyByPageIds', { error: err.message }); TODO: use socket to tell user
     }
 
-    const pagesToNormalize = omitDuplicateAreaPageFromPages(normalizablePages);
+    /*
+     * Sub Operation
+     */
+    await this.normalizeParentRecursivelyByPageIdsSubOperation(normalizablePages, user);
+  }
+
+  async normalizeParentRecursivelyByPageIdsSubOperation(pages, user): Promise<void> {
+    const Page = mongoose.model('Page') as unknown as PageModel;
+
+    const pagesToNormalize = omitDuplicateAreaPageFromPages(pages);
     const pageIdsToNormalize = pagesToNormalize.map(p => p._id);
     const pathsToNormalize = Array.from(new Set(pagesToNormalize.map(p => p.path)));