Merge branch 'master' into imprv/88872-revalidation-for-rename-modal-pagetree

.github/workflows/reusable-app-prod.yml

@@ -37,10 +37,10 @@ jobs:
       with:
         path: |
           **/node_modules
-        key: node_modules-${{ runner.OS }}-node${{ matrix.node-version }}-${{ hashFiles('**/yarn.lock') }}-${{ hashFiles('packages/app/package.json') }}
+        key: node_modules-${{ runner.OS }}-node${{ inputs.node-version }}-${{ hashFiles('**/yarn.lock') }}-${{ hashFiles('packages/app/package.json') }}
         restore-keys: |
-          node_modules-${{ runner.OS }}-node${{ matrix.node-version }}-${{ hashFiles('**/yarn.lock') }}-
-          node_modules-${{ runner.OS }}-node${{ matrix.node-version }}-
+          node_modules-${{ runner.OS }}-node${{ inputs.node-version }}-${{ hashFiles('**/yarn.lock') }}-
+          node_modules-${{ runner.OS }}-node${{ inputs.node-version }}-
 
     - name: lerna bootstrap
       run: |

packages/app/resource/locales/en_US/translation.json

@@ -383,9 +383,6 @@
       "no_deadline":"This page has no expiration date"
     }
   },
-  "page_table_of_contents": {
-    "empty": "Table of Contents is empty"
-  },
   "page_edit": {
     "Show active line": "Show active line",
     "auto_format_table": "Auto format table",

packages/app/resource/locales/ja_JP/translation.json

@@ -382,9 +382,6 @@
       "no_deadline": "このページに有効期限は設定されていません。"
     }
   },
-  "page_table_of_contents": {
-    "empty": "目次は空です"
-  },
   "page_edit": {
     "Show active line": "アクティブ行をハイライト",
     "auto_format_table": "表の自動整形",

packages/app/resource/locales/zh_CN/translation.json

@@ -369,9 +369,6 @@
 			"conflict": "无法保存您所做的更改，因为其他人正在编辑此页。请在重新加载页面后重新编辑受影响的部分。"
 		}
   },
-  "page_table_of_contents": {
-    "empty": "目录为空"
-  },
   "page_comment": {
     "display_the_page_when_posting_this_comment": "Display the page when posting this comment"
   },

packages/app/src/components/Admin/UserGroup/UserGroupPage.tsx

