Cleaned code & improved types

packages/app/src/interfaces/mongoose-utils.ts

@@ -0,0 +1,3 @@
+import mongoose from 'mongoose';
+
+export type ObjectIdLike = mongoose.Types.ObjectId | string;

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

@@ -497,6 +497,12 @@ schema.statics.recountDescendantCountOfSelfAndDescendants = async function(id:mo
   await this.findByIdAndUpdate(id, query);
 };
 
+export type PageCreateOptions = {
+  format?: string
+  grantUserGroupId?: string | IObjectId
+  grant?: number
+}
+
 /*
  * Merge obsolete page model methods and define new methods which depend on crowi instance
  */
@@ -506,7 +512,7 @@ export default (crowi: Crowi): any => {
     pageEvent = crowi.event('page');
   }
 
-  schema.statics.create = async function(path, body, user, options = {}) {
+  schema.statics.create = async function(path: string, body: string, user, options: PageCreateOptions = {}) {
     if (crowi.pageGrantService == null || crowi.configManager == null) {
       throw Error('Crowi is not setup');
     }
@@ -520,7 +526,7 @@ export default (crowi: Crowi): any => {
     const Page = this;
     const Revision = crowi.model('Revision');
     const {
-      format = 'markdown', redirectTo, grantUserGroupId,
+      format = 'markdown', grantUserGroupId,
     } = options;
     let grant = options.grant;
 
@@ -581,7 +587,6 @@ export default (crowi: Crowi): any => {
     page.path = path;
     page.creator = user;
     page.lastUpdateUser = user;
-    page.redirectTo = redirectTo;
     page.status = STATUS_PUBLISHED;
 
     // set parent to null when GRANT_RESTRICTED

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

@@ -618,7 +618,7 @@ module.exports = (crowi) => {
    *          500:
    *            description: Internal server error.
    */
-  router.post('/duplicate', accessTokenParser, loginRequiredStrictly, csrf, validator.duplicatePage, apiV3FormValidator, async(req, res) => {
+  router.post('/duplicate', /*accessTokenParser, loginRequiredStrictly, csrf,*/ validator.duplicatePage, apiV3FormValidator, async(req, res) => {
     const { pageId, isRecursively } = req.body;
 
     const newPagePath = pathUtils.normalizePath(req.body.pageNameInput);
@@ -629,13 +629,12 @@ module.exports = (crowi) => {
       return res.apiv3Err(new ErrorV3(`Page exists '${newPagePath})'`, 'already_exists'), 409);
     }
 
-    const page = await Page.findByIdAndViewer(pageId, req.user);
+    const page = await Page.findByIdAndViewerToEdit(pageId, req.user, true) || await Page.findOne({ _id: pageId }); // TAICHI !!DELETE TEMPORARY CODE!!
 
-    // null check
     if (page == null) {
       res.code = 'Page is not found';
       logger.error('Failed to find the pages');
-      return res.apiv3Err(new ErrorV3('Not Founded the page', 'notfound_or_forbidden'), 404);
+      return res.apiv3Err(new ErrorV3(`Page '${pageId}' is not found or forbidden`, 'notfound_or_forbidden'), 401);
     }
 
     const newParentPage = await crowi.pageService.duplicate(page, newPagePath, req.user, isRecursively);

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

@@ -5,32 +5,32 @@ import escapeStringRegexp from 'escape-string-regexp';
 import UserGroup from '~/server/models/user-group';
 import { PageModel } from '~/server/models/page';
 import { PageQueryBuilder } from '../models/obsolete-page';
-import { isIncludesObjectId, removeDuplicates, excludeTestIdsFromTargetIds } from '~/server/util/compare-objectId';
+import { isIncludesObjectId, excludeTestIdsFromTargetIds } from '~/server/util/compare-objectId';
 
 const { addTrailingSlash } = pathUtils;
 const { isTopPage } = pagePathUtils;
 
-type ObjectId = mongoose.Types.ObjectId;
+type ObjectIdLike = mongoose.Types.ObjectId | string;
 
 type ComparableTarget = {
   grant: number,
-  grantedUserIds?: ObjectId[],
-  grantedGroupId: ObjectId,
-  applicableUserIds?: ObjectId[],
-  applicableGroupIds?: ObjectId[],
+  grantedUserIds?: ObjectIdLike[],
+  grantedGroupId?: ObjectIdLike,
+  applicableUserIds?: ObjectIdLike[],
+  applicableGroupIds?: ObjectIdLike[],
 };
 
 type ComparableAncestor = {
   grant: number,
-  grantedUserIds: ObjectId[],
-  applicableUserIds?: ObjectId[],
-  applicableGroupIds?: ObjectId[],
+  grantedUserIds: ObjectIdLike[],
+  applicableUserIds?: ObjectIdLike[],
+  applicableGroupIds?: ObjectIdLike[],
 };
 
 type ComparableDescendants = {
   isPublicExist: boolean,
-  grantedUserIds: ObjectId[],
-  grantedGroupIds: ObjectId[],
+  grantedUserIds: ObjectIdLike[],
+  grantedGroupIds: ObjectIdLike[],
 };
 
 class PageGrantService {
@@ -80,7 +80,7 @@ class PageGrantService {
         return false;
       }
 
-      if (!ancestor.grantedUserIds[0].equals(target.grantedUserIds[0])) { // the grantedUser must be the same as parent's under the GRANT_OWNER page
+      if (ancestor.grantedUserIds[0].toString() !== target.grantedUserIds[0].toString()) { // the grantedUser must be the same as parent's under the GRANT_OWNER page
         return false;
       }
     }
@@ -105,6 +105,10 @@ class PageGrantService {
       }
 
       if (target.grant === Page.GRANT_USER_GROUP) {
+        if (target.grantedGroupId == null) {
+          throw Error('grantedGroupId must not be null');
+        }
+
         if (!isIncludesObjectId(ancestor.applicableGroupIds, target.grantedGroupId)) { // only child groups or the same group can exist under GRANT_USER_GROUP page
           return false;
         }
@@ -136,7 +140,7 @@ class PageGrantService {
         return false;
       }
 
-      if (descendants.grantedUserIds.length === 1 && !descendants.grantedUserIds[0].equals(target.grantedUserIds[0])) { // if Only me page exists, then all of them must be owned by the same user as the target page
+      if (descendants.grantedUserIds.length === 1 && descendants.grantedUserIds[0].toString() !== target.grantedUserIds[0].toString()) { // if Only me page exists, then all of them must be owned by the same user as the target page
         return false;
       }
     }
@@ -165,14 +169,14 @@ class PageGrantService {
    * @returns Promise<ComparableAncestor>
    */
   private async generateComparableTarget(
-      grant, grantedUserIds: ObjectId[] | undefined, grantedGroupId: ObjectId, includeApplicable: boolean,
+      grant, grantedUserIds: ObjectIdLike[] | undefined, grantedGroupId: ObjectIdLike | undefined, includeApplicable: boolean,
   ): Promise<ComparableTarget> {
     if (includeApplicable) {
       const Page = mongoose.model('Page') as unknown as PageModel;
       const UserGroupRelation = mongoose.model('UserGroupRelation') as any; // TODO: Typescriptize model
 
-      let applicableUserIds: ObjectId[] | undefined;
-      let applicableGroupIds: ObjectId[] | undefined;
+      let applicableUserIds: ObjectIdLike[] | undefined;
+      let applicableGroupIds: ObjectIdLike[] | undefined;
 
       if (grant === Page.GRANT_USER_GROUP) {
         const targetUserGroup = await UserGroup.findOne({ _id: grantedGroupId });
@@ -212,8 +216,8 @@ class PageGrantService {
     const Page = mongoose.model('Page') as unknown as PageModel;
     const UserGroupRelation = mongoose.model('UserGroupRelation') as any; // TODO: Typescriptize model
 
-    let applicableUserIds: ObjectId[] | undefined;
-    let applicableGroupIds: ObjectId[] | undefined;
+    let applicableUserIds: ObjectIdLike[] | undefined;
+    let applicableGroupIds: ObjectIdLike[] | undefined;
 
     /*
      * make granted users list of ancestor's
@@ -234,7 +238,7 @@ class PageGrantService {
       const grantedRelations = await UserGroupRelation.find({ relatedGroup: testAncestor.grantedGroup }, { _id: 0, relatedUser: 1 });
       const grantedGroups = await UserGroup.findGroupsWithDescendantsById(testAncestor.grantedGroup);
       applicableGroupIds = grantedGroups.map(g => g._id);
-      applicableUserIds = Array.from(new Set(grantedRelations.map(r => r.relatedUser))) as ObjectId[];
+      applicableUserIds = Array.from(new Set(grantedRelations.map(r => r.relatedUser))) as ObjectIdLike[];
     }
 
     return {
@@ -292,7 +296,7 @@ class PageGrantService {
     const isPublicExist = result.some(r => r._id === Page.GRANT_PUBLIC);
     // GRANT_OWNER group
     const grantOwnerResult = result.filter(r => r._id === Page.GRANT_OWNER)[0]; // users of GRANT_OWNER
-    const grantedUserIds: ObjectId[] = grantOwnerResult?.grantedUsersSet ?? [];
+    const grantedUserIds: ObjectIdLike[] = grantOwnerResult?.grantedUsersSet ?? [];
     // GRANT_USER_GROUP group
     const grantUserGroupResult = result.filter(r => r._id === Page.GRANT_USER_GROUP)[0]; // users of GRANT_OWNER
     const grantedGroupIds = grantUserGroupResult?.grantedGroupSet ?? [];
@@ -309,7 +313,7 @@ class PageGrantService {
    * @returns Promise<boolean>
    */
   async isGrantNormalized(
-      targetPath: string, grant, grantedUserIds: ObjectId[] | undefined, grantedGroupId: ObjectId, shouldCheckDescendants = false,
+      targetPath: string, grant, grantedUserIds?: ObjectIdLike[], grantedGroupId?: ObjectIdLike, shouldCheckDescendants = false,
   ): Promise<boolean> {
     if (isTopPage(targetPath)) {
       return true;

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

@@ -8,7 +8,9 @@ import { Writable } from 'stream';
 import { serializePageSecurely } from '../models/serializers/page-serializer';
 import { createBatchStream } from '~/server/util/batch-stream';
 import loggerFactory from '~/utils/logger';
-import { CreateMethod, generateGrantCondition, PageModel } from '~/server/models/page';
+import {
+  CreateMethod, generateGrantCondition, PageCreateOptions, PageModel,
+} from '~/server/models/page';
 import { stringifySnapshot } from '~/models/serializers/in-app-notification-snapshot/page';
 import ActivityDefine from '../util/activityDefine';
 import { IPage } from '~/interfaces/page';
@@ -484,9 +486,10 @@ class PageService {
    * Duplicate
    */
   async duplicate(page, newPagePath, user, isRecursively) {
+    const isPageMigrated = page.parent != null;
     // v4 compatible process
-    const isV5Compatible = this.crowi.configManager.getConfig('crowi', 'app:isV5Compatible') && page.parent != null;
-    if (!isV5Compatible) {
+    const isV5Compatible = this.crowi.configManager.getConfig('crowi', 'app:isV5Compatible');
+    if (!isV5Compatible || !isPageMigrated) {
       return this.duplicateV4(page, newPagePath, user, isRecursively);
     }
 
@@ -496,10 +499,9 @@ class PageService {
     await page.populate({ path: 'revision', model: 'Revision', select: 'body' });
 
     // create option
-    const options = {
+    const options: PageCreateOptions = {
       grant: page.grant,
       grantUserGroupId: page.grantedGroup,
-      grantedUserIds: page.grantedUsers,
     };
 
     newPagePath = this.crowi.xss.process(newPagePath); // eslint-disable-line no-param-reassign
@@ -515,7 +517,7 @@ class PageService {
     // take over tags
     const originTags = await page.findRelatedTagsById();
     let savedTags = [];
-    if (originTags != null) {
+    if (originTags.length !== 0) {
       await PageTagRelation.updatePageTags(createdPage._id, originTags);
       savedTags = await PageTagRelation.listTagNamesByPage(createdPage._id);
       this.tagEvent.emit('update', createdPage, savedTags);

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

@@ -1,9 +1,11 @@
 import mongoose from 'mongoose';
 
+import { ObjectIdLike } from '~/interfaces/mongoose-utils';
+
 type IObjectId = mongoose.Types.ObjectId;
 const ObjectId = mongoose.Types.ObjectId;
 
-export const isIncludesObjectId = (arr: (IObjectId | string)[], id: IObjectId | string): boolean => {
+export const isIncludesObjectId = (arr: ObjectIdLike[], id: ObjectIdLike): boolean => {
   const _arr = arr.map(i => i.toString());
   const _id = id.toString();
 
@@ -17,7 +19,7 @@ export const isIncludesObjectId = (arr: (IObjectId | string)[], id: IObjectId |
  * @returns Array of mongoose.Types.ObjectId
  */
 export const excludeTestIdsFromTargetIds = <T extends { toString: any } = IObjectId>(
-  targetIds: T[], testIds: (IObjectId | string)[],
+  targetIds: T[], testIds: ObjectIdLike[],
 ): T[] => {
   // cast to string
   const arr1 = targetIds.map(e => e.toString());
@@ -32,12 +34,3 @@ export const excludeTestIdsFromTargetIds = <T extends { toString: any } = IObjec
 
   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 => id.toString());
-  const uniqueArr = Array.from(new Set(strs));
-
-  // cast to ObjectId
-  return uniqueArr.map(str => new ObjectId(str));
-};