Taichi Masuyama před 4 roky
rodič
revize
58d7205d0a

+ 2 - 6
packages/app/src/server/models/page.ts

@@ -39,7 +39,7 @@ type TargetAndAncestorsResult = {
 export interface PageModel extends Model<PageDocument> {
   [x: string]: any; // for obsolete methods
   createEmptyPagesByPaths(paths: string[], publicOnly?: boolean): Promise<void>
-  getParentIdAndFillAncestors(path: string, parent: PageDocument): Promise<string | null>
+  getParentIdAndFillAncestors(path: string, parent: (PageDocument & { _id: any }) | null): Promise<string | null>
   findByPathAndViewer(path: string | null, user, userGroups?, useFindOne?: boolean, includeEmpty?: boolean): Promise<PageDocument[]>
   findTargetAndAncestorsByPathOrId(pathOrId: string): Promise<TargetAndAncestorsResult>
   findChildrenByParentPathOrIdAndViewer(parentPathOrId: string, user, userGroups?): Promise<PageDocument[]>
@@ -151,10 +151,6 @@ schema.statics.createEmptyPagesByPaths = async function(paths: string[], publicO
   }
 };
 
-schema.statics.findOneParentByParentPath = async function(parentPath: string): Promise<PageDocument | null> {
-  return this.findOne({ path: parentPath }); // find the oldest parent which must always be the true parent
-};
-
 /*
  * Find the parent and update if the parent exists.
  * If not,
@@ -435,7 +431,7 @@ export default (crowi: Crowi): any => {
 
     let parentId: string | null = null;
     const parentPath = nodePath.dirname(path);
-    const parent = await this.findOneParentByParentPath(parentPath);
+    const parent = await this.findOne({ path: parentPath }); // find the oldest parent which must always be the true parent
     if (!isTopPage(path)) {
       parentId = await Page.getParentIdAndFillAncestors(path, parent);
     }

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

@@ -67,7 +67,7 @@ class PageGrantService {
       // do nothing
     }
     // GRANT_OWNER
-    if (ancestor.grant === Page.GRANT_OWNER) {
+    else if (ancestor.grant === Page.GRANT_OWNER) {
       if (target.grant !== Page.GRANT_OWNER) {
         return false;
       }
@@ -77,7 +77,7 @@ class PageGrantService {
       }
     }
     // GRANT_USER_GROUP
-    if (ancestor.grant === Page.GRANT_USER_GROUP) {
+    else if (ancestor.grant === Page.GRANT_USER_GROUP) {
       if (ancestor.applicableGroupIds == null || ancestor.applicableUserIds == null) {
         throw Error('applicableGroupIds and applicableUserIds are not specified');
       }
@@ -111,7 +111,7 @@ class PageGrantService {
       // do nothing
     }
     // GRANT_OWNER
-    if (target.grant === Page.GRANT_OWNER) {
+    else if (target.grant === Page.GRANT_OWNER) {
       if (descendants.descendantGroupIds.length !== 0 || descendants.grantedUserIds.length > 1) {
         return false;
       }
@@ -121,7 +121,7 @@ class PageGrantService {
       }
     }
     // GRANT_USER_GROUP
-    if (target.grant === Page.GRANT_USER_GROUP) {
+    else if (target.grant === Page.GRANT_USER_GROUP) {
       if (target.applicableGroupIds == null) {
         throw Error('applicableGroupIds must not be null');
       }

+ 2 - 2
packages/app/src/server/service/user-group.ts

@@ -2,7 +2,7 @@ import mongoose from 'mongoose';
 
 import loggerFactory from '~/utils/logger';
 import UserGroup, { UserGroupDocument } from '~/server/models/user-group';
-import { compareObjectId, isIncludesObjectId } from '~/server/util/compare-objectId';
+import { isIncludesObjectId } from '~/server/util/compare-objectId';
 
 const logger = loggerFactory('growi:service:UserGroupService'); // eslint-disable-line no-unused-vars
 
@@ -61,7 +61,7 @@ class UserGroupService {
 
     // throw if parent was in its descendants
     const descendantsWithTarget = await UserGroup.findGroupsWithDescendantsRecursively([userGroup]);
-    const descendants = descendantsWithTarget.filter(d => compareObjectId(d._id, userGroup._id));
+    const descendants = descendantsWithTarget.filter(d => d._id.equals(userGroup._id));
     if (isIncludesObjectId(descendants, parent._id)) {
       throw Error('It is not allowed to choose parent from descendant groups.');
     }

+ 14 - 20
packages/app/src/server/util/compare-objectId.ts

@@ -2,22 +2,11 @@ import mongoose from 'mongoose';
 
 type IObjectId = mongoose.Types.ObjectId;
 const ObjectId = mongoose.Types.ObjectId;
-
-const castToString = (val: string | IObjectId) => {
-  if (typeof val === 'string') {
-    return val;
-  }
-
-  return val.toString();
-};
-
-export const compareObjectId = (id1: IObjectId | string, id2: IObjectId | string): boolean => {
-  return castToString(id1) === castToString(id2);
-};
+type ObjectIdLike = IObjectId | string;
 
 export const isIncludesObjectId = (arr: (IObjectId | string)[], id: IObjectId | string): boolean => {
-  const _arr = arr.map(i => castToString(i));
-  const _id = castToString(id);
+  const _arr = arr.map(i => i.toString());
+  const _id = id.toString();
 
   return _arr.includes(_id);
 };
@@ -28,21 +17,26 @@ export const isIncludesObjectId = (arr: (IObjectId | string)[], id: IObjectId |
  * @param testIds Array of mongoose.Types.ObjectId
  * @returns Array of mongoose.Types.ObjectId
  */
-export const excludeTestIdsFromTargetIds = (targetIds: (IObjectId | string)[], testIds: (IObjectId | string)[]): IObjectId[] => {
+export const excludeTestIdsFromTargetIds = <T extends { toString: any } = IObjectId>(
+  targetIds: T[], testIds: (IObjectId | string)[],
+): T[] => {
   // cast to string
-  const arr1 = targetIds.map(e => castToString(e));
-  const arr2 = testIds.map(e => castToString(e));
+  const arr1 = targetIds.map(e => e.toString());
+  const arr2 = testIds.map(e => e.toString());
 
   // filter
   const excluded = arr1.filter(e => !arr2.includes(e));
-
   // cast to ObjectId
-  return excluded.map(e => new ObjectId(e));
+  const shouldReturnString = (arr: any[]): arr is string[] => {
+    return typeof arr[0] === 'string';
+  };
+
+  return shouldReturnString(targetIds) ? excluded : excluded.map(e => new ObjectId(e));
 };
 
 export const removeDuplicates = (objectIds: (IObjectId | string)[]): IObjectId[] => {
   // cast to string
-  const strs = objectIds.map(id => castToString(id));
+  const strs = objectIds.map(id => id.toString());
   const uniqueArr = Array.from(new Set(strs));
 
   // cast to ObjectId