Ver Fonte

Added validation

Taichi Masuyama há 4 anos atrás
pai
commit
071847505f

+ 2 - 0
packages/app/src/server/routes/apiv3/pages.js

@@ -175,6 +175,7 @@ module.exports = (crowi) => {
       body('newPagePath').isLength({ min: 1 }).withMessage('newPagePath is required'),
       body('isRenameRedirect').if(value => value != null).isBoolean().withMessage('isRenameRedirect must be boolean'),
       body('isRemainMetadata').if(value => value != null).isBoolean().withMessage('isRemainMetadata must be boolean'),
+      body('isMoveMode').if(value => value != null).isBoolean().withMessage('isMoveMode must be boolean'),
     ],
     duplicatePage: [
       body('pageId').isMongoId().withMessage('pageId is required'),
@@ -473,6 +474,7 @@ module.exports = (crowi) => {
     const options = {
       createRedirectPage: req.body.isRenameRedirect,
       updateMetadata: !req.body.isRemainMetadata,
+      isMoveMode: req.body.isMoveMode,
     };
 
     if (!isCreatablePage(newPagePath)) {

+ 34 - 7
packages/app/src/server/service/page-operation.ts

@@ -2,7 +2,7 @@ import { pagePathUtils } from '@growi/core';
 
 import PageOperation from '~/server/models/page-operation';
 
-const { isOperatable } = pagePathUtils;
+const { isEitherOfPathAreaOverlap, isPathAreaOverlap, isTrashPage } = pagePathUtils;
 
 class PageOperationService {
 
@@ -16,11 +16,11 @@ class PageOperationService {
   }
 
   /**
-   * Check if the operation is operatable by comparing paths with all Main PageOperation documents
-   * @param fromPath The path to operate from
-   * @param toPath The path to operate to
-   * @param actionType The action type of the operation
-   * @returns Promise<boolean>
+   * Check if the operation is operatable
+   * @param isRecursively Boolean that determines whether the operation is recursive or not
+   * @param fromPathToOp The path to operate from
+   * @param toPathToOp The path to operate to
+   * @returns boolean
    */
   async canOperate(isRecursively: boolean, fromPathToOp: string | null, toPathToOp: string | null): Promise<boolean> {
     const mainOps = await PageOperation.findMainOps();
@@ -31,7 +31,34 @@ class PageOperationService {
 
     const toPaths = mainOps.map(op => op.toPath).filter((p): p is string => p != null);
 
-    return isOperatable(isRecursively, fromPathToOp, toPathToOp, toPaths);
+    if (isRecursively) {
+
+      if (fromPathToOp != null && !isTrashPage(fromPathToOp)) {
+        const flag = toPaths.some(p => isEitherOfPathAreaOverlap(p, fromPathToOp));
+        if (flag) return false;
+      }
+
+      if (toPathToOp != null && !isTrashPage(toPathToOp)) {
+        const flag = toPaths.some(p => isPathAreaOverlap(p, toPathToOp));
+        if (flag) return false;
+      }
+
+    }
+    else {
+
+      if (fromPathToOp != null && !isTrashPage(fromPathToOp)) {
+        const flag = toPaths.some(p => isPathAreaOverlap(p, fromPathToOp));
+        if (flag) return false;
+      }
+
+      if (toPathToOp != null && !isTrashPage(toPathToOp)) {
+        const flag = toPaths.some(p => isPathAreaOverlap(p, toPathToOp));
+        if (flag) return false;
+      }
+
+    }
+
+    return true;
   }
 
 }

+ 10 - 5
packages/app/src/server/service/page.ts

@@ -29,7 +29,7 @@ const debug = require('debug')('growi:services:page');
 const logger = loggerFactory('growi:services:page');
 const {
   isTrashPage, isTopPage, omitDuplicateAreaPageFromPages,
-  collectAncestorPaths, isMovablePage,
+  collectAncestorPaths, isMovablePage, canMoveByPath,
 } = pagePathUtils;
 
 const { addTrailingSlash } = pathUtils;
@@ -344,8 +344,7 @@ class PageService {
 
     const isExist = await Page.exists({ path: newPagePath });
     if (isExist) {
-      // if page found, cannot rename to that path
-      throw new Error('the path already exists');
+      throw Error(`Page already exists at ${newPagePath}`);
     }
 
     if (isTopPage(page.path)) {
@@ -358,8 +357,14 @@ class PageService {
       return this.renamePageV4(page, newPagePath, user, options);
     }
 
-    if (await Page.exists({ path: newPagePath })) {
-      throw Error(`Page already exists at ${newPagePath}`);
+    if (options.isMoveMode) {
+      const fromPath = page.path;
+      const toPath = newPagePath;
+      const canMove = canMoveByPath(fromPath, toPath) && await Page.exists({ path: newPagePath });
+
+      if (!canMove) {
+        throw Error('Cannot move to this path.');
+      }
     }
 
     const canOperate = await this.crowi.pageOperationService.canOperate(true, page.path, newPagePath);

+ 30 - 33
packages/core/src/utils/page-path-utils.ts

@@ -196,8 +196,14 @@ export const omitDuplicateAreaPageFromPages = (pages: any[]): any[] => {
   });
 };
 
-
-const isEitherOfPathAreaOverlap = (path1: string, path2: string): boolean => {
+/**
+ * Check if the area of either path1 or path2 includes the area of the other path
+ * The area of path is the same as /^\/hoge\//i
+ * @param pathToTest string
+ * @param pathToBeTested string
+ * @returns boolean
+ */
+export const isEitherOfPathAreaOverlap = (path1: string, path2: string): boolean => {
   if (path1 === path2) {
     return true;
   }
@@ -205,8 +211,8 @@ const isEitherOfPathAreaOverlap = (path1: string, path2: string): boolean => {
   const path1WithSlash = addTrailingSlash(path1);
   const path2WithSlash = addTrailingSlash(path2);
 
-  const path1Area = new RegExp(`^${escapeStringRegexp(path1WithSlash)}`);
-  const path2Area = new RegExp(`^${escapeStringRegexp(path2WithSlash)}`);
+  const path1Area = new RegExp(`^${escapeStringRegexp(path1WithSlash)}`, 'i');
+  const path2Area = new RegExp(`^${escapeStringRegexp(path2WithSlash)}`, 'i');
 
   if (path1Area.test(path2) || path2Area.test(path1)) {
     return true;
@@ -214,14 +220,22 @@ const isEitherOfPathAreaOverlap = (path1: string, path2: string): boolean => {
 
   return false;
 };
-const isPathAreaOverlap = (pathToTest: string, pathToBeTested: string): boolean => {
+
+/**
+ * Check if the area of pathToTest includes the area of pathToBeTested
+ * The area of path is the same as /^\/hoge\//i
+ * @param pathToTest string
+ * @param pathToBeTested string
+ * @returns boolean
+ */
+export const isPathAreaOverlap = (pathToTest: string, pathToBeTested: string): boolean => {
   if (pathToTest === pathToBeTested) {
     return true;
   }
 
   const pathWithSlash = addTrailingSlash(pathToTest);
 
-  const pathAreaToTest = new RegExp(`^${escapeStringRegexp(pathWithSlash)}`);
+  const pathAreaToTest = new RegExp(`^${escapeStringRegexp(pathWithSlash)}`, 'i');
   if (pathAreaToTest.test(pathToBeTested)) {
     return true;
   }
@@ -229,33 +243,16 @@ const isPathAreaOverlap = (pathToTest: string, pathToBeTested: string): boolean
   return false;
 };
 
-export const isOperatable = (isRecursively: boolean, fromPathToOp: string | null, toPathToOp: string | null, toPathsToTest: string[]): boolean => {
-  if (isRecursively) {
-
-    if (fromPathToOp != null && !isTrashPage(fromPathToOp)) {
-      const flag = toPathsToTest.some(p => isEitherOfPathAreaOverlap(p, fromPathToOp));
-      if (flag) return false;
-    }
-
-    if (toPathToOp != null && !isTrashPage(toPathToOp)) {
-      const flag = toPathsToTest.some(p => isPathAreaOverlap(p, toPathToOp));
-      if (flag) return false;
-    }
-
-  }
-  else {
-
-    if (fromPathToOp != null && !isTrashPage(fromPathToOp)) {
-      const flag = toPathsToTest.some(p => isPathAreaOverlap(p, fromPathToOp));
-      if (flag) return false;
-    }
-
-    if (toPathToOp != null && !isTrashPage(toPathToOp)) {
-      const flag = toPathsToTest.some(p => isPathAreaOverlap(p, toPathToOp));
-      if (flag) return false;
-    }
-
+/**
+ * Determine whether can move by fromPath and toPath
+ * @param fromPath string
+ * @param toPath string
+ * @returns boolean
+ */
+export const canMoveByPath = (fromPath: string, toPath: string): boolean => {
+  if (fromPath === toPath) {
+    return false;
   }
 
-  return true;
+  return !isPathAreaOverlap(fromPath, toPath);
 };