@@ -81,7 +81,6 @@ const UserGroupPage: FC = () => {
       await apiv3Post('/user-groups', {
         name: userGroupData.name,
         description: userGroupData.description,
-        parent: userGroupData.parent,
       });
       toastSuccess(t('toaster.update_successed', { target: t('UserGroup') }));
       await mutateUserGroups();

packages/app/src/components/Admin/UserGroupDetail/UserGroupDetailPage.tsx

@@ -5,6 +5,7 @@ import { useTranslation } from 'react-i18next';
 
 import UserGroupForm from '../UserGroup/UserGroupForm';
 import UserGroupTable from '../UserGroup/UserGroupTable';
+import UserGroupCreateModal from '../UserGroup/UserGroupCreateModal';
 import UserGroupDeleteModal from '../UserGroup/UserGroupDeleteModal';
 import UserGroupDropdown from '../UserGroup/UserGroupDropdown';
 import UserGroupUserTable from './UserGroupUserTable';
@@ -33,11 +34,11 @@ const UserGroupDetailPage: FC = () => {
    */
   const [userGroup, setUserGroup] = useState<IUserGroupHasId>(JSON.parse(adminUserGroupDetailElem?.getAttribute('data-user-group') || 'null'));
   const [relatedPages, setRelatedPages] = useState<IPageHasId[]>([]); // For page list
-  const [isUserGroupUserModalOpen, setUserGroupUserModalOpen] = useState<boolean>(false);
   const [searchType, setSearchType] = useState<string>('partial');
   const [isAlsoMailSearched, setAlsoMailSearched] = useState<boolean>(false);
   const [isAlsoNameSearched, setAlsoNameSearched] = useState<boolean>(false);
   const [selectedUserGroup, setSelectedUserGroup] = useState<IUserGroupHasId | undefined>(undefined); // not null but undefined (to use defaultProps in UserGroupDeleteModal)
+  const [isCreateModalShown, setCreateModalShown] = useState<boolean>(false);
   const [isDeleteModalShown, setDeleteModalShown] = useState<boolean>(false);
 
   /*
@@ -86,14 +87,6 @@ const UserGroupDetailPage: FC = () => {
     }
   }, [t, userGroup._id, setUserGroup]);
 
-  const openUserGroupUserModal = useCallback(() => {
-    setUserGroupUserModalOpen(true);
-  }, []);
-
-  const closeUserGroupUserModal = useCallback(() => {
-    setUserGroupUserModalOpen(false);
-  }, []);
-
   const fetchApplicableUsers = useCallback(async(searchWord) => {
     const res = await apiv3Get(`/user-groups/${userGroup._id}/unrelated-users`, {
       searchWord,
@@ -135,10 +128,28 @@ const UserGroupDetailPage: FC = () => {
     }
   };
 
-  // TODO 87614: UserGroup New creation form can be displayed in modal
-  const onClickCreateChildGroupButtonHandler = () => {
-    console.log('button clicked!');
-  };
+  const showCreateModal = useCallback(() => {
+    setCreateModalShown(true);
+  }, [setCreateModalShown]);
+
+  const hideCreateModal = useCallback(() => {
+    setCreateModalShown(false);
+  }, [setCreateModalShown]);
+
+  const createChildUserGroup = useCallback(async(userGroupData: IUserGroup) => {
+    try {
+      await apiv3Post('/user-groups', {
+        name: userGroupData.name,
+        description: userGroupData.description,
+        parentId: userGroup._id,
+      });
+      mutateChildUserGroups();
+      toastSuccess(t('toaster.update_successed', { target: t('UserGroup') }));
+    }
+    catch (err) {
+      toastError(err);
+    }
+  }, [t, userGroup, mutateChildUserGroups]);
 
   const showDeleteModal = useCallback(async(group: IUserGroupHasId) => {
     setSelectedUserGroup(group);
@@ -199,26 +210,29 @@ const UserGroupDetailPage: FC = () => {
       <UserGroupDropdown
         selectableUserGroups={selectableUserGroups}
         onClickAddExistingUserGroupButtonHandler={onClickAddChildButtonHandler}
-        onClickCreateUserGroupButtonHandler={() => onClickCreateChildGroupButtonHandler()}
+        onClickCreateUserGroupButtonHandler={showCreateModal}
+      />
+      <UserGroupCreateModal
+        onClickCreateButton={createChildUserGroup}
+        isShow={isCreateModalShown}
+        onHide={hideCreateModal}
       />
 
-      <>
-        <UserGroupTable
-          userGroups={childUserGroups}
-          childUserGroups={grandChildUserGroups}
-          isAclEnabled={isAclEnabled ?? false}
-          onDelete={showDeleteModal}
-          userGroupRelations={childUserGroupRelations}
-        />
-        <UserGroupDeleteModal
-          userGroups={childUserGroups}
-          deleteUserGroup={selectedUserGroup}
-          onDelete={deleteChildUserGroupById}
-          isShow={isDeleteModalShown}
-          onShow={showDeleteModal}
-          onHide={hideDeleteModal}
-        />
-      </>
+      <UserGroupTable
+        userGroups={childUserGroups}
+        childUserGroups={grandChildUserGroups}
+        isAclEnabled={isAclEnabled ?? false}
+        onDelete={showDeleteModal}
+        userGroupRelations={childUserGroupRelations}
+      />
+      <UserGroupDeleteModal
+        userGroups={childUserGroups}
+        deleteUserGroup={selectedUserGroup}
+        onDelete={deleteChildUserGroupById}
+        isShow={isDeleteModalShown}
+        onShow={showDeleteModal}
+        onHide={hideDeleteModal}
+      />
 
       <h2 className="admin-setting-header mt-4">{t('Page')}</h2>
       <div className="page-list">

packages/app/src/components/Page.jsx

@@ -21,7 +21,7 @@ import { getOptionsToSave } from '~/client/util/editor';
 
 // TODO: remove this when omitting unstated is completed
 import {
-  useEditorMode, useSelectedGrant, useSelectedGrantGroupId, useSelectedGrantGroupName,
+  EditorMode, useEditorMode, useSelectedGrant, useSelectedGrantGroupId, useSelectedGrantGroupName,
 } from '~/stores/ui';
 import { useIsSlackEnabled } from '~/stores/editor';
 import { useSlackChannels } from '~/stores/context';
@@ -143,14 +143,15 @@ class Page extends React.Component {
   }
 
   render() {
-    const { appContainer, pageContainer } = this.props;
+    const { appContainer, pageContainer, editorMode } = this.props;
     const { isMobile } = appContainer;
     const isLoggedIn = appContainer.currentUser != null;
-    const { markdown } = pageContainer.state;
+    const { markdown, revisionId } = pageContainer.state;
+    const renderable = !(editorMode === EditorMode.View && revisionId == null);
 
     return (
       <div className={`mb-5 ${isMobile ? 'page-mobile' : ''}`}>
-        <RevisionRenderer growiRenderer={this.growiRenderer} markdown={markdown} />
+        <RevisionRenderer growiRenderer={this.growiRenderer} markdown={markdown} renderable={renderable} />
 
         { isLoggedIn && (
           <>

packages/app/src/components/Page/RevisionRenderer.jsx

@@ -33,16 +33,20 @@ class LegacyRevisionRenderer extends React.PureComponent {
   }
 
   componentDidMount() {
-    this.initCurrentRenderingContext();
-    this.renderHtml();
+    const { renderable } = this.props;
+
+    if (renderable) {
+      this.initCurrentRenderingContext();
+      this.renderHtml();
+    }
   }
 
   componentDidUpdate(prevProps) {
     const { markdown: prevMarkdown, highlightKeywords: prevHighlightKeywords } = prevProps;
-    const { markdown, highlightKeywords } = this.props;
+    const { markdown, renderable, highlightKeywords } = this.props;
 
     // render only when props.markdown is updated
-    if (markdown !== prevMarkdown || highlightKeywords !== prevHighlightKeywords) {
+    if ((markdown !== prevMarkdown || highlightKeywords !== prevHighlightKeywords) && renderable) {
       this.initCurrentRenderingContext();
       this.renderHtml();
       return;
@@ -172,6 +176,7 @@ LegacyRevisionRenderer.propTypes = {
   appContainer: PropTypes.instanceOf(AppContainer).isRequired,
   growiRenderer: PropTypes.instanceOf(GrowiRenderer).isRequired,
   markdown: PropTypes.string.isRequired,
+  renderable: PropTypes.bool,
   highlightKeywords: PropTypes.oneOfType([PropTypes.string, PropTypes.arrayOf(PropTypes.string)]),
   additionalClassName: PropTypes.string,
 };

packages/app/src/components/Sidebar/PageTree/Item.tsx

@@ -10,6 +10,8 @@ import nodePath from 'path';
 
 import { pathUtils, pagePathUtils } from '@growi/core';
 
+import loggerFactory from '~/utils/logger';
+
 import { toastWarning, toastError, toastSuccess } from '~/client/util/apiNotification';
 
 import { useSWRxPageChildren } from '~/stores/page-listing';
@@ -21,6 +23,11 @@ import { bookmark, unbookmark } from '~/client/services/page-operation';
 import ClosableTextInput, { AlertInfo, AlertType } from '../../Common/ClosableTextInput';
 import { PageItemControl } from '../../Common/Dropdown/PageItemControl';
 import { ItemNode } from './ItemNode';
+import { IPageHasId } from '~/interfaces/page';
+
+
+const logger = loggerFactory('growi:cli:Item');
+
 
 interface ItemProps {
   isEnableActions: boolean
@@ -55,6 +62,37 @@ const bookmarkMenuItemClickHandler = async(_pageId: string, _newValue: boolean):
 };
 
 
+/**
+ * Return new page path after the droppedPagePath is moved under the newParentPagePath
+ * @param droppedPagePath
+ * @param newParentPagePath
+ * @returns
+ */
+const getNewPathAfterMoved = (droppedPagePath: string, newParentPagePath: string): string => {
+  const pageTitle = nodePath.basename(droppedPagePath);
+  return nodePath.join(newParentPagePath, pageTitle);
+};
+
+/**
+ * Return whether the fromPage could be moved under the newParentPage
+ * @param fromPage
+ * @param newParentPage
+ * @param printLog
+ * @returns
+ */
+const canMoveUnderNewParent = (fromPage?: Partial<IPageHasId>, newParentPage?: Partial<IPageHasId>, printLog = false): boolean => {
+  if (fromPage == null || newParentPage == null || fromPage.path == null || newParentPage.path == null) {
+    if (printLog) {
+      logger.warn('Any of page, page.path or droppedPage.path is null');
+    }
+    return false;
+  }
+
+  const newPathAfterMoved = getNewPathAfterMoved(fromPage.path, newParentPage.path);
+  return pagePathUtils.canMoveByPath(fromPage.path, newPathAfterMoved);
+};
+
+
 type ItemCountProps = {
   descendantCount: number
 }
@@ -119,16 +157,18 @@ const Item: FC<ItemProps> = (props: ItemProps) => {
     }),
   }));
 
