Merge pull request #5181 from weseek/imprv/normalize-legacy-pages

imprv: Normalize legacy pages parent

packages/app/src/client/services/AdminAppContainer.js

@@ -452,10 +452,9 @@ export default class AdminAppContainer extends Container {
   /**
    * Start v5 page migration
    * @memberOf AdminAppContainer
-   * @property action takes only 'initialMigration' for now. 'initialMigration' will start or resume migration
    */
-  async v5PageMigrationHandler(action) {
-    const response = await this.appContainer.apiv3.post('/pages/v5-schema-migration', { action });
+  async v5PageMigrationHandler() {
+    const response = await this.appContainer.apiv3.post('/pages/v5-schema-migration');
     const { isV5Compatible } = response.data;
     return { isV5Compatible };
   }

packages/app/src/components/Admin/App/V5PageMigration.tsx

@@ -6,7 +6,7 @@ import { withUnstatedContainers } from '../../UnstatedUtils';
 import { toastSuccess, toastError } from '../../../client/util/apiNotification';
 
 type Props = {
-  adminAppContainer: typeof AdminAppContainer & { v5PageMigrationHandler: (action: string) => Promise<{ isV5Compatible: boolean }> },
+  adminAppContainer: typeof AdminAppContainer & { v5PageMigrationHandler: () => Promise<{ isV5Compatible: boolean }> },
 }
 
 const V5PageMigration: FC<Props> = (props: Props) => {
@@ -17,7 +17,7 @@ const V5PageMigration: FC<Props> = (props: Props) => {
   const onConfirm = async() => {
     setIsV5PageMigrationModalShown(false);
     try {
-      const { isV5Compatible } = await adminAppContainer.v5PageMigrationHandler('initialMigration');
+      const { isV5Compatible } = await adminAppContainer.v5PageMigrationHandler();
       if (isV5Compatible) {
 
         return toastSuccess(t('admin:v5_page_migration.already_upgraded'));

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

@@ -165,7 +165,7 @@ schema.statics.createEmptyPage = async function(
  * @param exPage a page document to be replaced
  * @returns Promise<void>
  */
-schema.statics.replaceTargetWithEmptyPage = async function(exPage): Promise<void> {
+schema.statics.replaceTargetWithPage = async function(exPage, pageToReplaceWith?): Promise<void> {
   // find parent
   const parent = await this.findOne({ _id: exPage.parent });
   if (parent == null) {
@@ -173,7 +173,7 @@ schema.statics.replaceTargetWithEmptyPage = async function(exPage): Promise<void
   }
 
   // create empty page at path
-  const newTarget = await this.createEmptyPage(exPage.path, parent);
+  const newTarget = pageToReplaceWith == null ? await this.createEmptyPage(exPage.path, parent) : pageToReplaceWith;
 
   // find children by ex-page _id
   const children = await this.find({ parent: exPage._id });

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

@@ -179,14 +179,14 @@ module.exports = (crowi) => {
       body('isRenameRedirect').if(value => value != null).isBoolean().withMessage('isRenameRedirect must be boolean'),
       body('isRemainMetadata').if(value => value != null).isBoolean().withMessage('isRemainMetadata must be boolean'),
     ],
-
     duplicatePage: [
       body('pageId').isMongoId().withMessage('pageId is required'),
       body('pageNameInput').trim().isLength({ min: 1 }).withMessage('pageNameInput is required'),
       body('isRecursively').if(value => value != null).isBoolean().withMessage('isRecursively must be boolean'),
     ],
-    v5PageMigration: [
-      body('action').isString().withMessage('action is required'),
+    legacyPagesMigration: [
+      body('pageIds').isArray().withMessage('pageIds is required'),
+      body('isRecursively').isBoolean().withMessage('isRecursively is required'),
     ],
   };
 
@@ -708,26 +708,14 @@ module.exports = (crowi) => {
 
   });
 
-  router.post('/v5-schema-migration', accessTokenParser, loginRequired, adminRequired, csrf, validator.v5PageMigration, apiV3FormValidator, async(req, res) => {
-    const { action, pageIds } = req.body;
+  router.post('/v5-schema-migration', accessTokenParser, loginRequired, adminRequired, csrf, async(req, res) => {
     const isV5Compatible = crowi.configManager.getConfig('crowi', 'app:isV5Compatible');
     const Page = crowi.model('Page');
 
     try {
-      switch (action) {
-        case 'initialMigration':
-          if (!isV5Compatible) {
-            // this method throws and emit socketIo event when error occurs
-            crowi.pageService.v5InitialMigration(Page.GRANT_PUBLIC); // not await
-          }
-          break;
-        case 'privateLegacyPages':
-          crowi.pageService.v5MigrationByPageIds(pageIds);
-          break;
-
-        default:
-          logger.error(`${action} action is not supported.`);
-          return res.apiv3Err(new ErrorV3('This action is not supported.', 'not_supported'), 400);
+      if (!isV5Compatible) {
+        // this method throws and emit socketIo event when error occurs
+        crowi.pageService.v5InitialMigration(Page.GRANT_PUBLIC); // not await
       }
     }
     catch (err) {
@@ -737,6 +725,26 @@ module.exports = (crowi) => {
     return res.apiv3({ isV5Compatible });
   });
 
+  // eslint-disable-next-line max-len
+  router.post('/legacy-pages-migration', accessTokenParser, loginRequired, adminRequired, csrf, validator.legacyPagesMigration, apiV3FormValidator, async(req, res) => {
+    const { pageIds, isRecursively } = req.body;
+
+    if (isRecursively) {
+      // this method innerly uses socket to send message
+      crowi.pageService.normalizeParentRecursivelyByPageIds(pageIds);
+    }
+    else {
+      try {
+        await crowi.pageService.normalizeParentByPageIds(pageIds);
+      }
+      catch (err) {
+        return res.apiv3Err(new ErrorV3(`Failed to migrate pages: ${err.message}`), 500);
+      }
+    }
+
+    return res.apiv3({});
+  });
+
   router.get('/v5-migration-status', accessTokenParser, loginRequired, async(req, res) => {
     try {
       const isV5Compatible = crowi.configManager.getConfig('crowi', 'app:isV5Compatible');

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

@@ -3,7 +3,7 @@ import { pagePathUtils, pathUtils } from '@growi/core';
 import escapeStringRegexp from 'escape-string-regexp';
 
 import UserGroup from '~/server/models/user-group';
-import { PageModel } from '~/server/models/page';
+import { PageDocument, PageModel } from '~/server/models/page';
 import { PageQueryBuilder } from '../models/obsolete-page';
 import { isIncludesObjectId, excludeTestIdsFromTargetIds } from '~/server/util/compare-objectId';
 
@@ -212,7 +212,7 @@ class PageGrantService {
    * @param targetPath string of the target path
    * @returns Promise<ComparableAncestor>
    */
-  private async generateComparableAncestor(targetPath: string): Promise<ComparableAncestor> {
+  private async generateComparableAncestor(targetPath: string, includeNotMigratedPages: boolean): Promise<ComparableAncestor> {
     const Page = mongoose.model('Page') as unknown as PageModel;
     const UserGroupRelation = mongoose.model('UserGroupRelation') as any; // TODO: Typescriptize model
 
@@ -223,6 +223,9 @@ class PageGrantService {
      * make granted users list of ancestor's
      */
     const builderForAncestors = new PageQueryBuilder(Page.find(), false);
+    if (!includeNotMigratedPages) {
+      builderForAncestors.addConditionAsMigrated();
+    }
     const ancestors = await builderForAncestors
       .addConditionToListOnlyAncestors(targetPath)
       .addConditionToSortPagesByDescPath()
@@ -254,7 +257,7 @@ class PageGrantService {
    * @param targetPath string of the target path
    * @returns ComparableDescendants
    */
-  private async generateComparableDescendants(targetPath: string): Promise<ComparableDescendants> {
+  private async generateComparableDescendants(targetPath: string, includeNotMigratedPages: boolean): Promise<ComparableDescendants> {
     const Page = mongoose.model('Page') as unknown as PageModel;
 
     /*
@@ -263,12 +266,17 @@ class PageGrantService {
     const pathWithTrailingSlash = addTrailingSlash(targetPath);
     const startsPattern = escapeStringRegexp(pathWithTrailingSlash);
 
+    const $match: any = {
+      path: new RegExp(`^${startsPattern}`),
+      isEmpty: { $ne: true },
+    };
+    if (includeNotMigratedPages) {
+      $match.parent = { $ne: null };
+    }
+
     const result = await Page.aggregate([
       { // match to descendants excluding empty pages
-        $match: {
-          path: new RegExp(`^${startsPattern}`),
-          isEmpty: { $ne: true },
-        },
+        $match,
       },
       {
         $project: {
@@ -310,16 +318,18 @@ class PageGrantService {
 
   /**
    * About the rule of validation, see: https://dev.growi.org/61b2cdabaa330ce7d8152844
+   * Only v5 schema pages will be used to compare.
    * @returns Promise<boolean>
    */
   async isGrantNormalized(
-      targetPath: string, grant, grantedUserIds?: ObjectIdLike[], grantedGroupId?: ObjectIdLike, shouldCheckDescendants = false,
+      // eslint-disable-next-line max-len
+      targetPath: string, grant, grantedUserIds?: ObjectIdLike[], grantedGroupId?: ObjectIdLike, shouldCheckDescendants = false, includeNotMigratedPages = false,
   ): Promise<boolean> {
     if (isTopPage(targetPath)) {
       return true;
     }
 
-    const comparableAncestor = await this.generateComparableAncestor(targetPath);
+    const comparableAncestor = await this.generateComparableAncestor(targetPath, includeNotMigratedPages);
 
     if (!shouldCheckDescendants) { // checking the parent is enough
       const comparableTarget = await this.generateComparableTarget(grant, grantedUserIds, grantedGroupId, false);
@@ -327,11 +337,42 @@ class PageGrantService {
     }
 
     const comparableTarget = await this.generateComparableTarget(grant, grantedUserIds, grantedGroupId, true);
-    const comparableDescendants = await this.generateComparableDescendants(targetPath);
+    const comparableDescendants = await this.generateComparableDescendants(targetPath, includeNotMigratedPages);
 
     return this.processValidation(comparableTarget, comparableAncestor, comparableDescendants);
   }
 
+  async separateNormalizedAndNonNormalizedPages(pageIds: ObjectIdLike[]): Promise<[(PageDocument & { _id: any })[], (PageDocument & { _id: any })[]]> {
+    const Page = mongoose.model('Page') as unknown as PageModel;
+    const { PageQueryBuilder } = Page;
+    const shouldCheckDescendants = true;
+    const shouldIncludeNotMigratedPages = true;
+
+    const normalizedPages: (PageDocument & { _id: any })[] = [];
+    const nonNormalizedPages: (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,
+      } = page;
+
+      const isNormalized = await this.isGrantNormalized(path, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants, shouldIncludeNotMigratedPages);
+      if (isNormalized) {
+        normalizedPages.push(page);
+      }
+      else {
+        nonNormalizedPages.push(page);
+      }
+    }
+
+    return [normalizedPages, nonNormalizedPages];
+  }
+
 }
 
 export default PageGrantService;

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

@@ -14,6 +14,7 @@ import {
 import { stringifySnapshot } from '~/models/serializers/in-app-notification-snapshot/page';
 import ActivityDefine from '../util/activityDefine';
 import { IPage } from '~/interfaces/page';
+import { ObjectIdLike } from '../interfaces/mongoose-utils';
 
 const debug = require('debug')('growi:services:page');
 
@@ -942,6 +943,12 @@ class PageService {
       throw new Error('Page is not deletable.');
     }
 
+    // replace with an empty page
+    const shouldReplace = !isRecursively && await Page.exists({ parent: page._id });
+    if (shouldReplace) {
+      await Page.replaceTargetWithPage(page);
+    }
+
     if (isRecursively) {
       this.deleteDescendantsWithStream(page, user, shouldUseV4Process); // use the same process in both version v4 and v5
     }
@@ -1188,7 +1195,7 @@ class PageService {
     // replace with an empty page
     const shouldReplace = !isRecursively && !isTrashPage(page.path) && await Page.exists({ parent: page._id });
     if (shouldReplace) {
-      await Page.replaceTargetWithEmptyPage(page);
+      await Page.replaceTargetWithPage(page);
     }
 
     await this.deleteCompletelyOperation(ids, paths);
@@ -1563,16 +1570,76 @@ class PageService {
     await inAppNotificationService.emitSocketIo(targetUsers);
   }
 
-  async v5MigrationByPageIds(pageIds) {
-    const Page = mongoose.model('Page');
+  async normalizeParentByPageIds(pageIds: ObjectIdLike[]): Promise<void> {
+    for await (const pageId of pageIds) {
+      try {
+        await this.normalizeParentByPageId(pageId);
+      }
+      catch (err) {
+        // socket.emit('normalizeParentByPageIds', { error: err.message }); TODO: use socket to tell user
+      }
+    }
+  }
+
+  private async normalizeParentByPageId(pageId: ObjectIdLike) {
+    const Page = mongoose.model('Page') as unknown as PageModel;
+    const target = await Page.findById(pageId);
+    if (target == null) {
+      throw Error('target does not exist');
+    }
+
+    const {
+      path, grant, grantedUsers: grantedUserIds, grantedGroup: grantedGroupId,
+    } = target;
+
+    /*
+     * UserGroup & Owner validation
+     */
+    if (target.grant !== Page.GRANT_RESTRICTED) {
+      let isGrantNormalized = false;
+      try {
+        const shouldCheckDescendants = true;
+
+        isGrantNormalized = await this.crowi.pageGrantService.isGrantNormalized(path, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants);
+      }
+      catch (err) {
+        logger.error(`Failed to validate grant of page at "${path}"`, err);
+        throw err;
+      }
+      if (!isGrantNormalized) {
+        throw Error('This page cannot be migrated since the selected grant or grantedGroup is not assignable to this page.');
+      }
+    }
+    else {
+      throw Error('Restricted pages can not be migrated');
+    }
+
+    // getParentAndFillAncestors
+    const parent = await Page.getParentAndFillAncestors(target.path);
 
+    return Page.updateOne({ _id: pageId }, { parent: parent._id });
+  }
+
+  async normalizeParentRecursivelyByPageIds(pageIds) {
     if (pageIds == null || pageIds.length === 0) {
       logger.error('pageIds is null or 0 length.');
       return;
     }
 
+    const [normalizedIds, notNormalizedPaths] = await this.crowi.pageGrantService.separateNormalizedAndNonNormalizedPages(pageIds);
+
+    if (normalizedIds.length === 0) {
+      // socket.emit('normalizeParentRecursivelyByPageIds', { error: err.message }); TODO: use socket to tell user
+      return;
+    }
+
+    if (notNormalizedPaths.length !== 0) {
+      // TODO: iterate notNormalizedPaths and send socket error to client so that the user can know which path failed to migrate
+      // socket.emit('normalizeParentRecursivelyByPageIds', { error: err.message }); TODO: use socket to tell user
+    }
+
     // generate regexps
-    const regexps = await this._generateRegExpsByPageIds(pageIds);
+    const regexps = await this._generateRegExpsByPageIds(normalizedIds);
 
     // migrate recursively
     try {
@@ -1580,7 +1647,7 @@ class PageService {
     }
     catch (err) {
       logger.error('V5 initial miration failed.', err);
-      // socket.emit('v5InitialMirationFailed', { error: err.message }); TODO: use socket to tell user
+      // socket.emit('normalizeParentRecursivelyByPageIds', { error: err.message }); TODO: use socket to tell user
 
       throw err;
     }
@@ -1681,7 +1748,7 @@ class PageService {
     }
 
     const { pages } = result;
-    const regexps = pages.map(page => new RegExp(`^${page.path}`));
+    const regexps = pages.map(page => new RegExp(`^${escapeStringRegexp(page.path)}`));
 
     return regexps;
   }

packages/app/src/test/integration/service/v5-migration.test.js

@@ -16,7 +16,7 @@ describe('V5 page migration', () => {
   });
 
 
-  describe('v5MigrationByPageIds()', () => {
+  describe('normalizeParentRecursivelyByPageIds()', () => {
     test('should migrate all pages specified by pageIds', async() => {
       jest.restoreAllMocks();
 
@@ -50,7 +50,7 @@ describe('V5 page migration', () => {
 
       const pageIds = pages.map(page => page._id);
       // migrate
-      await crowi.pageService.v5MigrationByPageIds(pageIds);
+      await crowi.pageService.normalizeParentRecursivelyByPageIds(pageIds);
 
       const migratedPages = await Page.find({
         path: {