Просмотр исходного кода

WIP: refactor creating page API

Yuki Takei 2 лет назад
Родитель
Сommit
5abeba3cb8

+ 5 - 14
apps/app/src/client/services/page-operation.ts

@@ -3,7 +3,7 @@ import { useCallback } from 'react';
 import { SubscriptionStatusType, type Nullable } from '@growi/core';
 import { SubscriptionStatusType, type Nullable } from '@growi/core';
 import urljoin from 'url-join';
 import urljoin from 'url-join';
 
 
-import type { OptionsToSave } from '~/interfaces/page-operation';
+import type { IApiv3PageCreateParams, IApiv3PageCreateResponse, OptionsToSave } from '~/interfaces/page-operation';
 import { useEditingMarkdown, useIsEnabledUnsavedWarning, usePageTagsForEditors } from '~/stores/editor';
 import { useEditingMarkdown, useIsEnabledUnsavedWarning, usePageTagsForEditors } from '~/stores/editor';
 import { useCurrentPageId, useSWRMUTxCurrentPage, useSWRxTagsInfo } from '~/stores/page';
 import { useCurrentPageId, useSWRMUTxCurrentPage, useSWRxTagsInfo } from '~/stores/page';
 import { useSetRemoteLatestPageData } from '~/stores/remote-latest-page';
 import { useSetRemoteLatestPageData } from '~/stores/remote-latest-page';
@@ -87,18 +87,9 @@ export const resumeRenameOperation = async(pageId: string): Promise<void> => {
   await apiv3Post('/pages/resume-rename', { pageId });
   await apiv3Post('/pages/resume-rename', { pageId });
 };
 };
 
 
-// TODO: define return type
-export const createPage = async(pagePath: string, markdown: string, tmpParams: OptionsToSave) => {
-  // clone
-  const params = Object.assign(tmpParams, {
-    path: pagePath,
-    body: markdown,
-  });
-
-  const res = await apiv3Post('/page', params);
-  const { page, tags, revision } = res.data;
-
-  return { page, tags, revision };
+export const createPage = async(params: IApiv3PageCreateParams): Promise<IApiv3PageCreateResponse> => {
+  const res = await apiv3Post<IApiv3PageCreateResponse>('/page', params);
+  return res.data;
 };
 };
 
 
 // TODO: define return type
 // TODO: define return type
