Futa Arai 2 лет назад
Родитель
Сommit
70948ebc48

+ 1 - 1
apps/app/src/migrations/20230723061824-granted-group-to-array-of-objects.js

@@ -1,6 +1,6 @@
 import loggerFactory from '~/utils/logger';
 
-const logger = loggerFactory('growi:remove-basic-auth-related-config');
+const logger = loggerFactory('growi:granted-group-to-array-of-objects');
 
 module.exports = {
   async up(db, client) {

+ 28 - 0
apps/app/src/migrations/20231223155127-non-null-granted-groups.js

@@ -0,0 +1,28 @@
+import loggerFactory from '~/utils/logger';
+
+const logger = loggerFactory('growi:non-null-granted-groups');
+
+module.exports = {
+  async up(db, client) {
+    logger.info('Apply migration');
+
+    const pageCollection = await db.collection('pages');
+
+    await pageCollection.updateMany(
+      { grantedGroups: { $eq: null } },
+      [
+        {
+          $set: {
+            grantedGroups: [],
+          },
+        },
+      ],
+    );
+
+    logger.info('Migration has successfully applied');
+  },
+
+  async down(db, client) {
+    // No rollback
+  },
+};

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

@@ -675,7 +675,7 @@ export const getPageSchema = (crowi) => {
         updateOne: {
           filter: { _id: page._id },
           update: {
-            grantedGroups: null,
+            grantedGroups: [],
             grant: this.GRANT_PUBLIC,
           },
         },

+ 1 - 0
apps/app/src/server/models/page.ts

@@ -126,6 +126,7 @@ const schema = new Schema<PageDocument, PageModel>({
       return arr.length === uniqueItemValues.size;
     }, 'grantedGroups contains non unique item'],
     default: [],
+    required: true,
   },
   creator: { type: ObjectId, ref: 'User', index: true },
   lastUpdateUser: { type: ObjectId, ref: 'User' },

+ 3 - 2
apps/app/src/server/service/page.ts

@@ -4099,7 +4099,7 @@ class PageService {
     const clonedPageData = Page.hydrate(pageData.toObject());
     const newPageData = pageData;
 
-    // use the previous data if absence
+    // use the previous data if absent
     const grant = options.grant ?? clonedPageData.grant;
     const grantUserGroupIds = options.userRelatedGrantUserGroupIds != null
       ? (await this.getNewGrantedGroups(options.userRelatedGrantUserGroupIds, clonedPageData, user))
@@ -4214,7 +4214,8 @@ class PageService {
     const Page = mongoose.model('Page') as unknown as PageModel;
     const Revision = mongoose.model('Revision') as any; // TODO: TypeScriptize model
 
-    const grant = options.grant || pageData.grant; // use the previous data if absence
+    // use the previous data if absent
+    const grant = options.grant || pageData.grant;
     const grantUserGroupIds = options.userRelatedGrantUserGroupIds != null
       ? (await this.getNewGrantedGroups(options.userRelatedGrantUserGroupIds, pageData, user))
       : pageData.grantedGroups;

+ 24 - 24
apps/app/test/integration/models/v5.page.test.js

@@ -255,7 +255,7 @@ describe('Page', () => {
         creator: upodUserB,
         lastUpdateUser: upodUserB,
         grantedUsers: [upodUserB._id],
-        grantedGroups: null,
+        grantedGroups: [],
         parent: upodPageIdgAB1,
       },
       // case 2
@@ -266,7 +266,7 @@ describe('Page', () => {
         creator: upodUserA,
         lastUpdateUser: upodUserA,
         grantedUsers: null,
-        grantedGroups: null,
+        grantedGroups: [],
         parent: rootPage._id,
       },
       {
@@ -299,7 +299,7 @@ describe('Page', () => {
         creator: upodUserA,
         lastUpdateUser: upodUserA,
         grantedUsers: [upodUserA._id],
-        grantedGroups: null,
+        grantedGroups: [],
         parent: upodPageIdPublic2,
       },
       // case 3
@@ -310,7 +310,7 @@ describe('Page', () => {
         creator: upodUserA,
         lastUpdateUser: upodUserA,
         grantedUsers: null,
-        grantedGroups: null,
+        grantedGroups: [],
         parent: rootPage._id,
       },
       {
@@ -343,7 +343,7 @@ describe('Page', () => {
         creator: upodUserB,
         lastUpdateUser: upodUserB,
         grantedUsers: [upodUserB._id],
-        grantedGroups: null,
+        grantedGroups: [],
         parent: upodPageIdPublic3,
       },
       // case 4
@@ -354,7 +354,7 @@ describe('Page', () => {
         creator: upodUserA,
         lastUpdateUser: upodUserA,
         grantedUsers: null,
-        grantedGroups: null,
+        grantedGroups: [],
         parent: rootPage._id,
       },
       {
@@ -389,7 +389,7 @@ describe('Page', () => {
         creator: upodUserA,
         lastUpdateUser: upodUserA,
         grantedUsers: null,
-        grantedGroups: null,
+        grantedGroups: [],
         parent: rootPage._id,
       },
       {
@@ -410,7 +410,7 @@ describe('Page', () => {
         creator: upodUserC,
         lastUpdateUser: upodUserC,
         grantedUsers: [upodUserC._id],
-        grantedGroups: null,
+        grantedGroups: [],
         parent: upodPageIdPublic5,
       },
       // case 6
@@ -421,7 +421,7 @@ describe('Page', () => {
         creator: upodUserA,
         lastUpdateUser: upodUserA,
         grantedUsers: null,
-        grantedGroups: null,
+        grantedGroups: [],
         parent: rootPage._id,
       },
       {
@@ -430,7 +430,7 @@ describe('Page', () => {
         creator: upodUserC,
         lastUpdateUser: upodUserC,
         grantedUsers: [upodUserC._id],
-        grantedGroups: null,
+        grantedGroups: [],
         parent: upodPageIdPublic6,
       },
     ]);
@@ -1014,7 +1014,7 @@ describe('Page', () => {
         expect(page1).toBeTruthy();
         expect(page2).toBeTruthy();
 
-        const options = { grant: Page.GRANT_RESTRICTED, grantUserGroupIds: null };
+        const options = { grant: Page.GRANT_RESTRICTED, userRelatedGrantUserGroupIds: null };
         await updatePage(page2, 'newRevisionBody', 'oldRevisionBody', dummyUser1, options);
 
         const _pageT = await Page.findOne({ path: pathT });
@@ -1221,7 +1221,7 @@ describe('Page', () => {
 
           const options = {
             grant: Page.GRANT_USER_GROUP,
-            grantUserGroupIds: newGrantedGroups,
+            userRelatedGrantUserGroupIds: newGrantedGroups,
           };
           const updatedPage = await updatePage(_page2, 'new', 'old', pModelUser1, options); // from GRANT_PUBLIC to GRANT_USER_GROUP(userGroupIdPModelA)
 
@@ -1251,7 +1251,7 @@ describe('Page', () => {
 
           const options = {
             grant: Page.GRANT_USER_GROUP,
-            grantUserGroupIds: newGrantedGroups,
+            userRelatedGrantUserGroupIds: newGrantedGroups,
           };
           const updatedPage = await updatePage(_page1, 'new', 'old', pModelUser1, options); // from GRANT_RESTRICTED to GRANT_USER_GROUP(userGroupIdPModelA)
 
@@ -1289,7 +1289,7 @@ describe('Page', () => {
 
           const options = {
             grant: Page.GRANT_USER_GROUP,
-            grantUserGroupIds: newGrantedGroups,
+            userRelatedGrantUserGroupIds: newGrantedGroups,
           };
           const updatedPage = await updatePage(_page2, 'new', 'old', pModelUser1, options); // from GRANT_OWNER to GRANT_USER_GROUP(userGroupIdPModelA)
 
@@ -1327,7 +1327,7 @@ describe('Page', () => {
           ];
           const options = {
             grant: Page.GRANT_USER_GROUP,
-            grantUserGroupIds: newGrantedGroups,
+            userRelatedGrantUserGroupIds: newGrantedGroups,
           };
           const updatedPage = await updatePage(_page2, 'new', 'old', pModelUser3, options); // from GRANT_OWNER to GRANT_USER_GROUP(userGroupIdPModelB)
 
@@ -1348,7 +1348,7 @@ describe('Page', () => {
             { item: userGroupIdPModelC, type: GroupType.userGroup },
             { item: externalUserGroupIdPModelC, type: GroupType.externalUserGroup },
           ];
-          const secondRoundOptions = { grant: Page.GRANT_USER_GROUP, grantUserGroupIds: secondRoundNewGrantedGroups }; // from GRANT_USER_GROUP(userGroupIdPModelB) to GRANT_USER_GROUP(userGroupIdPModelC)
+          const secondRoundOptions = { grant: Page.GRANT_USER_GROUP, userRelatedGrantUserGroupIds: secondRoundNewGrantedGroups }; // from GRANT_USER_GROUP(userGroupIdPModelB) to GRANT_USER_GROUP(userGroupIdPModelC)
           // undo grantedGroups populate to prevent Page.hydrate error
           _page2.grantedGroups.forEach((group) => {
             group.item = group.item._id;
@@ -1379,7 +1379,7 @@ describe('Page', () => {
 
           const options = {
             grant: Page.GRANT_USER_GROUP,
-            grantUserGroupIds: [
+            userRelatedGrantUserGroupIds: [
               { item: userGroupIdPModelIsolate, type: GroupType.userGroup },
               { item: externalUserGroupIdPModelIsolate, type: GroupType.externalUserGroup },
             ],
@@ -1410,7 +1410,7 @@ describe('Page', () => {
 
           const options = {
             grant: Page.GRANT_USER_GROUP,
-            grantUserGroupIds: [
+            userRelatedGrantUserGroupIds: [
               { item: userGroupIdPModelA, type: GroupType.userGroup },
               { item: externalUserGroupIdPModelA, type: GroupType.externalUserGroup },
             ],
@@ -1444,7 +1444,7 @@ describe('Page', () => {
           expect(_page1).toBeTruthy();
           expect(_page2).toBeTruthy();
 
-          const options = { grant: Page.GRANT_USER_GROUP, grantUserGroupIds: [{ item: userGroupIdPModelA, type: GroupType.userGroup }] };
+          const options = { grant: Page.GRANT_USER_GROUP, userRelatedGrantUserGroupIds: [{ item: userGroupIdPModelA, type: GroupType.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.'));
 
@@ -1536,7 +1536,7 @@ describe('Page', () => {
       expect(upodPageonlyAUpdated.grant).toBe(newGrant);
       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' });
@@ -1557,7 +1557,7 @@ describe('Page', () => {
       // Update
       const options = {
         grant: PageGrant.GRANT_USER_GROUP,
-        grantUserGroupIds: [
+        userRelatedGrantUserGroupIds: [
           { item: upodUserGroupIdAB, type: GroupType.userGroup },
           { item: upodExternalUserGroupIdAB, type: GroupType.externalUserGroup },
         ],
@@ -1603,7 +1603,7 @@ describe('Page', () => {
       // Update
       const options = {
         grant: PageGrant.GRANT_USER_GROUP,
-        grantUserGroupIds: [
+        userRelatedGrantUserGroupIds: [
           { item: upodUserGroupIdAB, type: GroupType.userGroup },
           { item: upodExternalUserGroupIdAB, type: GroupType.externalUserGroup },
         ],
@@ -1631,7 +1631,7 @@ describe('Page', () => {
       // Update
       const options = {
         grant: PageGrant.GRANT_USER_GROUP,
-        grantUserGroupIds: [
+        userRelatedGrantUserGroupIds: [
           { item: upodUserGroupIdAB, type: GroupType.userGroup },
           { item: upodExternalUserGroupIdAB, type: GroupType.externalUserGroup },
         ],
@@ -1654,7 +1654,7 @@ describe('Page', () => {
       // Update
       const options = {
         grant: PageGrant.GRANT_USER_GROUP,
-        grantUserGroupIds: [
+        userRelatedGrantUserGroupIds: [
           { item: upodUserGroupIdAB, type: GroupType.userGroup },
           { item: upodExternalUserGroupIdAB, type: GroupType.externalUserGroup },
         ],

+ 4 - 4
apps/app/test/integration/service/page-grant.test.js

@@ -199,7 +199,7 @@ describe('PageGrantService', () => {
         creator: user1,
         lastUpdateUser: user1,
         grantedUsers: null,
-        grantedGroups: null,
+        grantedGroups: [],
         parent: rootPage._id,
       },
       {
@@ -328,7 +328,7 @@ describe('PageGrantService', () => {
         creator: user1,
         lastUpdateUser: user1,
         grantedUsers: null,
-        grantedGroups: null,
+        grantedGroups: [],
         parent: emptyPage1._id,
       },
       {
@@ -337,7 +337,7 @@ describe('PageGrantService', () => {
         creator: user1,
         lastUpdateUser: user1,
         grantedUsers: [user1._id],
-        grantedGroups: null,
+        grantedGroups: [],
         parent: emptyPage2._id,
       },
       {
@@ -364,7 +364,7 @@ describe('PageGrantService', () => {
         creator: user1,
         lastUpdateUser: user1,
         grantedUsers: [user1._id],
-        grantedGroups: null,
+        grantedGroups: [],
         parent: emptyPage3._id,
       },
     ]);

+ 4 - 4
apps/app/test/integration/service/v5.page.test.ts

@@ -370,7 +370,7 @@ describe('Test page service methods', () => {
           status: 'published',
           grant: 1,
           grantedUsers: [],
-          grantedGroups: null,
+          grantedGroups: [],
           creator: dummyUser1._id,
           lastUpdateUser: dummyUser1._id,
         },
@@ -399,7 +399,7 @@ describe('Test page service methods', () => {
           status: 'published',
           grant: 1,
           grantedUsers: [],
-          grantedGroups: null,
+          grantedGroups: [],
           creator: dummyUser1._id,
           lastUpdateUser: dummyUser1._id,
         },
@@ -428,7 +428,7 @@ describe('Test page service methods', () => {
           status: 'published',
           grant: 1,
           grantedUsers: [],
-          grantedGroups: null,
+          grantedGroups: [],
           creator: dummyUser1._id,
           lastUpdateUser: dummyUser1._id,
         },
@@ -457,7 +457,7 @@ describe('Test page service methods', () => {
           status: 'published',
           grant: Page.GRANT_PUBLIC,
           grantedUsers: [],
-          grantedGroups: null,
+          grantedGroups: [],
           creator: dummyUser1._id,
           lastUpdateUser: dummyUser1._id,
         },

+ 1 - 1
apps/app/test/integration/service/v5.public-page.test.ts

@@ -453,7 +453,7 @@ describe('PageService page operations with only public pages', () => {
           status: 'published',
           grant: 1,
           grantedUsers: [],
-          grantedGroups: null,
+          grantedGroups: [],
           creator: dummyUser1._id,
           lastUpdateUser: dummyUser1._id,
         },