fix v5 page tests

apps/app/src/features/external-user-group/server/models/external-user-group-relation.integ.ts

@@ -72,4 +72,26 @@ describe('ExternalUserGroupRelation model', () => {
       expect(relationsAfterRemoval.length).toBe(0);
     });
   });
+
+  describe('findAllUserIdsForUserGroups', () => {
+    const groupId1 = new mongoose.Types.ObjectId();
+    const groupId2 = new mongoose.Types.ObjectId();
+    const groupId3 = new mongoose.Types.ObjectId();
+
+    let user2;
+
+    beforeAll(async() => {
+      user2 = await User.create({
+        name: 'user2', username: 'user2', email: 'user2@example.com',
+      });
+
+      await ExternalUserGroupRelation.createRelations([groupId1, groupId2], user1);
+      await ExternalUserGroupRelation.create({ relatedGroup: groupId3, relatedUser: user2._id });
+    });
+
+    it('finds all unique user ids for specified user groups', async() => {
+      const userIds = await ExternalUserGroupRelation.findAllUserIdsForUserGroups([groupId1, groupId2, groupId3]);
+      expect(userIds).toStrictEqual([userId1.toString(), user2._id.toString()]);
+    });
+  });
 });

apps/app/src/features/external-user-group/server/models/external-user-group-relation.ts

@@ -36,4 +36,6 @@ schema.statics.findGroupsWithDescendantsByGroupAndUser = UserGroupRelation.findG
 
 schema.statics.countByGroupIdAndUser = UserGroupRelation.countByGroupIdAndUser;
 
+schema.statics.findAllUserIdsForUserGroups = UserGroupRelation.findAllUserIdsForUserGroups;
+
 export default getOrCreateModel<ExternalUserGroupRelationDocument, ExternalUserGroupRelationModel>('ExternalUserGroupRelation', schema);

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

