Merge branch 'fix-page-service-and-page-grant-service' into fix-page-model-tests

apps/app/src/server/models/interfaces/page-operation.ts

@@ -1,4 +1,4 @@
-import { GroupType } from '@growi/core';
+import { GrantedGroup } from '@growi/core';
 
 import { PageGrant } from '~/interfaces/page';
 
@@ -11,10 +11,7 @@ export type IPageForResuming = {
   parent?: ObjectIdLike,
   grant?: number,
   grantedUsers?: ObjectIdLike[],
-  grantedGroups: {
-    type: GroupType,
-    item: ObjectIdLike,
-  }[],
+  grantedGroups: GrantedGroup[],
   descendantCount: number,
   status?: number,
   revision?: ObjectIdLike,
@@ -28,20 +25,14 @@ export type IUserForResuming = {
 
 export type IOptionsForUpdate = {
   grant?: PageGrant,
-  grantUserGroupIds?: {
-    type: GroupType,
-    item: ObjectIdLike,
-  }[],
+  grantUserGroupIds?: GrantedGroup[],
   isSyncRevisionToHackmd?: boolean,
   overwriteScopesOfDescendants?: boolean,
 };
 
 export type IOptionsForCreate = {
   format?: string,
-  grantUserGroupIds?: {
-    type: GroupType,
-    item: ObjectIdLike,
-  }[],
+  grantUserGroupIds?: GrantedGroup[],
   grant?: PageGrant,
   overwriteScopesOfDescendants?: boolean,
   isSynchronously?: boolean,

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

@@ -3,6 +3,7 @@ import {
   pagePathUtils, pathUtils, pageUtils,
   PageGrant, PageGrantCanBeOnTree,
 } from '@growi/core';
+import { et } from 'date-fns/locale';
 import escapeStringRegexp from 'escape-string-regexp';
 import mongoose from 'mongoose';
 
@@ -11,7 +12,7 @@ import ExternalUserGroupRelation from '~/features/external-user-group/server/mod
 import { IRecordApplicableGrant } from '~/interfaces/page-grant';
 import { PageDocument, PageModel } from '~/server/models/page';
 import UserGroup from '~/server/models/user-group';
-import { isIncludesObjectId, excludeTestIdsFromTargetIds, hasIntersection } from '~/server/util/compare-objectId';
+import { includesObjectIds, excludeTestIdsFromTargetIds } from '~/server/util/compare-objectId';
 
 import { ObjectIdLike } from '../interfaces/mongoose-utils';
 import { divideByType } from '../util/granted-group';
@@ -140,7 +141,7 @@ class PageGrantService {
           throw Error('grantedUserIds must have one user');
         }
 
-        if (!isIncludesObjectId(ancestor.applicableUserIds, target.grantedUserIds[0])) { // GRANT_OWNER pages under GRAND_USER_GROUP page must be owned by the member of the grantedGroup of the GRAND_USER_GROUP page
+        if (!includesObjectIds(ancestor.applicableUserIds, [target.grantedUserIds[0]])) { // GRANT_OWNER pages under GRAND_USER_GROUP page must be owned by the member of the grantedGroup of the GRAND_USER_GROUP page
           return false;
         }
       }
@@ -149,8 +150,8 @@ class PageGrantService {
         if (target.grantedGroupIds == null || target.grantedGroupIds.length === 0) {
           throw Error('grantedGroupId must not be empty');
         }
-
-        if (!hasIntersection(ancestor.applicableGroupIds, target.grantedGroupIds.map(e => e.item))) { // only child groups or the same group can exist under GRANT_USER_GROUP page
+        const targetGrantedGroupStrIds = target.grantedGroupIds.map(e => (typeof e.item === 'string' ? e.item : e.item._id));
+        if (!includesObjectIds(ancestor.applicableGroupIds, targetGrantedGroupStrIds)) { // only child groups or the same group can exist under GRANT_USER_GROUP page
           return false;
         }
       }
@@ -233,7 +234,7 @@ class PageGrantService {
 
         const userGroupRelations = await UserGroupRelation.find({ relatedGroup: { $in: targetUserGroups.map(g => g._id) } });
         const externalUserGroupRelations = await ExternalUserGroupRelation.find({ relatedGroup: { $in: targetExternalUserGroups.map(g => g._id) } });
-        applicableUserIds = [...userGroupRelations, ...externalUserGroupRelations].map(u => u.relatedUser);
+        applicableUserIds = Array.from(new Set([...userGroupRelations, ...externalUserGroupRelations].map(u => u.relatedUser)));
 
         const applicableUserGroups = (await Promise.all(targetUserGroups.map((group) => {
           return UserGroup.findGroupsWithDescendantsById(group._id);
@@ -645,7 +646,7 @@ class PageGrantService {
   private calcIsAllDescendantsGrantedByOperator(operatorGrantInfo: OperatorGrantInfo, descendantPagesGrantInfo: DescendantPagesGrantInfo): boolean {
     if (descendantPagesGrantInfo.grantSet.has(PageGrant.GRANT_OWNER)) {
       const isNonApplicableOwnerExist = descendantPagesGrantInfo.grantedUserIds.size >= 2
-        || !isIncludesObjectId([...descendantPagesGrantInfo.grantedUserIds], operatorGrantInfo.userId);
+        || !includesObjectIds([...descendantPagesGrantInfo.grantedUserIds], [operatorGrantInfo.userId]);
       if (isNonApplicableOwnerExist) {
         return false;
       }

apps/app/src/server/service/user-group.ts

@@ -3,7 +3,7 @@ import { Model } from 'mongoose';
 import { IUser } from '~/interfaces/user';
 import { ObjectIdLike } from '~/server/interfaces/mongoose-utils';
 import UserGroup, { UserGroupDocument, UserGroupModel } from '~/server/models/user-group';
-import { excludeTestIdsFromTargetIds, isIncludesObjectId } from '~/server/util/compare-objectId';
+import { excludeTestIdsFromTargetIds, includesObjectIds } from '~/server/util/compare-objectId';
 import loggerFactory from '~/utils/logger';
 
 import UserGroupRelation, { UserGroupRelationDocument, UserGroupRelationModel } from '../models/user-group-relation';
@@ -78,7 +78,7 @@ class UserGroupService {
 
     // throw if parent was in self and its descendants
     const descendantsWithTarget = await UserGroup.findGroupsWithDescendantsRecursively([userGroup]);
-    if (isIncludesObjectId(descendantsWithTarget.map(d => d._id), parent._id)) {
+    if (includesObjectIds(descendantsWithTarget.map(d => d._id), [parent._id])) {
       throw Error('It is not allowed to choose parent from descendant groups.');
     }
 

apps/app/src/server/util/compare-objectId.spec.ts

@@ -0,0 +1,28 @@
+import { Types } from 'mongoose';
+
+import { includesObjectIds } from './compare-objectId';
+
+describe('includesObjectIds', () => {
+  const id1 = new Types.ObjectId();
+  const id2 = new Types.ObjectId();
+  const id3 = new Types.ObjectId();
+  const id4 = new Types.ObjectId();
+
+  describe('When subset of array given', () => {
+    const arr = [id1, id2, id3, id4];
+    const subset = [id1, id4];
+
+    it('returns true', () => {
+      expect(includesObjectIds(arr, subset)).toBe(true);
+    });
+  });
+
+  describe('When set that intersects with array given', () => {
+    const arr = [id1, id2, id3];
+    const subset = [id1, id4];
+
+    it('returns false', () => {
+      expect(includesObjectIds(arr, subset)).toBe(false);
+    });
+  });
+});

apps/app/src/server/util/compare-objectId.ts

@@ -5,18 +5,17 @@ import { ObjectIdLike } from '~/server/interfaces/mongoose-utils';
 type IObjectId = mongoose.Types.ObjectId;
 const ObjectId = mongoose.Types.ObjectId;
 
-export const isIncludesObjectId = (arr: ObjectIdLike[], id: ObjectIdLike): boolean => {
+/**
+ * Check if array contains all specified ObjectIds
+ * @param arr array that potentially contains potentialSubset
+ * @param potentialSubset array that is potentially a subset of arr
+ * @returns Whether or not arr includes all elements of potentialSubset
+ */
+export const includesObjectIds = (arr: ObjectIdLike[], potentialSubset: ObjectIdLike[]): boolean => {
   const _arr = arr.map(i => i.toString());
-  const _id = id.toString();
+  const _potentialSubset = potentialSubset.map(i => i.toString());
 
-  return _arr.includes(_id);
-};
-
-// Check if two arrays have an intersection
-export const hasIntersection = (arr: ObjectIdLike[], arr2: ObjectIdLike[]): boolean => {
-  const _arr = arr.map(i => i.toString());
-  const _arr2 = arr2.map(i => i.toString());
-  return _arr.some(item => _arr2.includes(item));
+  return _potentialSubset.every(id => _arr.includes(id));
 };
 
 /**

apps/app/src/server/util/granted-group.ts

@@ -10,11 +10,12 @@ export const divideByType = (grantedGroups: GrantedGroup[]): {
   const grantedExternalUserGroups: ObjectIdLike[] = [];
 
   grantedGroups.forEach((group) => {
+    const id = typeof group.item === 'string' ? group.item : group.item._id;
     if (group.type === GroupType.userGroup) {
-      grantedUserGroups.push(group.item);
+      grantedUserGroups.push(id);
     }
     else {
-      grantedExternalUserGroups.push(group.item);
+      grantedExternalUserGroups.push(id);
     }
   });
 

packages/core/src/interfaces/page.ts

@@ -3,14 +3,16 @@ import type { HasObjectId } from './has-object-id';
 import type { IRevision, HasRevisionShortbody, IRevisionHasId } from './revision';
 import type { SubscriptionStatusType } from './subscription';
 import type { ITag } from './tag';
-import type { IUser, IUserGroupHasId, IUserHasId } from './user';
+import type {
+  IUser, IUserGroup, IUserGroupHasId, IUserHasId,
+} from './user';
 
 export const GroupType = { userGroup: 'UserGroup', externalUserGroup: 'ExternalUserGroup' } as const;
 export type GroupType = typeof GroupType[keyof typeof GroupType];
 
 export type GrantedGroup = {
   type: GroupType,
-  item: Ref<any>,
+  item: Ref<IUserGroup>,
 }
 
 export type IPage = {