-  const pageItemDropHandler = async(item, monitor) => {
-    if (page == null || page.path == null) {
+  const pageItemDropHandler = async(item: ItemNode) => {
+    const { page: droppedPage } = item;
+
+    if (!canMoveUnderNewParent(droppedPage, page, true)) {
       return;
     }
 
-    const { page: droppedPage } = item;
+    if (droppedPage.path == null || page.path == null) {
+      return;
+    }
 
-    const pageTitle = nodePath.basename(droppedPage.path);
-    const newParentPath = page.path;
-    const newPagePath = nodePath.join(newParentPath, pageTitle);
+    const newPagePath = getNewPathAfterMoved(droppedPage.path, page.path);
 
     try {
       await apiv3Put('/pages/rename', {
@@ -157,7 +197,7 @@ const Item: FC<ItemProps> = (props: ItemProps) => {
     }
   };
 
-  const [{ isOver }, drop] = useDrop(() => ({
+  const [{ isOver }, drop] = useDrop<ItemNode, Promise<void>, { isOver: boolean }>(() => ({
     accept: 'PAGE_TREE',
     drop: pageItemDropHandler,
     hover: (item, monitor) => {
@@ -170,6 +210,10 @@ const Item: FC<ItemProps> = (props: ItemProps) => {
         }, 1000);
       }
     },
+    canDrop: (item) => {
+      const { page: droppedPage } = item;
+      return canMoveUnderNewParent(droppedPage, page);
+    },
     collect: monitor => ({
       isOver: monitor.isOver(),
     }),

packages/app/src/components/TableOfContents.jsx

@@ -74,7 +74,6 @@ const TableOfContents = (props) => {
             id="revision-toc-content"
             className="revision-toc-content mb-2"
           >
-            <span className="text-muted">({t('page_table_of_contents.empty')})</span>
           </div>
         ) }
 

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)) {

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

@@ -1,10 +1,8 @@
-import { pagePathUtils, pathUtils } from '@growi/core';
-import escapeStringRegexp from 'escape-string-regexp';
+import { pagePathUtils } from '@growi/core';
 
 import PageOperation from '~/server/models/page-operation';
 
-const { addTrailingSlash } = pathUtils;
-const { isTrashPage } = pagePathUtils;
+const { isEitherOfPathAreaOverlap, isPathAreaOverlap, isTrashPage } = pagePathUtils;
 
 class PageOperationService {
 
@@ -18,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();
@@ -36,12 +34,12 @@ class PageOperationService {
     if (isRecursively) {
 
       if (fromPathToOp != null && !isTrashPage(fromPathToOp)) {
-        const flag = toPaths.some(p => this.isEitherOfPathAreaOverlap(p, fromPathToOp));
+        const flag = toPaths.some(p => isEitherOfPathAreaOverlap(p, fromPathToOp));
         if (flag) return false;
       }
 
       if (toPathToOp != null && !isTrashPage(toPathToOp)) {
-        const flag = toPaths.some(p => this.isPathAreaOverlap(p, toPathToOp));
+        const flag = toPaths.some(p => isPathAreaOverlap(p, toPathToOp));
         if (flag) return false;
       }
 
@@ -49,12 +47,12 @@ class PageOperationService {
     else {
 
       if (fromPathToOp != null && !isTrashPage(fromPathToOp)) {
-        const flag = toPaths.some(p => this.isPathAreaOverlap(p, fromPathToOp));
+        const flag = toPaths.some(p => isPathAreaOverlap(p, fromPathToOp));
         if (flag) return false;
       }
 
       if (toPathToOp != null && !isTrashPage(toPathToOp)) {
-        const flag = toPaths.some(p => this.isPathAreaOverlap(p, toPathToOp));
+        const flag = toPaths.some(p => isPathAreaOverlap(p, toPathToOp));
         if (flag) return false;
       }
 
@@ -63,39 +61,6 @@ class PageOperationService {
     return true;
   }
 
-  private isEitherOfPathAreaOverlap(path1: string, path2: string): boolean {
-    if (path1 === path2) {
-      return true;
-    }
-
-    const path1WithSlash = addTrailingSlash(path1);
-    const path2WithSlash = addTrailingSlash(path2);
-
-    const path1Area = new RegExp(`^${escapeStringRegexp(path1WithSlash)}`);
-    const path2Area = new RegExp(`^${escapeStringRegexp(path2WithSlash)}`);
-
-    if (path1Area.test(path2) || path2Area.test(path1)) {
-      return true;
-    }
-
-    return false;
-  }
-
-  private isPathAreaOverlap(pathToTest: string, pathToBeTested: string): boolean {
-    if (pathToTest === pathToBeTested) {
-      return true;
-    }
-
-    const pathWithSlash = addTrailingSlash(pathToTest);
-
-    const pathAreaToTest = new RegExp(`^${escapeStringRegexp(pathWithSlash)}`);
-    if (pathAreaToTest.test(pathToBeTested)) {
-      return true;
-    }
-
-    return false;
-  }
-
 }
 
 export default PageOperationService;

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);

packages/core/src/utils/page-path-utils.ts

@@ -1,6 +1,7 @@
 import nodePath from 'path';
 
 import escapeStringRegexp from 'escape-string-regexp';
+import { addTrailingSlash } from './path-utils';
 
 /**
  * Whether path is the top page
@@ -194,3 +195,60 @@ export const omitDuplicateAreaPageFromPages = (pages: any[]): any[] => {
     return !isDuplicate;
   });
 };
+
+/**
+ * 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;
+  }
+
+  const path1WithSlash = addTrailingSlash(path1);
+  const path2WithSlash = addTrailingSlash(path2);
+
+  const path1Area = new RegExp(`^${escapeStringRegexp(path1WithSlash)}`, 'i');
+  const path2Area = new RegExp(`^${escapeStringRegexp(path2WithSlash)}`, 'i');
+
+  if (path1Area.test(path2) || path2Area.test(path1)) {
+    return true;
+  }
+
+  return false;
+};
+
+/**
+ * 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)}`, 'i');
+  if (pathAreaToTest.test(pathToBeTested)) {
+    return true;
+  }
+
+  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 => {
+  return !isPathAreaOverlap(fromPath, toPath);
+};