@@ -138,7 +129,7 @@ export const useSaveOrUpdate = (): SaveOrUpdateFunction => {
 
 
     let res;
     let res;
     if (pageId == null || revisionId == null) {
     if (pageId == null || revisionId == null) {
-      res = await createPage(path, markdown, options);
+      res = await createPage({ path, body: markdown, ...options });
     }
     }
     else {
     else {
       if (revisionId == null) {
       if (revisionId == null) {

+ 2 - 2
apps/app/src/components/Sidebar/PageCreateButton/PageCreateButton.tsx

@@ -8,7 +8,7 @@ import { Dropdown } from 'reactstrap';
 
 
 import { useOnTemplateButtonClicked } from '~/client/services/use-on-template-button-clicked';
 import { useOnTemplateButtonClicked } from '~/client/services/use-on-template-button-clicked';
 import { toastError } from '~/client/util/toastr';
 import { toastError } from '~/client/util/toastr';
-import { LabelType } from '~/interfaces/template';
+import type { LabelType } from '~/interfaces/template';
 import { useCurrentUser } from '~/stores/context';
 import { useCurrentUser } from '~/stores/context';
 import { useCurrentPagePath, useSWRxCurrentPage } from '~/stores/page';
 import { useCurrentPagePath, useSWRxCurrentPage } from '~/stores/page';
 
 
@@ -40,7 +40,7 @@ export const PageCreateButton = React.memo((): JSX.Element => {
     : generateTodaysPath(currentUser, t('create_page_dropdown.todays.memo'));
     : generateTodaysPath(currentUser, t('create_page_dropdown.todays.memo'));
 
 
   const { onClickHandler: onClickNewButton, isPageCreating: isNewPageCreating } = useOnNewButtonClicked(
   const { onClickHandler: onClickNewButton, isPageCreating: isNewPageCreating } = useOnNewButtonClicked(
-    currentPage?.path, currentPage?.grant, currentPage?.grantedGroups, isLoading,
+    currentPage?.path, isLoading,
   );
   );
   // TODO: https://redmine.weseek.co.jp/issues/138806
   // TODO: https://redmine.weseek.co.jp/issues/138806
   const { onClickHandler: onClickTodaysButton, isPageCreating: isTodaysPageCreating } = useOnTodaysButtonClicked(todaysPath);
   const { onClickHandler: onClickTodaysButton, isPageCreating: isTodaysPageCreating } = useOnTodaysButtonClicked(todaysPath);

+ 8 - 35
apps/app/src/components/Sidebar/PageCreateButton/hooks.tsx

@@ -1,6 +1,5 @@
 import { useCallback, useState } from 'react';
 import { useCallback, useState } from 'react';
 
 
-import type { PageGrant, IGrantedGroup } from '@growi/core';
 import { useRouter } from 'next/router';
 import { useRouter } from 'next/router';
 
 
 import { createPage, exist } from '~/client/services/page-operation';
 import { createPage, exist } from '~/client/services/page-operation';
@@ -9,8 +8,6 @@ import { EditorMode, useEditorMode } from '~/stores/ui';
 
 
 export const useOnNewButtonClicked = (
 export const useOnNewButtonClicked = (
     currentPagePath?: string,
     currentPagePath?: string,
-    currentPageGrant?: PageGrant,
-    currentPageGrantedGroups?: IGrantedGroup[],
     isLoading?: boolean,
     isLoading?: boolean,
 ): {
 ): {
   onClickHandler: () => Promise<void>,
   onClickHandler: () => Promise<void>,
@@ -27,28 +24,12 @@ export const useOnNewButtonClicked = (
     try {
     try {
       setIsPageCreating(true);
       setIsPageCreating(true);
 
 
-      /**
-       * !! NOTICE !! - Verification of page createable or not is checked on the server side.
-       * since the new page path is not generated on the client side.
-       * need shouldGeneratePath flag.
-       */
-      const shouldUseRootPath = currentPagePath == null || currentPageGrant == null;
-      const parentPath = shouldUseRootPath
-        ? '/'
-        : currentPagePath;
-
-      const params = {
-        isSlackEnabled: false,
-        slackChannels: '',
-        grant: shouldUseRootPath ? 1 : currentPageGrant,
-        grantUserGroupIds: shouldUseRootPath ? undefined : currentPageGrantedGroups,
-        shouldGeneratePath: true,
-      };
-
-      // !! NOTICE !! - if shouldGeneratePath is flagged, send the parent page path
-      const response = await createPage(parentPath, '', params);
-
-      await router.push(`/${response.page.id}#edit`);
+      const response = await createPage({
+        parentPath: currentPagePath,
+        optionalParentPath: '/',
+      });
+
+      await router.push(`/${response.page._id}#edit`);
       mutateEditorMode(EditorMode.Editor);
       mutateEditorMode(EditorMode.Editor);
     }
     }
     catch (err) {
     catch (err) {
@@ -57,7 +38,7 @@ export const useOnNewButtonClicked = (
     finally {
     finally {
       setIsPageCreating(false);
       setIsPageCreating(false);
     }
     }
-  }, [currentPageGrant, currentPageGrantedGroups, currentPagePath, isLoading, mutateEditorMode, router]);
+  }, [currentPagePath, isLoading, mutateEditorMode, router]);
 
 
   return { onClickHandler, isPageCreating };
   return { onClickHandler, isPageCreating };
 };
 };
@@ -81,17 +62,9 @@ export const useOnTodaysButtonClicked = (
     try {
     try {
       setIsPageCreating(true);
       setIsPageCreating(true);
 
 
-      // TODO: get grant, grantUserGroupId data from parent page
-      // https://redmine.weseek.co.jp/issues/133892
-      const params = {
-        isSlackEnabled: false,
-        slackChannels: '',
-        grant: 4,
-      };
-
       const res = await exist(JSON.stringify([todaysPath]));
       const res = await exist(JSON.stringify([todaysPath]));
       if (!res.pages[todaysPath]) {
       if (!res.pages[todaysPath]) {
-        await createPage(todaysPath, '', params);
+        await createPage({ path: todaysPath });
       }
       }
 
 
       await router.push(`${todaysPath}#edit`);
       await router.push(`${todaysPath}#edit`);

+ 26 - 2
apps/app/src/interfaces/page-operation.ts

@@ -1,4 +1,6 @@
-import type { IGrantedGroup } from '@growi/core';
+import type {
+  IGrantedGroup, IPageHasId, IRevisionHasId, ITag, PageGrant,
+} from '@growi/core';
 
 
 export const PageActionType = {
 export const PageActionType = {
   Create: 'Create',
   Create: 'Create',
@@ -32,7 +34,29 @@ export type IPageOperationProcessInfo = {
 export type OptionsToSave = {
 export type OptionsToSave = {
   isSlackEnabled: boolean;
   isSlackEnabled: boolean;
   slackChannels: string;
   slackChannels: string;
-  grant: number;
+  grant: PageGrant;
   // userRelatedGrantUserGroupIds?: IGrantedGroup[];
   // userRelatedGrantUserGroupIds?: IGrantedGroup[];
   // isSyncRevisionToHackmd?: boolean;
   // isSyncRevisionToHackmd?: boolean;
 };
 };
+
+export type IApiv3PageCreateParams = {
+  path?: string,
+  parentPath?: string,
+  optionalParentPath?: string,
+
+  body?: string,
+  pageTags?: string[],
+
+  grant?: PageGrant,
+  grantUserGroupIds?: IGrantedGroup[],
+  overwriteScopesOfDescendants?: boolean,
+
+  isSlackEnabled?: boolean,
+  slackChannels?: string,
+};
+
+export type IApiv3PageCreateResponse = {
+  page: IPageHasId,
+  tags: ITag[],
+  revision: IRevisionHasId,
+};

+ 73 - 53
apps/app/src/server/routes/apiv3/page/create-page.ts

@@ -4,7 +4,7 @@ import type {
 } from '@growi/core';
 } from '@growi/core';
 import { ErrorV3 } from '@growi/core/dist/models';
 import { ErrorV3 } from '@growi/core/dist/models';
 import { isCreatablePage, isUserPage } from '@growi/core/dist/utils/page-path-utils';
 import { isCreatablePage, isUserPage } from '@growi/core/dist/utils/page-path-utils';
-import { addHeadingSlash, attachTitleHeader } from '@growi/core/dist/utils/path-utils';
+import { attachTitleHeader, normalizePath } from '@growi/core/dist/utils/path-utils';
 import type { Request, RequestHandler } from 'express';
 import type { Request, RequestHandler } from 'express';
 import type { ValidationChain } from 'express-validator';
 import type { ValidationChain } from 'express-validator';
 import { body } from 'express-validator';
 import { body } from 'express-validator';
@@ -31,29 +31,65 @@ import type { ApiV3Response } from '../interfaces/apiv3-response';
 const logger = loggerFactory('growi:routes:apiv3:page:create-page');
 const logger = loggerFactory('growi:routes:apiv3:page:create-page');
 
 
 
 
-async function generateUniquePath(basePath: string, index = 1): Promise<string> {
+async function generateUntitledPath(parentPath: string, basePathname: string, index = 1): Promise<string> {
   const Page = mongoose.model<IPage>('Page');
   const Page = mongoose.model<IPage>('Page');
 
 
-  const path = basePath + index;
-  const existingPageId = await Page.exists({ path, isEmpty: false });
-  if (existingPageId != null) {
-    return generateUniquePath(basePath, index + 1);
+  const path = `${normalizePath(parentPath)}${normalizePath(basePathname)}-${index}`;
+  if (await Page.exists({ path, isEmpty: false }) != null) {
+    return generateUntitledPath(parentPath, basePathname, index + 1);
   }
   }
   return path;
   return path;
 }
 }
 
 
+async function determinePath(parentPath?: string, path?: string, optionalParentPath?: string): Promise<string> {
+  // TODO: i18n
+  const basePathname = 'Untitled';
+
+  if (path != null) {
+    // when path is valid
+    if (isCreatablePage(path)) {
+      return normalizePath(path);
+    }
+    // when optionalParentPath is set
+    if (optionalParentPath != null) {
+      return generateUntitledPath(optionalParentPath, basePathname);
+    }
+    // when path is invalid
+    throw new Error('Could not create the page');
+  }
+
+  if (parentPath != null) {
+    // when parentPath is valid
+    if (isCreatablePage(parentPath)) {
+      return generateUntitledPath(parentPath, basePathname);
+    }
+    // when optionalParentPath is set
+    if (optionalParentPath != null) {
+      return generateUntitledPath(optionalParentPath, basePathname);
+    }
+    // when parentPath is invalid
+    throw new Error('Could not create the page');
+  }
+
+  // when both path and parentPath are not specified
+  return generateUntitledPath('/', basePathname);
+}
+
+
 type ReqBody = {
 type ReqBody = {
-  path: string,
+  path?: string,
+  parentPath?: string,
+  optionalParentPath?: string,
+
+  body?: string,
+  pageTags?: string[],
 
 
   grant?: PageGrant,
   grant?: PageGrant,
   grantUserGroupIds?: IGrantedGroup[],
   grantUserGroupIds?: IGrantedGroup[],
-
-  body?: string,
   overwriteScopesOfDescendants?: boolean,
   overwriteScopesOfDescendants?: boolean,
+
   isSlackEnabled?: boolean,
   isSlackEnabled?: boolean,
   slackChannels?: any,
   slackChannels?: any,
-  pageTags?: string[],
-  shouldGeneratePath?: boolean,
 }
 }
 
 
 interface CreatePageRequest extends Request<undefined, ApiV3Response, ReqBody> {
 interface CreatePageRequest extends Request<undefined, ApiV3Response, ReqBody> {
@@ -72,16 +108,20 @@ export const createPageHandlersFactory: CreatePageHandlersFactory = (crowi) => {
 
 
   // define validators for req.body
   // define validators for req.body
   const validator: ValidationChain[] = [
   const validator: ValidationChain[] = [
+    body('path').optional().not().isEmpty({ ignore_whitespace: true })
+      .withMessage("The empty value is not allowd for the 'path'"),
+    body('parentPath').optional().not().isEmpty({ ignore_whitespace: true })
+      .withMessage("The empty value is not allowd for the 'parentPath'"),
+    body('optionalParentPath').optional().not().isEmpty({ ignore_whitespace: true })
+      .withMessage("The empty value is not allowd for the 'optionalParentPath'"),
     body('body').optional().isString()
     body('body').optional().isString()
       .withMessage('body must be string or undefined'),
       .withMessage('body must be string or undefined'),
-    body('path').exists().not().isEmpty({ ignore_whitespace: true })
-      .withMessage('path is required'),
     body('grant').optional().isInt({ min: 0, max: 5 }).withMessage('grant must be integer from 1 to 5'),
     body('grant').optional().isInt({ min: 0, max: 5 }).withMessage('grant must be integer from 1 to 5'),
     body('overwriteScopesOfDescendants').optional().isBoolean().withMessage('overwriteScopesOfDescendants must be boolean'),
     body('overwriteScopesOfDescendants').optional().isBoolean().withMessage('overwriteScopesOfDescendants must be boolean'),
+    body('pageTags').optional().isArray().withMessage('pageTags must be array'),
     body('isSlackEnabled').optional().isBoolean().withMessage('isSlackEnabled must be boolean'),
     body('isSlackEnabled').optional().isBoolean().withMessage('isSlackEnabled must be boolean'),
     body('slackChannels').optional().isString().withMessage('slackChannels must be string'),
     body('slackChannels').optional().isString().withMessage('slackChannels must be string'),
-    body('pageTags').optional().isArray().withMessage('pageTags must be array'),
-    body('shouldGeneratePath').optional().isBoolean().withMessage('shouldGeneratePath is must be boolean or undefined'),
+    // body('shouldGeneratePath').optional().isBoolean().withMessage('shouldGeneratePath is must be boolean or undefined'),
   ];
   ];
 
 
 
 
@@ -142,60 +182,34 @@ export const createPageHandlersFactory: CreatePageHandlersFactory = (crowi) => {
     validator, apiV3FormValidator,
     validator, apiV3FormValidator,
     async(req: CreatePageRequest, res: ApiV3Response) => {
     async(req: CreatePageRequest, res: ApiV3Response) => {
       const {
       const {
-        body, overwriteScopesOfDescendants, pageTags, shouldGeneratePath,
+        body, pageTags,
       } = req.body;
       } = req.body;
 
 
-      let { path, grant, grantUserGroupIds } = req.body;
-
-      // check whether path starts slash
-      path = addHeadingSlash(path);
-
-      if (shouldGeneratePath) {
-        try {
-          const rootPath = '/';
-          const defaultTitle = '/Untitled';
-          const basePath = path === rootPath ? defaultTitle : path + defaultTitle;
-          path = await generateUniquePath(basePath);
-
-          // if the generated path is not creatable, create the path under the root path
-          if (!isCreatablePage(path)) {
-            path = await generateUniquePath(defaultTitle);
-            // initialize grant data
-            grant = 1;
-            grantUserGroupIds = undefined;
-          }
-        }
-        catch (err) {
-          return res.apiv3Err(new ErrorV3('Failed to generate unique path'));
-        }
+      let pathToCreate: string;
+      try {
+        const { path, parentPath, optionalParentPath } = req.body;
+        pathToCreate = await determinePath(parentPath, path, optionalParentPath);
       }
       }
-
-      if (!isCreatablePage(path)) {
-        return res.apiv3Err(`Could not use the path '${path}'`);
+      catch (err) {
+        return res.apiv3Err(new ErrorV3('Could not create the page.', 'could_not_create_page'));
       }
       }
 
 
-      if (isUserPage(path)) {
-        const isExistUser = await User.isExistUserByUserPagePath(path);
+      if (isUserPage(pathToCreate)) {
+        const isExistUser = await User.isExistUserByUserPagePath(pathToCreate);
         if (!isExistUser) {
         if (!isExistUser) {
           return res.apiv3Err("Unable to create a page under a non-existent user's user page");
           return res.apiv3Err("Unable to create a page under a non-existent user's user page");
         }
         }
       }
       }
 
 
-      const options: IOptionsForCreate = { overwriteScopesOfDescendants };
-      if (grant != null) {
-        options.grant = grant;
-        options.grantUserGroupIds = grantUserGroupIds;
-      }
-
       let initialTags: string[] = pageTags ?? [];
       let initialTags: string[] = pageTags ?? [];
       let initialBody = body ?? '';
       let initialBody = body ?? '';
       if (body == null) {
       if (body == null) {
         const isEnabledAttachTitleHeader = await configManager.getConfig('crowi', 'customize:isEnabledAttachTitleHeader');
         const isEnabledAttachTitleHeader = await configManager.getConfig('crowi', 'customize:isEnabledAttachTitleHeader');
         if (isEnabledAttachTitleHeader) {
         if (isEnabledAttachTitleHeader) {
-          initialBody += `${attachTitleHeader(path)}\n`;
+          initialBody += `${attachTitleHeader(pathToCreate)}\n`;
         }
         }
 
 
-        const templateData = await Page.findTemplate(path);
+        const templateData = await Page.findTemplate(pathToCreate);
         if (templateData.templateTags != null) {
         if (templateData.templateTags != null) {
           initialTags = templateData.templateTags;
           initialTags = templateData.templateTags;
         }
         }
@@ -206,8 +220,14 @@ export const createPageHandlersFactory: CreatePageHandlersFactory = (crowi) => {
 
 
       let createdPage;
       let createdPage;
       try {
       try {
+        const { grant, grantUserGroupIds, overwriteScopesOfDescendants } = req.body;
+        const options: IOptionsForCreate = { overwriteScopesOfDescendants };
+        if (grant != null) {
+          options.grant = grant;
+          options.grantUserGroupIds = grantUserGroupIds;
+        }
         createdPage = await crowi.pageService.create(
         createdPage = await crowi.pageService.create(
-          path,
+          pathToCreate,
           initialBody,
           initialBody,
           req.user,
           req.user,
           options,
           options,