resolve conflicts cherry picking d8480288e4

packages/app/src/server/models/obsolete-page.js

@@ -718,6 +718,11 @@ export const getPageSchema = (crowi) => {
 
     pageEvent.emit('create', savedPage, user);
 
+    // update scopes for descendants
+    if (options.overwriteScopesOfDescendants) {
+      Page.applyScopesToDescendantsAsyncronously(savedPage, user, true);
+    }
+
     return savedPage;
   };
 
@@ -748,15 +753,27 @@ export const getPageSchema = (crowi) => {
       }
     }
 
+    // update scopes for descendants
+    if (options.overwriteScopesOfDescendants) {
+      this.applyScopesToDescendantsAsyncronously(savedPage, user, true);
+    }
+
 
     pageEvent.emit('update', savedPage, user);
 
     return savedPage;
   };
 
-  pageSchema.statics.applyScopesToDescendantsAsyncronously = async function(parentPage, user) {
+  pageSchema.statics.applyScopesToDescendantsAsyncronously = async function(parentPage, user, isV4 = false) {
     const builder = new this.PageQueryBuilder(this.find());
-    builder.addConditionToListWithDescendants(parentPage.path);
+    builder.addConditionToListOnlyDescendants(parentPage.path);
+
+    if (isV4) {
+      builder.addConditionAsRootOrNotOnTree();
+    }
+    else {
+      builder.addConditionAsOnTree();
+    }
 
     // add grant conditions
     await addConditionToFilteringByViewerToEdit(builder, user);
@@ -764,15 +781,12 @@ export const getPageSchema = (crowi) => {
     // get all pages that the specified user can update
     const pages = await builder.query.exec();
 
-    for (const page of pages) {
-      // skip parentPage
-      if (page.id === parentPage.id) {
-        continue;
-      }
+    const promises = pages.map(async(page) => {
+      await page.applyScope(user, parentPage.grant, parentPage.grantedGroup);
+      await page.save();
+    });
 
-      page.applyScope(user, parentPage.grant, parentPage.grantedGroup);
-      page.save();
-    }
+    await Promise.all(promises);
   };
 
   pageSchema.statics.removeByPath = function(path) {

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

@@ -340,7 +340,7 @@ export class PageQueryBuilder {
           { grant: { $ne: GRANT_SPECIFIED } },
         ],
       });
-    this.addConditionAsNotMigrated();
+    this.addConditionAsRootOrNotOnTree();
     this.addConditionAsNonRootPage();
     this.addConditionToExcludeTrashed();
     await this.addConditionForParentNormalization(user);
@@ -384,7 +384,7 @@ export class PageQueryBuilder {
     return this;
   }
 
-  addConditionAsNotMigrated(): PageQueryBuilder {
+  addConditionAsRootOrNotOnTree(): PageQueryBuilder {
     this.query = this.query
       .and({ parent: null });
 
@@ -956,6 +956,7 @@ export type PageCreateOptions = {
   format?: string
   grantUserGroupId?: ObjectIdLike
   grant?: number
+  overwriteScopesOfDescendants?: boolean
 }
 
 /*
@@ -1044,7 +1045,7 @@ export default (crowi): any => {
 
       if (options.overwriteScopesOfDescendants) {
         const updateGrantInfo = await pageGrantService.generateUpdateGrantInfo(user, grant, options.grantUserGroupId);
-        const canOverwriteDescendants = await pageGrantService.canOverwriteDescendants(pageData, user, updateGrantInfo);
+        const canOverwriteDescendants = await pageGrantService.canOverwriteDescendants(pageData.path, user, updateGrantInfo);
 
         if (!canOverwriteDescendants) {
           throw Error('Cannot overwrite scopes of descendants.');
@@ -1111,6 +1112,12 @@ export default (crowi): any => {
     }
 
     // Sub operation
+    // update scopes for descendants
+    // TODO: remove await (cause: tests are not working with await)
+    if (options.overwriteScopesOfDescendants) {
+      await this.applyScopesToDescendantsAsyncronously(savedPage, user);
+    }
+
     // 1. Update descendantCount
     const shouldPlusDescCount = !wasOnTree && shouldBeOnTree;
     const shouldMinusDescCount = wasOnTree && !shouldBeOnTree;

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

@@ -298,7 +298,7 @@ module.exports = (crowi) => {
     // check whether path starts slash
     path = pathUtils.addHeadingSlash(path);
 
-    const options = {};
+    const options = { overwriteScopesOfDescendants };
     if (grant != null) {
       options.grant = grant;
       options.grantUserGroupId = grantUserGroupId;
@@ -323,11 +323,6 @@ module.exports = (crowi) => {
       revision: serializeRevisionSecurely(createdPage.revision),
     };
 
-    // update scopes for descendants
-    if (overwriteScopesOfDescendants) {
-      Page.applyScopesToDescendantsAsyncronously(createdPage, req.user);
-    }
-
     const parameters = {
       targetModel: SupportedTargetModel.MODEL_PAGE,
       target: createdPage,

packages/app/src/server/routes/page.js

@@ -870,7 +870,7 @@ module.exports = function(crowi, app) {
       return res.json(ApiResponse.error('Page exists', 'already_exists'));
     }
 
-    const options = {};
+    const options = { overwriteScopesOfDescendants };
     if (grant != null) {
       options.grant = grant;
       options.grantUserGroupId = grantUserGroupId;
@@ -891,11 +891,6 @@ module.exports = function(crowi, app) {
     };
     res.json(ApiResponse.success(result));
 
-    // update scopes for descendants
-    if (overwriteScopesOfDescendants) {
-      Page.applyScopesToDescendantsAsyncronously(createdPage, req.user);
-    }
-
     // global notification
     try {
       await globalNotificationService.fire(GlobalNotificationSetting.EVENT.PAGE_CREATE, createdPage, req.user);
@@ -1044,11 +1039,6 @@ module.exports = function(crowi, app) {
     };
     res.json(ApiResponse.success(result));
 
-    // update scopes for descendants
-    if (overwriteScopesOfDescendants) {
-      Page.applyScopesToDescendantsAsyncronously(page, req.user);
-    }
-
     // global notification
     try {
       await globalNotificationService.fire(GlobalNotificationSetting.EVENT.PAGE_EDIT, page, req.user);

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

@@ -52,8 +52,8 @@ type UpdateGrantInfo = {
 
 type DescendantPagesGrantInfo = {
   grantSet: Set<number>,
-  grantedUserIds: Set<ObjectIdLike>, // only me users
-  grantedUserGroupIds: Set<ObjectIdLike>, // user groups
+  grantedUserIds: Set<ObjectIdLike>, // all only me users of descendant pages
+  grantedUserGroupIds: Set<ObjectIdLike>, // all user groups of descendant pages
 };
 
 /**
@@ -505,12 +505,12 @@ class PageGrantService {
 
   /**
    * see: https://dev.growi.org/635a314eac6bcd85cbf359fc
-   * @param targetPage
+   * @param {string} targetPath
    * @param operator
    * @param {UpdateGrantInfo} updateGrantInfo
    * @returns {Promise<boolean>}
    */
-  async canOverwriteDescendants(targetPage, operator: { _id: ObjectIdLike }, updateGrantInfo: UpdateGrantInfo): Promise<boolean> {
+  async canOverwriteDescendants(targetPath: string, operator: { _id: ObjectIdLike }, updateGrantInfo: UpdateGrantInfo): Promise<boolean> {
     const UserGroupRelationModel = mongoose.model('UserGroupRelation') as any; // TODO: TypeScriptize model
 
     const relatedGroupIds = await UserGroupRelationModel.findAllUserGroupIdsRelatedToUser(operator);
@@ -519,7 +519,7 @@ class PageGrantService {
       userGroupIds: new Set<ObjectIdLike>(relatedGroupIds),
     };
 
-    const comparableDescendants = await this.generateComparableDescendants(targetPage.path, operator);
+    const comparableDescendants = await this.generateComparableDescendants(targetPath, operator);
 
     const grantSet = new Set<PageGrant>();
     if (comparableDescendants.isPublicExist) {
@@ -533,8 +533,8 @@ class PageGrantService {
     }
     const descendantPagesGrantInfo = {
       grantSet,
-      grantedUserIds: new Set(comparableDescendants.grantedUserIds), // only me users of descendant pages
-      grantedUserGroupIds: new Set(comparableDescendants.grantedGroupIds), // user groups of descendant pages
+      grantedUserIds: new Set(comparableDescendants.grantedUserIds), // all only me users of descendant pages
+      grantedUserGroupIds: new Set(comparableDescendants.grantedGroupIds), // all user groups of descendant pages
     };
 
     return this.calcCanOverwriteDescendants(operatorGrantInfo, updateGrantInfo, descendantPagesGrantInfo);
@@ -580,18 +580,18 @@ class PageGrantService {
   private calcIsAllDescendantsGrantedByOperator(operatorGrantInfo: OperatorGrantInfo, descendantPagesGrantInfo: DescendantPagesGrantInfo): boolean {
     if (descendantPagesGrantInfo.grantSet.has(PageGrant.GRANT_OWNER)) {
       const isNonApplicableOwnerExist = descendantPagesGrantInfo.grantedUserIds.size >= 2
-        || !descendantPagesGrantInfo.grantedUserIds.has(operatorGrantInfo.userId);
+        || !isIncludesObjectId([...descendantPagesGrantInfo.grantedUserIds], operatorGrantInfo.userId);
       if (isNonApplicableOwnerExist) {
         return false;
       }
     }
 
     if (descendantPagesGrantInfo.grantSet.has(PageGrant.GRANT_USER_GROUP)) {
-      const isOtherFamilyGroupExist = excludeTestIdsFromTargetIds(
-        [...operatorGrantInfo.userGroupIds], [...descendantPagesGrantInfo.grantedUserGroupIds],
+      const isNonApplicableGroupExist = excludeTestIdsFromTargetIds(
+        [...descendantPagesGrantInfo.grantedUserGroupIds], [...operatorGrantInfo.userGroupIds],
       ).length > 0;
 
-      if (isOtherFamilyGroupExist) {
+      if (isNonApplicableGroupExist) {
         return false;
       }
     }

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

@@ -340,7 +340,7 @@ class PageService {
     const { PageQueryBuilder } = Page;
 
     const builder = new PageQueryBuilder(Page.find(), true)
-      .addConditionAsNotMigrated() // to avoid affecting v5 pages
+      .addConditionAsRootOrNotOnTree() // to avoid affecting v5 pages
       .addConditionToListOnlyDescendants(targetPagePath);
 
     await Page.addConditionToFilteringByViewerToEdit(builder, userToOperate);
@@ -2376,7 +2376,7 @@ class PageService {
 
     // This validation is not 100% correct since it ignores user to count
     const builder = new PageQueryBuilder(Page.find());
-    builder.addConditionAsNotMigrated();
+    builder.addConditionAsRootOrNotOnTree();
     builder.addConditionToListWithDescendants(path);
     const nEstimatedNormalizationTarget: number = await builder.query.exec('count');
     if (nEstimatedNormalizationTarget === 0) {
@@ -3415,6 +3415,7 @@ class PageService {
       },
       shouldValidateGrant: boolean,
       user?,
+      options?: Partial<PageCreateOptions>,
   ): Promise<boolean> {
     const Page = mongoose.model('Page') as unknown as PageModel;
 
@@ -3454,6 +3455,15 @@ class PageService {
       if (!isGrantNormalized) {
         throw Error('The selected grant or grantedGroup is not assignable to this page.');
       }
+
+      if (options?.overwriteScopesOfDescendants) {
+        const updateGrantInfo = await this.crowi.pageGrantService.generateUpdateGrantInfo(user, grant, options.grantUserGroupId);
+        const canOverwriteDescendants = await this.crowi.pageGrantService.canOverwriteDescendants(path, user, updateGrantInfo);
+
+        if (!canOverwriteDescendants) {
+          throw Error('Cannot overwrite scopes of descendants.');
+        }
+      }
     }
 
     return true;
@@ -3534,6 +3544,11 @@ class PageService {
       logger.error('Failed to delete PageRedirect');
     }
 
+    // update scopes for descendants
+    if (options.overwriteScopesOfDescendants) {
+      Page.applyScopesToDescendantsAsyncronously(savedPage, user);
+    }
+
     return savedPage;
   }
 

packages/app/test/integration/models/v5.page.test.js

@@ -777,10 +777,22 @@ describe('Page', () => {
 
   describe('update', () => {
 
+    // TODO*
     const updatePage = async(page, newRevisionBody, oldRevisionBody, user, options = {}) => {
       const mockedEmitPageEventUpdate = jest.spyOn(Page, 'emitPageEventUpdate').mockReturnValue(null);
+      const mockedApplyScopesToDescendantsAsyncronously = jest.spyOn(Page, 'applyScopesToDescendantsAsyncronously').mockReturnValue(null);
+
       const savedPage = await Page.updatePage(page, newRevisionBody, oldRevisionBody, user, options);
+
+      const argsForApplyScopesToDescendantsAsyncronously = mockedApplyScopesToDescendantsAsyncronously.mock.calls[0];
+
       mockedEmitPageEventUpdate.mockRestore();
+      mockedApplyScopesToDescendantsAsyncronously.mockRestore();
+
+      if (options.overwriteScopesOfDescendants) {
+        await Page.applyScopesToDescendantsAsyncronously(...argsForApplyScopesToDescendantsAsyncronously);
+      }
+
       return savedPage;
     };
 
@@ -1222,11 +1234,11 @@ describe('Page', () => {
 
       // Changed
       const newGrant = PageGrant.GRANT_PUBLIC;
-      expect(updatedPage.grant).toBeNull(newGrant);
+      expect(updatedPage.grant).toBe(newGrant);
       // Not changed
-      expect(upodPagegBUpdated.grant).toBeNull(PageGrant.GRANT_USER_GROUP);
+      expect(upodPagegBUpdated.grant).toBe(PageGrant.GRANT_USER_GROUP);
       expect(upodPagegBUpdated.grantedGroup).toStrictEqual(upodPagegB.grantedGroup);
-      expect(upodPageonlyBUpdated.grant).toBeNull(PageGrant.GRANT_OWNER);
+      expect(upodPageonlyBUpdated.grant).toBe(PageGrant.GRANT_OWNER);
       expect(upodPageonlyBUpdated.grantedUsers).toStrictEqual(upodPageonlyB.grantedUsers);
     });
     test('(case 2) it should update all granted descendant pages when all descendant pages are granted by the operator', async() => {
@@ -1262,13 +1274,13 @@ describe('Page', () => {
       expect(updatedPage.grant).toBe(newGrant);
       expect(updatedPage.grantedUsers).toStrictEqual(newGrantedUsers);
       expect(upodPagegAUpdated.grant).toBe(newGrant);
-      expect(upodPagegAUpdated.grantedGroup).toStrictEqual(newGrantedUsers);
+      expect(upodPagegAUpdated.grantedUsers).toStrictEqual(newGrantedUsers);
       expect(upodPagegAIsolatedUpdated.grant).toBe(newGrant);
-      expect(upodPagegAIsolatedUpdated.grantedGroup).toStrictEqual(newGrantedUsers);
+      expect(upodPagegAIsolatedUpdated.grantedUsers).toStrictEqual(newGrantedUsers);
       expect(upodPageonlyAUpdated.grant).toBe(newGrant);
-      expect(upodPageonlyAUpdated.grantedGroup).toStrictEqual(newGrantedUsers);
+      expect(upodPageonlyAUpdated.grantedUsers).toStrictEqual(newGrantedUsers);
     });
-    test(`(case 3) it should update all granted descendant pages when update grant is GRANT_USER_GROUP
+    test.only(`(case 3) it should update all granted descendant pages when update grant is GRANT_USER_GROUP
     , all user groups of descendants are the children or itself of the update user group
     , and all users of descendants belong to the update user group`, async() => {
       const upodPagePublic = await Page.findOne({ path: '/public_upod_3' });