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

Merge pull request #8442 from weseek/feat/136139-140076-enable-and-fix-page-duplication-of-user-unrelated-pages

feat: Enable and fix page duplication of user unrelated pages
Yuki Takei 2 лет назад
Родитель
Сommit
e9a2c18a16

+ 0 - 5
apps/app/src/server/routes/apiv3/pages.js

@@ -826,11 +826,6 @@ module.exports = (crowi) => {
 
 
       const page = await Page.findByIdAndViewer(pageId, req.user, null, true);
       const page = await Page.findByIdAndViewer(pageId, req.user, null, true);
 
 
-      // TODO: remove in https://redmine.weseek.co.jp/issues/136139
-      if (page.grantedGroups != null && page.grantedGroups.length > 1) {
-        return res.apiv3Err('Cannot grant multiple groups to page at the moment');
-      }
-
       const isEmptyAndNotRecursively = page?.isEmpty && !isRecursively;
       const isEmptyAndNotRecursively = page?.isEmpty && !isRecursively;
       if (page == null || isEmptyAndNotRecursively) {
       if (page == null || isEmptyAndNotRecursively) {
         res.code = 'Page is not found';
         res.code = 'Page is not found';

+ 0 - 5
apps/app/src/server/routes/page.js

@@ -923,11 +923,6 @@ module.exports = function(crowi, app) {
       return res.json(ApiResponse.error(`Page '${pageId}' is not found or forbidden`, 'notfound_or_forbidden'));
       return res.json(ApiResponse.error(`Page '${pageId}' is not found or forbidden`, 'notfound_or_forbidden'));
     }
     }
 
 
-    // TODO: remove in https://redmine.weseek.co.jp/issues/136139
-    if (page.grantedGroups != null && page.grantedGroups.length > 1) {
-      return res.apiv3Err('Cannot grant multiple groups to page at the moment');
-    }
-
     // check whether path starts slash
     // check whether path starts slash
     newPagePath = pathUtils.addHeadingSlash(newPagePath);
     newPagePath = pathUtils.addHeadingSlash(newPagePath);
 
 

+ 33 - 30
apps/app/src/server/service/page/index.ts

@@ -13,17 +13,19 @@ import {
   pagePathUtils, pathUtils,
   pagePathUtils, pathUtils,
 } from '@growi/core/dist/utils';
 } from '@growi/core/dist/utils';
 import escapeStringRegexp from 'escape-string-regexp';
 import escapeStringRegexp from 'escape-string-regexp';
-import mongoose, { ObjectId, Cursor } from 'mongoose';
+import type { ObjectId, Cursor } from 'mongoose';
+import mongoose from 'mongoose';
 import streamToPromise from 'stream-to-promise';
 import streamToPromise from 'stream-to-promise';
 
 
 import { Comment } from '~/features/comment/server';
 import { Comment } from '~/features/comment/server';
 import ExternalUserGroupRelation from '~/features/external-user-group/server/models/external-user-group-relation';
 import ExternalUserGroupRelation from '~/features/external-user-group/server/models/external-user-group-relation';
 import { SupportedAction } from '~/interfaces/activity';
 import { SupportedAction } from '~/interfaces/activity';
 import { V5ConversionErrCode } from '~/interfaces/errors/v5-conversion-error';
 import { V5ConversionErrCode } from '~/interfaces/errors/v5-conversion-error';
+import type { IPageDeleteConfigValueToProcessValidation } from '~/interfaces/page-delete-config';
 import {
 import {
-  PageDeleteConfigValue, IPageDeleteConfigValueToProcessValidation, PageSingleDeleteCompConfigValue,
+  PageDeleteConfigValue, PageSingleDeleteCompConfigValue,
 } from '~/interfaces/page-delete-config';
 } from '~/interfaces/page-delete-config';
-import { PopulatedGrantedGroup } from '~/interfaces/page-grant';
+import type { PopulatedGrantedGroup } from '~/interfaces/page-grant';
 import {
 import {
   type IPageOperationProcessInfo, type IPageOperationProcessData, PageActionStage, PageActionType,
   type IPageOperationProcessInfo, type IPageOperationProcessData, PageActionStage, PageActionType,
 } from '~/interfaces/page-operation';
 } from '~/interfaces/page-operation';
@@ -35,12 +37,13 @@ import { createBatchStream } from '~/server/util/batch-stream';
 import loggerFactory from '~/utils/logger';
 import loggerFactory from '~/utils/logger';
 import { prepareDeleteConfigValuesForCalc } from '~/utils/page-delete-config';
 import { prepareDeleteConfigValuesForCalc } from '~/utils/page-delete-config';
 
 
-import { ObjectIdLike } from '../../interfaces/mongoose-utils';
+import type { ObjectIdLike } from '../../interfaces/mongoose-utils';
 import { Attachment } from '../../models';
 import { Attachment } from '../../models';
 import { PathAlreadyExistsError } from '../../models/errors';
 import { PathAlreadyExistsError } from '../../models/errors';
-import { IOptionsForCreate, IOptionsForUpdate } from '../../models/interfaces/page-operation';
-import PageOperation, { PageOperationDocument } from '../../models/page-operation';
-import { PageRedirectModel } from '../../models/page-redirect';
+import type { IOptionsForCreate, IOptionsForUpdate } from '../../models/interfaces/page-operation';
+import type { PageOperationDocument } from '../../models/page-operation';
+import PageOperation from '../../models/page-operation';
+import type { PageRedirectModel } from '../../models/page-redirect';
 import { serializePageSecurely } from '../../models/serializers/page-serializer';
 import { serializePageSecurely } from '../../models/serializers/page-serializer';
 import ShareLink from '../../models/share-link';
 import ShareLink from '../../models/share-link';
 import Subscription from '../../models/subscription';
 import Subscription from '../../models/subscription';
@@ -48,11 +51,11 @@ import UserGroupRelation from '../../models/user-group-relation';
 import { V5ConversionError } from '../../models/vo/v5-conversion-error';
 import { V5ConversionError } from '../../models/vo/v5-conversion-error';
 import { divideByType } from '../../util/granted-group';
 import { divideByType } from '../../util/granted-group';
 import { configManager } from '../config-manager';
 import { configManager } from '../config-manager';
-import { IPageGrantService } from '../page-grant';
+import type { IPageGrantService } from '../page-grant';
 import { preNotifyService } from '../pre-notify';
 import { preNotifyService } from '../pre-notify';
 
 
 import { BULK_REINDEX_SIZE, LIMIT_FOR_MULTIPLE_PAGE_OP } from './consts';
 import { BULK_REINDEX_SIZE, LIMIT_FOR_MULTIPLE_PAGE_OP } from './consts';
-import { IPageService } from './page-service';
+import type { IPageService } from './page-service';
 import { shouldUseV4Process } from './should-use-v4-process';
 import { shouldUseV4Process } from './should-use-v4-process';
 
 
 export * from './page-service';
 export * from './page-service';
@@ -1203,7 +1206,7 @@ class PageService implements IPageService {
     const shouldNormalize = this.shouldNormalizeParent(page);
     const shouldNormalize = this.shouldNormalizeParent(page);
     if (shouldNormalize) {
     if (shouldNormalize) {
       try {
       try {
-        await this.normalizeParentAndDescendantCountOfDescendants(newPagePath, user);
+        await this.normalizeParentAndDescendantCountOfDescendants(newPagePath, user, true);
         logger.info(`Successfully normalized duplicated descendant pages under "${newPagePath}"`);
         logger.info(`Successfully normalized duplicated descendant pages under "${newPagePath}"`);
       }
       }
       catch (err) {
       catch (err) {
@@ -1346,12 +1349,11 @@ class PageService implements IPageService {
       const isDuplicateTarget = !page.isEmpty
       const isDuplicateTarget = !page.isEmpty
       && (!onlyDuplicateUserRelatedResources || this.pageGrantService.isUserGrantedPageAccess(page, user, userRelatedGroups));
       && (!onlyDuplicateUserRelatedResources || this.pageGrantService.isUserGrantedPageAccess(page, user, userRelatedGroups));
 
 
-      let newPage;
       if (isDuplicateTarget) {
       if (isDuplicateTarget) {
         const grantedGroups = onlyDuplicateUserRelatedResources
         const grantedGroups = onlyDuplicateUserRelatedResources
           ? this.pageGrantService.getUserRelatedGrantedGroupsSyncronously(userRelatedGroups, page)
           ? this.pageGrantService.getUserRelatedGrantedGroupsSyncronously(userRelatedGroups, page)
           : page.grantedGroups;
           : page.grantedGroups;
-        newPage = {
+        const newPage = {
           _id: newPageId,
           _id: newPageId,
           path: newPagePath,
           path: newPagePath,
           creator: user._id,
           creator: user._id,
@@ -1364,8 +1366,8 @@ class PageService implements IPageService {
         newRevisions.push({
         newRevisions.push({
           _id: revisionId, pageId: newPageId, body: pageIdRevisionMapping[page._id].body, author: user._id, format: 'markdown',
           _id: revisionId, pageId: newPageId, body: pageIdRevisionMapping[page._id].body, author: user._id, format: 'markdown',
         });
         });
+        newPages.push(newPage);
       }
       }
-      newPages.push(newPage);
     });
     });
 
 
     await Page.insertMany(newPages, { ordered: false });
     await Page.insertMany(newPages, { ordered: false });
@@ -3064,7 +3066,7 @@ class PageService implements IPageService {
 
 
     // then migrate
     // then migrate
     try {
     try {
-      await this.normalizeParentRecursively(['/'], null, true);
+      await this.normalizeParentRecursively(['/'], null, false, true);
     }
     }
     catch (err) {
     catch (err) {
       logger.error('V5 initial miration failed.', err);
       logger.error('V5 initial miration failed.', err);
@@ -3098,8 +3100,8 @@ class PageService implements IPageService {
     }
     }
   }
   }
 
 
-  private async normalizeParentAndDescendantCountOfDescendants(path: string, user): Promise<void> {
-    await this.normalizeParentRecursively([path], user);
+  private async normalizeParentAndDescendantCountOfDescendants(path: string, user, isDuplicateOperation = false): Promise<void> {
+    await this.normalizeParentRecursively([path], user, isDuplicateOperation);
 
 
     // update descendantCount of descendant pages
     // update descendantCount of descendant pages
     await this.updateDescendantCountOfSelfAndDescendants(path);
     await this.updateDescendantCountOfSelfAndDescendants(path);
@@ -3111,7 +3113,7 @@ class PageService implements IPageService {
    * @param user To be used to filter pages to update. If null, only public pages will be updated.
    * @param user To be used to filter pages to update. If null, only public pages will be updated.
    * @returns Promise<void>
    * @returns Promise<void>
    */
    */
-  async normalizeParentRecursively(paths: string[], user: any | null, shouldEmitProgress = false): Promise<number> {
+  async normalizeParentRecursively(paths: string[], user: any | null, isDuplicateOperation = false, shouldEmitProgress = false): Promise<number> {
     const Page = mongoose.model('Page') as unknown as PageModel;
     const Page = mongoose.model('Page') as unknown as PageModel;
 
 
     const ancestorPaths = paths.flatMap(p => collectAncestorPaths(p, []));
     const ancestorPaths = paths.flatMap(p => collectAncestorPaths(p, []));
@@ -3127,12 +3129,14 @@ class PageService implements IPageService {
       ...(await ExternalUserGroupRelation.findAllUserGroupIdsRelatedToUser(user)),
       ...(await ExternalUserGroupRelation.findAllUserGroupIdsRelatedToUser(user)),
     ] : null;
     ] : null;
 
 
-    const grantFiltersByUser: { $or: any[] } = Page.generateGrantCondition(user, userGroups);
+    const grantFiltersByUser: { $or: any[] } | null = !isDuplicateOperation ? Page.generateGrantCondition(user, userGroups) : null;
 
 
-    return this._normalizeParentRecursively(pathAndRegExpsToNormalize, ancestorPaths, grantFiltersByUser, user, shouldEmitProgress);
+    return this._normalizeParentRecursively(pathAndRegExpsToNormalize, ancestorPaths, user, grantFiltersByUser, shouldEmitProgress);
   }
   }
 
 
-  private buildFilterForNormalizeParentRecursively(pathOrRegExps: (RegExp | string)[], publicPathsToNormalize: string[], grantFiltersByUser: { $or: any[] }) {
+  private buildFilterForNormalizeParentRecursively(
+      pathOrRegExps: (RegExp | string)[], publicPathsToNormalize: string[], grantFiltersByUser?: { $or: any[] } | null,
+  ) {
     const Page = mongoose.model('Page') as unknown as PageModel;
     const Page = mongoose.model('Page') as unknown as PageModel;
 
 
     const andFilter: any = {
     const andFilter: any = {
@@ -3166,7 +3170,7 @@ class PageService implements IPageService {
     // Merge filters
     // Merge filters
     const mergedFilter = {
     const mergedFilter = {
       $and: [
       $and: [
-        { $and: [grantFiltersByUser, ...andFilter.$and] },
+        { $and: grantFiltersByUser != null ? [grantFiltersByUser, ...andFilter.$and] : [...andFilter.$and] },
         { $or: orFilter.$or },
         { $or: orFilter.$or },
       ],
       ],
     };
     };
@@ -3177,8 +3181,8 @@ class PageService implements IPageService {
   private async _normalizeParentRecursively(
   private async _normalizeParentRecursively(
       pathOrRegExps: (RegExp | string)[],
       pathOrRegExps: (RegExp | string)[],
       publicPathsToNormalize: string[],
       publicPathsToNormalize: string[],
-      grantFiltersByUser: { $or: any[] },
       user,
       user,
+      grantFiltersByUser: { $or: any[] } | null,
       shouldEmitProgress = false,
       shouldEmitProgress = false,
       count = 0,
       count = 0,
       skiped = 0,
       skiped = 0,
@@ -3267,13 +3271,10 @@ class PageService implements IPageService {
         await Page.createEmptyPagesByPaths(parentPaths, aggregationPipeline);
         await Page.createEmptyPagesByPaths(parentPaths, aggregationPipeline);
 
 
         // 3. Find parents
         // 3. Find parents
-        const addGrantCondition = (builder) => {
-          builder.query = builder.query.and(grantFiltersByUser);
-
-          return builder;
-        };
         const builder2 = new PageQueryBuilder(Page.find(), true);
         const builder2 = new PageQueryBuilder(Page.find(), true);
-        addGrantCondition(builder2);
+        if (grantFiltersByUser != null) {
+          builder2.query = builder2.query.and(grantFiltersByUser);
+        }
         const parents = await builder2
         const parents = await builder2
           .addConditionToListByPathsArray(parentPaths)
           .addConditionToListByPathsArray(parentPaths)
           .addConditionToFilterByApplicableAncestors(publicPathsToNormalize)
           .addConditionToFilterByApplicableAncestors(publicPathsToNormalize)
@@ -3296,9 +3297,11 @@ class PageService implements IPageService {
                 path: { $in: pathOrRegExps.concat(publicPathsToNormalize) },
                 path: { $in: pathOrRegExps.concat(publicPathsToNormalize) },
               },
               },
               filterForApplicableAncestors,
               filterForApplicableAncestors,
-              grantFiltersByUser,
             ],
             ],
           };
           };
+          if (grantFiltersByUser != null) {
+            filter.$and.push(grantFiltersByUser);
+          }
 
 
           return {
           return {
             updateMany: {
             updateMany: {
@@ -3355,8 +3358,8 @@ class PageService implements IPageService {
       return this._normalizeParentRecursively(
       return this._normalizeParentRecursively(
         pathOrRegExps,
         pathOrRegExps,
         publicPathsToNormalize,
         publicPathsToNormalize,
-        grantFiltersByUser,
         user,
         user,
+        grantFiltersByUser,
         shouldEmitProgress,
         shouldEmitProgress,
         nextCount,
         nextCount,
         nextSkiped,
         nextSkiped,