@@ -252,7 +252,7 @@ export const getPageSchema = (crowi) => {
   pageSchema.methods.applyScope = function(user, grant, grantUserGroupIds) {
     // Reset
     this.grantedUsers = [];
-    this.grantedGroup = null;
+    this.grantedGroups = [];
 
     this.grant = grant || GRANT_PUBLIC;
 

apps/app/src/server/models/user-group-relation.ts

@@ -1,6 +1,7 @@
 import { IUserGroupRelation } from '@growi/core';
 import mongoose, { Model, Schema, Document } from 'mongoose';
 
+import { ObjectIdLike } from '../interfaces/mongoose-utils';
 import { getOrCreateModel } from '../util/mongoose-utils';
 
 import { UserGroupDocument } from './user-group';
@@ -81,13 +82,14 @@ schema.statics.findAllRelationForUserGroup = function(userGroup) {
     .exec();
 };
 
-schema.statics.findAllUserIdsForUserGroup = async function(userGroup) {
+schema.statics.findAllUserIdsForUserGroups = async function(userGroupIds: ObjectIdLike[]): Promise<string[]> {
   const relations = await this
-    .find({ relatedGroup: userGroup })
+    .find({ relatedGroup: { $in: userGroupIds } })
     .select('relatedUser')
     .exec();
 
-  return relations.map(r => r.relatedUser);
+  // return unique ids
+  return [...new Set(relations.map(r => r.relatedUser.toString()))];
 };
 
 /**

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

@@ -54,7 +54,6 @@ type UpdateGrantInfo = {
 } | {
   grant: typeof PageGrant.GRANT_USER_GROUP,
   grantedUserGroupInfo: {
-    groupId: ObjectIdLike,
     userIds: Set<ObjectIdLike>,
     childrenOrItselfGroupIds: Set<ObjectIdLike>,
   },
@@ -63,7 +62,7 @@ type UpdateGrantInfo = {
 type DescendantPagesGrantInfo = {
   grantSet: Set<number>,
   grantedUserIds: Set<ObjectIdLike>, // all only me users of descendant pages
-  grantedUserGroupIds: Set<GrantedGroup>, // all user groups of descendant pages
+  grantedUserGroupIds: Set<ObjectIdLike>, // all user groups of descendant pages
 };
 
 /**
@@ -585,13 +584,15 @@ class PageGrantService {
     const descendantPagesGrantInfo = {
       grantSet,
       grantedUserIds: new Set(comparableDescendants.grantedUserIds), // all only me users of descendant pages
-      grantedUserGroupIds: new Set(comparableDescendants.grantedGroupIds), // all user groups of descendant pages
+      grantedUserGroupIds: new Set(comparableDescendants.grantedGroupIds.map(g => g.item)), // all user groups of descendant pages
     };
 
     return this.calcCanOverwriteDescendants(operatorGrantInfo, updateGrantInfo, descendantPagesGrantInfo);
   }
 
-  async generateUpdateGrantInfoToOverwriteDescendants(operator, updateGrant: PageGrantCanBeOnTree, grantUserGroupId?: ObjectIdLike): Promise<UpdateGrantInfo> {
+  async generateUpdateGrantInfoToOverwriteDescendants(
+      operator, updateGrant: PageGrantCanBeOnTree, grantGroupIds?: GrantedGroup[],
+  ): Promise<UpdateGrantInfo> {
     let updateGrantInfo: UpdateGrantInfo | null = null;
 
     if (updateGrant === PageGrant.GRANT_PUBLIC) {
@@ -606,18 +607,28 @@ class PageGrantService {
       };
     }
     else if (updateGrant === PageGrant.GRANT_USER_GROUP) {
-      if (grantUserGroupId == null) {
-        throw Error('The parameter `grantUserGroupId` is required.');
+      if (grantGroupIds == null) {
+        throw Error('The parameter `grantGroupIds` is required.');
       }
       const UserGroupRelation = mongoose.model('UserGroupRelation') as any; // TODO: Typescriptize model
-      const userIds = await UserGroupRelation.findAllUserIdsForUserGroup(grantUserGroupId);
-      const childrenOrItselfGroups = await UserGroup.findGroupsWithDescendantsById(grantUserGroupId);
+      const { grantedUserGroups: grantedUserGroupIds, grantedExternalUserGroups: grantedExternalUserGroupIds } = divideByType(grantGroupIds);
+
+      const userGroupUserIds = await UserGroupRelation.findAllUserIdsForUserGroups(grantedUserGroupIds);
+      const externalUserGroupUserIds = await ExternalUserGroupRelation.findAllUserIdsForUserGroups(grantedExternalUserGroupIds);
+      const userIds = [...userGroupUserIds, ...externalUserGroupUserIds];
+
+      const childrenOrItselfUserGroups = (await Promise.all(grantedUserGroupIds.map((groupId) => {
+        return UserGroup.findGroupsWithDescendantsById(groupId);
+      }))).flat();
+      const childrenOrItselfExternalUserGroups = (await Promise.all(externalUserGroupUserIds.map((groupId) => {
+        return ExternalUserGroup.findGroupsWithDescendantsById(groupId);
+      }))).flat();
+      const childrenOrItselfGroups = [...childrenOrItselfUserGroups, ...childrenOrItselfExternalUserGroups];
       const childrenOrItselfGroupIds = childrenOrItselfGroups.map(d => d._id);
 
       updateGrantInfo = {
         grant: PageGrant.GRANT_USER_GROUP,
         grantedUserGroupInfo: {
-          groupId: grantUserGroupId,
           userIds: new Set<ObjectIdLike>(userIds),
           childrenOrItselfGroupIds: new Set<ObjectIdLike>(childrenOrItselfGroupIds),
         },

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

@@ -3867,7 +3867,7 @@ class PageService {
     const newPageData = pageData;
 
     const grant = options.grant ?? clonedPageData.grant; // use the previous data if absence
-    const grantUserGroupId: undefined | ObjectIdLike = options.grantUserGroupIds ?? clonedPageData.grantedGroup?._id.toString();
+    const grantUserGroupIds: undefined | GrantedGroup[] = options.grantUserGroupIds ?? clonedPageData.grantedGroups;
 
     const grantedUserIds = clonedPageData.grantedUserIds || [user._id];
     const shouldBeOnTree = grant !== PageGrant.GRANT_RESTRICTED;
@@ -3880,7 +3880,7 @@ class PageService {
       try {
         const shouldCheckDescendants = !options.overwriteScopesOfDescendants;
         // eslint-disable-next-line max-len
-        isGrantNormalized = await pageGrantService.isGrantNormalized(user, clonedPageData.path, grant, grantedUserIds, grantUserGroupId, shouldCheckDescendants);
+        isGrantNormalized = await pageGrantService.isGrantNormalized(user, clonedPageData.path, grant, grantedUserIds, grantUserGroupIds, shouldCheckDescendants);
       }
       catch (err) {
         logger.error(`Failed to validate grant of page at "${clonedPageData.path}" of grant ${grant}:`, err);
@@ -3918,7 +3918,7 @@ class PageService {
       newPageData.descendantCount = 0;
     }
 
-    newPageData.applyScope(user, grant, grantUserGroupId);
+    newPageData.applyScope(user, grant, grantUserGroupIds);
 
     // update existing page
     let savedPage = await newPageData.save();

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

@@ -805,7 +805,7 @@ describe('Page', () => {
         expect(page1).toBeTruthy();
         expect(page2).toBeTruthy();
 
-        const options = { grant: Page.GRANT_RESTRICTED, grantUserGroupId: null };
+        const options = { grant: Page.GRANT_RESTRICTED, grantUserGroupIds: null };
         await updatePage(page2, 'newRevisionBody', 'oldRevisionBody', dummyUser1, options);
 
         const _pageT = await Page.findOne({ path: pathT });
@@ -963,7 +963,7 @@ describe('Page', () => {
         const page = await Page.findOne({ path });
         expect(page.grant).toBe(Page.GRANT_OWNER);
         expect(page.grantedUsers).toStrictEqual([pModelUser1._id]);
-        expect(page.grantedGroup).toBeNull();
+        expect(page.grantedGroups.length).toBe(0);
       });
       test('successfully change to GRANT_OWNER from GRANT_RESTRICTED', async() => {
         const path = '/mup21';
@@ -1005,7 +1005,7 @@ describe('Page', () => {
           expect(_page1).toBeTruthy();
           expect(_page2).toBeTruthy();
 
-          const options = { grant: Page.GRANT_USER_GROUP, grantUserGroupId: userGroupIdPModelA };
+          const options = { grant: Page.GRANT_USER_GROUP, grantUserGroupIds: [{ item: userGroupIdPModelA, type: 'UserGroup' }] };
           const updatedPage = await updatePage(_page2, 'new', 'old', pModelUser1, options); // from GRANT_PUBLIC to GRANT_USER_GROUP(userGroupIdPModelA)
 
           const page1 = await Page.findById(_page1._id);
@@ -1017,7 +1017,7 @@ describe('Page', () => {
 
           // check page2 grant and group
           expect(page2.grant).toBe(Page.GRANT_USER_GROUP);
-          expect(page2.grantedGroup._id).toStrictEqual(userGroupIdPModelA);
+          expect(page2.grantedGroups.map(g => g.item)).toStrictEqual([userGroupIdPModelA]);
         });
 
         test('successfully change to GRANT_USER_GROUP from GRANT_RESTRICTED if parent page is GRANT_PUBLIC', async() => {
@@ -1027,7 +1027,7 @@ describe('Page', () => {
           const _page1 = await Page.findOne({ path: _path1, grant: Page.GRANT_RESTRICTED });
           expect(_page1).toBeTruthy();
 
-          const options = { grant: Page.GRANT_USER_GROUP, grantUserGroupId: userGroupIdPModelA };
+          const options = { grant: Page.GRANT_USER_GROUP, grantUserGroupIds: [{ item: userGroupIdPModelA, type: 'UserGroup' }] };
           const updatedPage = await updatePage(_page1, 'new', 'old', pModelUser1, options); // from GRANT_RESTRICTED to GRANT_USER_GROUP(userGroupIdPModelA)
 
           const page1 = await Page.findById(_page1._id);
@@ -1037,7 +1037,7 @@ describe('Page', () => {
 
           // updated page
           expect(page1.grant).toBe(Page.GRANT_USER_GROUP);
-          expect(page1.grantedGroup._id).toStrictEqual(userGroupIdPModelA);
+          expect(page1.grantedGroups.map(g => g.item)).toStrictEqual([userGroupIdPModelA]);
 
           // parent's grant check
           const parent = await Page.findById(page1.parent);
@@ -1057,7 +1057,7 @@ describe('Page', () => {
           expect(_page1).toBeTruthy();
           expect(_page2).toBeTruthy();
 
-          const options = { grant: Page.GRANT_USER_GROUP, grantUserGroupId: userGroupIdPModelA };
+          const options = { grant: Page.GRANT_USER_GROUP, grantUserGroupIds: [{ item: userGroupIdPModelA, type: 'UserGroup' }] };
           const updatedPage = await updatePage(_page2, 'new', 'old', pModelUser1, options); // from GRANT_OWNER to GRANT_USER_GROUP(userGroupIdPModelA)
 
           const page1 = await Page.findById(_page1._id);
@@ -1069,12 +1069,12 @@ describe('Page', () => {
 
           // grant check
           expect(page2.grant).toBe(Page.GRANT_USER_GROUP);
-          expect(page2.grantedGroup._id).toStrictEqual(userGroupIdPModelA);
+          expect(page2.grantedGroups.map(g => g.item)).toStrictEqual([userGroupIdPModelA]);
           expect(page2.grantedUsers.length).toBe(0);
         });
       });
       describe('update grant of a page under a page with GRANT_USER_GROUP', () => {
-        test('successfully change to GRANT_USER_GROUP if the group to set is the child or descendant of the parent page group', async() => {
+        test.only('successfully change to GRANT_USER_GROUP if the group to set is the child or descendant of the parent page group', async() => {
           // path
           const _path1 = '/mup29_A';
           const _path2 = '/mup29_A/mup30_owner';
@@ -1086,7 +1086,7 @@ describe('Page', () => {
           expect(_page1).toBeTruthy();
           expect(_page2).toBeTruthy();
 
-          const options = { grant: Page.GRANT_USER_GROUP, grantUserGroupId: userGroupIdPModelB };
+          const options = { grant: Page.GRANT_USER_GROUP, grantUserGroupIds: [{ item: userGroupIdPModelB, type: 'UserGroup' }] };
 
           // First round
           // Group relation(parent -> child): userGroupIdPModelA -> userGroupIdPModelB -> userGroupIdPModelC
@@ -1100,17 +1100,19 @@ describe('Page', () => {
           expect(updatedPage._id).toStrictEqual(page2._id);
 
           expect(page2.grant).toBe(Page.GRANT_USER_GROUP);
-          expect(page2.grantedGroup._id).toStrictEqual(userGroupIdPModelB);
+          expect(page2.grantedGroups.map(g => g.item)).toStrictEqual([userGroupIdPModelB]);
           expect(page2.grantedUsers.length).toBe(0);
 
           // Second round
           // Update group to groupC which is a grandchild from pageA's point of view
-          const secondRoundOptions = { grant: Page.GRANT_USER_GROUP, grantUserGroupId: userGroupIdPModelC }; // from GRANT_USER_GROUP(userGroupIdPModelB) to GRANT_USER_GROUP(userGroupIdPModelC)
+          const secondRoundOptions = { grant: Page.GRANT_USER_GROUP, grantUserGroupIds: [{ item: userGroupIdPModelC, type: 'UserGroup' }] }; // from GRANT_USER_GROUP(userGroupIdPModelB) to GRANT_USER_GROUP(userGroupIdPModelC)
+          console.log('い');
+          console.log(_page2);
           const secondRoundUpdatedPage = await updatePage(_page2, 'new', 'new', pModelUser3, secondRoundOptions);
 
           expect(secondRoundUpdatedPage).toBeTruthy();
           expect(secondRoundUpdatedPage.grant).toBe(Page.GRANT_USER_GROUP);
-          expect(secondRoundUpdatedPage.grantedGroup._id).toStrictEqual(userGroupIdPModelC);
+          expect(secondRoundUpdatedPage.grantedGroups.map(g => g.item)).toStrictEqual([userGroupIdPModelC]);
         });
         test('Fail to change to GRANT_USER_GROUP if the group to set is NOT the child or descendant of the parent page group', async() => {
           // path
@@ -1130,7 +1132,7 @@ describe('Page', () => {
           // group parent check
           expect(_groupIsolated.parent).toBeUndefined(); // should have no parent
 
-          const options = { grant: Page.GRANT_USER_GROUP, grantUserGroupId: userGroupIdPModelIsolate };
+          const options = { grant: Page.GRANT_USER_GROUP, grantUserGroupIds: [{ item: userGroupIdPModelIsolate, type: 'UserGroup' }] };
           await expect(updatePage(_page2, 'new', 'old', pModelUser1, options)) // from GRANT_OWNER to GRANT_USER_GROUP(userGroupIdPModelIsolate)
             .rejects.toThrow(new Error('The selected grant or grantedGroup is not assignable to this page.'));
 
@@ -1141,7 +1143,7 @@ describe('Page', () => {
 
           expect(page2.grant).toBe(Page.GRANT_OWNER); // should be the same before the update
           expect(page2.grantedUsers).toStrictEqual([pModelUser1._id]); // should be the same before the update
-          expect(page2.grantedGroup).toBeUndefined(); // no group should be set
+          expect(page2.grantedGroups.length).toBe(0); // no group should be set
         });
         test('Fail to change to GRANT_USER_GROUP if the group to set is an ancestor of the parent page group', async() => {
           // path
@@ -1155,7 +1157,7 @@ describe('Page', () => {
           expect(_page1).toBeTruthy();
           expect(_page2).toBeTruthy();
 
-          const options = { grant: Page.GRANT_USER_GROUP, grantUserGroupId: userGroupIdPModelA };
+          const options = { grant: Page.GRANT_USER_GROUP, grantUserGroupIds: [{ item: userGroupIdPModelA, type: 'UserGroup' }] };
 
           // Group relation(parent -> child): userGroupIdPModelA -> userGroupIdPModelB -> userGroupIdPModelC
           // this should fail because the groupC is a descendant of groupA
@@ -1169,7 +1171,7 @@ describe('Page', () => {
 
           expect(page2.grant).toBe(Page.GRANT_OWNER); // should be the same before the update
           expect(page2.grantedUsers).toStrictEqual([pModelUser3._id]); // should be the same before the update
-          expect(page2.grantedGroup).toBeUndefined(); // no group should be set
+          expect(page2.grantedGroups.length).toBe(0); // no group should be set
         });
       });
       describe('update grant of a page under a page with GRANT_OWNER', () => {
@@ -1185,7 +1187,7 @@ describe('Page', () => {
           expect(_page1).toBeTruthy();
           expect(_page2).toBeTruthy();
 
-          const options = { grant: Page.GRANT_USER_GROUP, grantUserGroupId: userGroupIdPModelA };
+          const options = { grant: Page.GRANT_USER_GROUP, grantUserGroupIds: [{ item: userGroupIdPModelA, type: 'UserGroup' }] };
           await expect(updatePage(_page2, 'new', 'old', pModelUser1, options)) // from GRANT_OWNER to GRANT_USER_GROUP(userGroupIdPModelA)
             .rejects.toThrow(new Error('The selected grant or grantedGroup is not assignable to this page.'));
 
@@ -1195,7 +1197,7 @@ describe('Page', () => {
           expect(page2).toBeTruthy();
           expect(page2.grant).toBe(Page.GRANT_OWNER); // should be the same before the update
           expect(page2.grantedUsers).toStrictEqual([pModelUser1._id]); // should be the same before the update
-          expect(page2.grantedGroup).toBeUndefined(); // no group should be set
+          expect(page2.grantedGroups.length).toBe(0); // no group should be set
         });
       });
 
@@ -1234,7 +1236,7 @@ describe('Page', () => {
       expect(updatedPage.grant).toBe(newGrant);
       // Not changed
       expect(upodPagegBUpdated.grant).toBe(PageGrant.GRANT_USER_GROUP);
-      expect(upodPagegBUpdated.grantedGroup).toStrictEqual(upodPagegB.grantedGroup);
+      expect(upodPagegBUpdated.grantedGroups).toStrictEqual(upodPagegB.grantedGroups);
       expect(upodPageonlyBUpdated.grant).toBe(PageGrant.GRANT_OWNER);
       expect(upodPageonlyBUpdated.grantedUsers).toStrictEqual(upodPageonlyB.grantedUsers);
     });
@@ -1298,7 +1300,7 @@ describe('Page', () => {
       // Update
       const options = {
         grant: PageGrant.GRANT_USER_GROUP,
-        grantUserGroupId: upodUserGroupIdAB,
+        grantUserGroupIds: [{ item: upodUserGroupIdAB, type: 'UserGroup' }],
         overwriteScopesOfDescendants: true,
       };
       const updatedPage = await updatePage(upodPagePublic, 'newRevisionBody', 'oldRevisionBody', upodUserA, options);
@@ -1311,12 +1313,12 @@ describe('Page', () => {
       const newGrant = PageGrant.GRANT_USER_GROUP;
       const newGrantedGroup = upodUserGroupIdAB;
       expect(updatedPage.grant).toBe(newGrant);
-      expect(updatedPage.grantedGroup._id).toStrictEqual(newGrantedGroup);
+      expect(updatedPage.grantedGroups.map(g => g.item._id)).toStrictEqual([newGrantedGroup]);
       expect(upodPagegABUpdated.grant).toBe(newGrant);
-      expect(upodPagegABUpdated.grantedGroup._id).toStrictEqual(newGrantedGroup);
+      expect(upodPagegABUpdated.grantedGroups.map(g => g.item)).toStrictEqual([newGrantedGroup]);
       // Not changed
       expect(upodPagegBUpdated.grant).toBe(PageGrant.GRANT_USER_GROUP);
-      expect(upodPagegBUpdated.grantedGroup._id).toStrictEqual(upodPagegB.grantedGroup);
+      expect(upodPagegBUpdated.grantedGroups).toStrictEqual(upodPagegB.grantedGroups);
       expect(upodPageonlyBUpdated.grant).toBe(PageGrant.GRANT_OWNER);
       expect(upodPageonlyBUpdated.grantedUsers).toStrictEqual(upodPageonlyB.grantedUsers);
     });
@@ -1338,7 +1340,7 @@ describe('Page', () => {
       // Update
       const options = {
         grant: PageGrant.GRANT_USER_GROUP,
-        grantUserGroupId: upodUserGroupIdAB,
+        grantUserGroupIds: [{ item: upodUserGroupIdAB, type: 'UserGroup' }],
         overwriteScopesOfDescendants: true,
       };
       const updatedPagePromise = updatePage(upodPagePublic, 'newRevisionBody', 'oldRevisionBody', upodUserA, options);
@@ -1363,7 +1365,7 @@ describe('Page', () => {
       // Update
       const options = {
         grant: PageGrant.GRANT_USER_GROUP,
-        grantUserGroupId: upodUserGroupIdAB,
+        grantUserGroupIds: [{ item: upodUserGroupIdAB, type: 'UserGroup' }],
         overwriteScopesOfDescendants: true,
       };
       const updatedPagePromise = updatePage(upodPagePublic, 'newRevisionBody', 'oldRevisionBody', upodUserA, options);
@@ -1383,7 +1385,7 @@ describe('Page', () => {
       // Update
       const options = {
         grant: PageGrant.GRANT_USER_GROUP,
-        grantUserGroupId: upodUserGroupIdAB,
+        grantUserGroupIds: [{ item: upodUserGroupIdAB, type: 'UserGroup' }],
         overwriteScopesOfDescendants: true,
       };
       const updatedPagePromise = updatePage(upodPagePublic, 'newRevisionBody', 'oldRevisionBody', upodUserA, options);