Merge pull request #5384 from weseek/support/refactor-findPageAndMetaDataByViewer

support: refactor page-path-utils and PageService.findPageAndMetaDataByViewer

packages/app/src/components/PageEditor/LinkEditModal.jsx

@@ -202,8 +202,9 @@ class LinkEditModal extends React.PureComponent {
   }
 
   handleChangeTypeahead(selected) {
-    const page = selected[0];
-    if (page != null) {
+    const pageWithMeta = selected[0];
+    if (pageWithMeta != null) {
+      const page = pageWithMeta.pageData;
       const permalink = `${window.location.origin}/${page.id}`;
       this.setState({ linkInputValue: page.path, permalink });
     }

packages/app/src/components/SearchTypeahead.tsx

@@ -177,7 +177,7 @@ const SearchTypeahead: ForwardRefRenderFunction<IFocusable, Props> = (props: Pro
       return emptyLabel;
     }
 
-    return false;
+    return <></>;
   };
 
   const defaultSelected = (keywordOnInit !== '')

packages/app/src/server/models/obsolete-page.js

@@ -15,7 +15,7 @@ const differenceInYears = require('date-fns/differenceInYears');
 const { pathUtils } = require('@growi/core');
 const escapeStringRegexp = require('escape-string-regexp');
 
-const { isTopPage, isTrashPage, isUserNamePage } = pagePathUtils;
+const { isTopPage, isTrashPage } = pagePathUtils;
 const { checkTemplatePath } = templateChecker;
 
 const logger = loggerFactory('growi:models:page');
@@ -612,10 +612,6 @@ export const getPageSchema = (crowi) => {
     return path.replace('/trash', '');
   };
 
-  pageSchema.statics.isDeletableName = function(path) {
-    return !isTopPage(path) && !isUserNamePage(path);
-  };
-
   pageSchema.statics.fixToCreatableName = function(path) {
     return path
       .replace(/\/\//g, '/');

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

@@ -22,7 +22,7 @@ export interface ISubscription {
 export interface SubscriptionDocument extends ISubscription, Document {}
 
 export interface SubscriptionModel extends Model<SubscriptionDocument> {
-  findByUserIdAndTargetId(userId: Types.ObjectId, targetId: Types.ObjectId): any
+  findByUserIdAndTargetId(userId: Types.ObjectId | string, targetId: Types.ObjectId | string): any
   upsertSubscription(user: Types.ObjectId, targetModel: string, target: Types.ObjectId, status: string): any
   subscribeByPageId(user: Types.ObjectId, pageId: Types.ObjectId, status: string): any
   getSubscription(target: Types.ObjectId): Promise<Types.ObjectId[]>

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

@@ -239,33 +239,36 @@ module.exports = (crowi) => {
       return res.apiv3Err(new ErrorV3('Parameter path or pageId is required.', 'invalid-request'));
     }
 
-    let result = {};
+    let page;
     try {
-      result = await pageService.findPageAndMetaDataByViewer({ pageId, path, user: req.user });
+      if (pageId != null) { // prioritized
+        page = await Page.findByIdAndViewer(pageId, req.user);
+      }
+      else {
+        page = await Page.findByPathAndViewer(path, req.user);
+      }
     }
     catch (err) {
       logger.error('get-page-failed', err);
       return res.apiv3Err(err, 500);
     }
 
-    const page = result.page;
-
     if (page == null) {
-      return res.apiv3(result);
+      return res.apiv3Err('Page is not found', 404);
     }
 
     try {
       page.initLatestRevisionField();
 
       // populate
-      result.page = await page.populateDataToShowRevision();
+      page = await page.populateDataToShowRevision();
     }
     catch (err) {
       logger.error('populate-page-failed', err);
       return res.apiv3Err(err, 500);
     }
 
-    return res.apiv3(result);
+    return res.apiv3({ page });
   });
 
   /**
@@ -360,51 +363,13 @@ module.exports = (crowi) => {
     const { pageId } = req.query;
 
     try {
-      const page = await Page.findByIdAndViewer(pageId, user, null, true);
+      const pageWithMeta = await pageService.findPageAndMetaDataByViewer(pageId, null, user, isSharedPage);
 
-      if (page == null) {
+      if (pageWithMeta == null) {
         return res.apiv3Err(`Page '${pageId}' is not found or forbidden`);
       }
 
-      if (isSharedPage) {
-        return {
-          isEmpty: page.isEmpty,
-          isMovable: false,
-          isDeletable: false,
-          isAbleToDeleteCompletely: false,
-          isRevertible: false,
-        };
-      }
-
-      const isGuestUser = !req.user;
-      const pageInfo = pageService.constructBasicPageInfo(page, isGuestUser);
-
-      const bookmarkCount = await Bookmark.countByPageId(pageId);
-
-      const responseBodyForGuest = {
-        ...pageInfo,
-        bookmarkCount,
-      };
-
-      if (isGuestUser) {
-        return res.apiv3(responseBodyForGuest);
-      }
-
-      const isBookmarked = await Bookmark.findByPageIdAndUserId(pageId, user._id);
-      const isLiked = page.isLiked(user);
-      const isAbleToDeleteCompletely = pageService.canDeleteCompletely(page.creator?._id, user);
-
-      const subscription = await Subscription.findByUserIdAndTargetId(user._id, pageId);
-
-      const responseBody = {
-        ...responseBodyForGuest,
-        isAbleToDeleteCompletely,
-        isBookmarked,
-        isLiked,
-        subscriptionStatus: subscription?.status,
-      };
-
-      return res.apiv3(responseBody);
+      return res.apiv3(pageWithMeta.pageMeta);
     }
     catch (err) {
       logger.error('get-page-info', err);

packages/app/src/server/routes/page.js

@@ -6,9 +6,8 @@ import mongoose from 'mongoose';
 import loggerFactory from '~/utils/logger';
 import { PageQueryBuilder } from '../models/obsolete-page';
 import UpdatePost from '../models/update-post';
-import { PageRedirectModel } from '../models/page-redirect';
 
-const { isCreatablePage, isTopPage } = pagePathUtils;
+const { isCreatablePage, isTopPage, isUsersHomePage } = pagePathUtils;
 const { serializePageSecurely } = require('../models/serializers/page-serializer');
 const { serializeRevisionSecurely } = require('../models/serializers/revision-serializer');
 const { serializeUserSecurely } = require('../models/serializers/user-serializer');
@@ -142,7 +141,6 @@ module.exports = function(crowi, app) {
 
   const Page = crowi.model('Page');
   const User = crowi.model('User');
-  const Bookmark = crowi.model('Bookmark');
   const PageTagRelation = crowi.model('PageTagRelation');
   const GlobalNotificationSetting = crowi.model('GlobalNotificationSetting');
   const ShareLink = crowi.model('ShareLink');
@@ -173,14 +171,6 @@ module.exports = function(crowi, app) {
     return pathUtils.normalizePath(req.pagePath || req.params[0] || '');
   }
 
-  function isUserPage(path) {
-    if (path.match(/^\/user\/[^/]+\/?$/)) {
-      return true;
-    }
-
-    return false;
-  }
-
   function generatePager(offset, limit, totalCount) {
     let prev = null;
 
@@ -450,7 +440,7 @@ module.exports = function(crowi, app) {
     const sharelinksNumber = await ShareLink.countDocuments({ relatedPage: page._id });
     renderVars.sharelinksNumber = sharelinksNumber;
 
-    if (isUserPage(path)) {
+    if (isUsersHomePage(path)) {
       // change template
       view = 'layout-growi/user_page';
       await addRenderVarsForUserPage(renderVars, page);

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

@@ -5,28 +5,29 @@ import streamToPromise from 'stream-to-promise';
 import pathlib from 'path';
 import { Readable, Writable } from 'stream';
 
-import { serializePageSecurely } from '../models/serializers/page-serializer';
+import { HasObjectId } from '~/interfaces/has-object-id';
+import { Ref } from '~/interfaces/common';
 import { createBatchStream } from '~/server/util/batch-stream';
 import loggerFactory from '~/utils/logger';
 import {
   CreateMethod, generateGrantCondition, PageCreateOptions, PageDocument, PageModel,
 } from '~/server/models/page';
 import { stringifySnapshot } from '~/models/serializers/in-app-notification-snapshot/page';
-import ActivityDefine from '../util/activityDefine';
 import {
-  IPage, IPageInfo, IPageInfoForEntity,
+  IPage, IPageInfo, IPageInfoForEntity, IPageWithMeta,
 } from '~/interfaces/page';
+import { serializePageSecurely } from '../models/serializers/page-serializer';
 import { PageRedirectModel } from '../models/page-redirect';
+import Subscription from '../models/subscription';
 import { ObjectIdLike } from '../interfaces/mongoose-utils';
 import { IUserHasId } from '~/interfaces/user';
-import { Ref } from '~/interfaces/common';
-import { HasObjectId } from '~/interfaces/has-object-id';
+import ActivityDefine from '../util/activityDefine';
 
 const debug = require('debug')('growi:services:page');
 
 const logger = loggerFactory('growi:services:page');
 const {
-  isCreatablePage, isTrashPage, isTopPage, isDeletablePage, omitDuplicateAreaPathFromPaths, omitDuplicateAreaPageFromPages, isUserPage, isUserNamePage,
+  isTrashPage, isTopPage, omitDuplicateAreaPageFromPages, isMovablePage,
 } = pagePathUtils;
 
 const BULK_REINDEX_SIZE = 100;
@@ -213,13 +214,14 @@ class PageService {
     return pages.filter(p => p.isEmpty || this.canDeleteCompletely(p.creator, user));
   }
 
-  async findPageAndMetaDataByViewer({ pageId, path, user }) {
+  // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
+  async findPageAndMetaDataByViewer(pageId: string, path: string, user: IUserHasId, isSharedPage = false): Promise<IPageWithMeta|null> {
 
     const Page = this.crowi.model('Page');
 
     let pagePath = path;
 
-    let page;
+    let page: PageModel & PageDocument & HasObjectId;
     if (pageId != null) { // prioritized
       page = await Page.findByIdAndViewer(pageId, user);
       pagePath = page.path;
@@ -228,26 +230,57 @@ class PageService {
       page = await Page.findByPathAndViewer(pagePath, user);
     }
 
-    const result: any = {};
-
     if (page == null) {
-      const isExist = await Page.count({ $or: [{ _id: pageId }, { pat: pagePath }] }) > 0;
-      result.isForbidden = isExist;
-      result.isNotFound = !isExist;
-      result.isCreatable = isCreatablePage(pagePath);
-      result.page = page;
+      return null;
+    }
 
-      return result;
+    if (isSharedPage) {
+      return {
+        pageData: page,
+        pageMeta: {
+          isEmpty: page.isEmpty,
+          isMovable: false,
+          isDeletable: false,
+          isAbleToDeleteCompletely: false,
+          isRevertible: false,
+        },
+      };
     }
 
-    result.page = page;
-    result.isForbidden = false;
-    result.isNotFound = false;
-    result.isCreatable = false;
-    result.isDeletable = isDeletablePage(pagePath);
-    result.isDeleted = page.isDeleted();
+    const isGuestUser = user == null;
+    const pageInfo = this.constructBasicPageInfo(page, isGuestUser);
 
-    return result;
+    const Bookmark = this.crowi.model('Bookmark');
+    const bookmarkCount = await Bookmark.countByPageId(pageId);
+
+    const metadataForGuest = {
+      ...pageInfo,
+      bookmarkCount,
+    };
+
+    if (isGuestUser) {
+      return {
+        pageData: page,
+        pageMeta: metadataForGuest,
+      };
+    }
+
+    const isBookmarked = await Bookmark.findByPageIdAndUserId(pageId, user._id);
+    const isLiked = page.isLiked(user);
+    const isAbleToDeleteCompletely = this.canDeleteCompletely((page.creator as IUserHasId)?._id, user);
+
+    const subscription = await Subscription.findByUserIdAndTargetId(user._id, pageId);
+
+    return {
+      pageData: page,
+      pageMeta: {
+        ...metadataForGuest,
+        isAbleToDeleteCompletely,
+        isBookmarked,
+        isLiked,
+        subscriptionStatus: subscription?.status,
+      },
+    };
   }
 
   private shouldUseV4Process(page): boolean {
@@ -1068,7 +1101,7 @@ class PageService {
       throw new Error('This method does NOT support deleting trashed pages.');
     }
 
-    if (!Page.isDeletableName(page.path)) {
+    if (!isMovablePage(page.path)) {
       throw new Error('Page is not deletable.');
     }
 
@@ -1144,7 +1177,7 @@ class PageService {
       throw new Error('This method does NOT support deleting trashed pages.');
     }
 
-    if (!Page.isDeletableName(page.path)) {
+    if (!isMovablePage(page.path)) {
       throw new Error('Page is not deletable.');
     }
 
@@ -1718,7 +1751,7 @@ class PageService {
   }
 
   constructBasicPageInfo(page: IPage, isGuestUser?: boolean): IPageInfo | IPageInfoForEntity {
-    const isMovable = isGuestUser ? false : !isTopPage(page.path) && !isUserPage(page.path) && !isUserNamePage(page.path);
+    const isMovable = isGuestUser ? false : isMovablePage(page.path);
 
     if (page.isEmpty) {
       return {
@@ -1733,8 +1766,6 @@ class PageService {
     const likers = page.liker.slice(0, 15) as Ref<IUserHasId>[];
     const seenUsers = page.seenUsers.slice(0, 15) as Ref<IUserHasId>[];
 
-    const Page = this.crowi.model('Page');
-    const isRevertible = isTrashPage(page.path);
     return {
       isEmpty: false,
       sumOfLikers: page.liker.length,
@@ -1742,9 +1773,9 @@ class PageService {
       seenUserIds: this.extractStringIds(seenUsers),
       sumOfSeenUsers: page.seenUsers.length,
       isMovable,
-      isDeletable: Page.isDeletableName(page.path),
+      isDeletable: isMovable,
       isAbleToDeleteCompletely: false,
-      isRevertible,
+      isRevertible: isTrashPage(page.path),
     };
 
   }

packages/app/test/integration/models/page.test.js

@@ -156,17 +156,6 @@ describe('Page', () => {
     });
   });
 
-  describe('.isDeletableName', () => {
-    test('should decide deletable or not', () => {
-      expect(Page.isDeletableName('/')).toBeFalsy();
-      expect(Page.isDeletableName('/hoge')).toBeTruthy();
-      expect(Page.isDeletableName('/user/xxx')).toBeFalsy();
-      expect(Page.isDeletableName('/user/xxx123')).toBeFalsy();
-      expect(Page.isDeletableName('/user/xxx/')).toBeTruthy();
-      expect(Page.isDeletableName('/user/xxx/hoge')).toBeTruthy();
-    });
-  });
-
   describe('.isAccessiblePageByViewer', () => {
     describe('with a granted page', () => {
       test('should return true with granted user', async() => {

packages/core/src/test/util/page-path-utils.test.js

@@ -1,5 +1,5 @@
 import {
-  isTopPage, convertToNewAffiliationPath, isCreatablePage, omitDuplicateAreaPathFromPaths,
+  isTopPage, isMovablePage, convertToNewAffiliationPath, isCreatablePage, omitDuplicateAreaPathFromPaths,
 } from '~/utils/page-path-utils';
 
 describe('TopPage Path test', () => {
@@ -21,6 +21,16 @@ describe('TopPage Path test', () => {
   });
 });
 
+describe('isMovablePage test', () => {
+  test('should decide deletable or not', () => {
+    expect(isMovablePage('/')).toBeFalsy();
+    expect(isMovablePage('/hoge')).toBeTruthy();
+    expect(isMovablePage('/user')).toBeFalsy();
+    expect(isMovablePage('/user/xxx')).toBeFalsy();
+    expect(isMovablePage('/user/xxx123')).toBeFalsy();
+    expect(isMovablePage('/user/xxx/hoge')).toBeTruthy();
+  });
+});
 
 describe('convertToNewAffiliationPath test', () => {
   test('Child path is not converted normally', () => {

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

@@ -11,38 +11,48 @@ export const isTopPage = (path: string): boolean => {
 };
 
 /**
- * Whether path belongs to the trash page
+ * Whether path is the top page of users
  * @param path
  */
-export const isTrashPage = (path: string): boolean => {
-  // https://regex101.com/r/BSDdRr/1
-  if (path.match(/^\/trash(\/.*)?$/)) {
+export const isUsersTopPage = (path: string): boolean => {
+  return path === '/user';
+};
+
+/**
+ * Whether path is user's home page
+ * @param path
+ */
+export const isUsersHomePage = (path: string): boolean => {
+  // https://regex101.com/r/utVQct/1
+  if (path.match(/^\/user\/[^/]+$/)) {
     return true;
   }
-
   return false;
 };
 
 /**
- * Whether path belongs to the user page
+ * Whether path is the protected pages for systems
  * @param path
  */
-export const isUserPage = (path: string): boolean => {
-  // https://regex101.com/r/SxPejV/1
-  if (path.match(/^\/user(\/.*)?$/)) {
-    return true;
-  }
+export const isUsersProtectedPages = (path: string): boolean => {
+  return isUsersTopPage(path) || isUsersHomePage(path);
+};
 
-  return false;
+/**
+ * Whether path is movable
+ * @param path
+ */
+export const isMovablePage = (path: string): boolean => {
+  return !isTopPage(path) && !isUsersProtectedPages(path);
 };
 
 /**
- * Whether path is right under the path '/user'
+ * Whether path belongs to the trash page
  * @param path
  */
-export const isUserNamePage = (path: string): boolean => {
-  // https://regex101.com/r/GUZntH/1
-  if (path.match(/^\/user\/[^/]+$/)) {
+export const isTrashPage = (path: string): boolean => {
+  // https://regex101.com/r/BSDdRr/1
+  if (path.match(/^\/trash(\/.*)?$/)) {
     return true;
   }
 
@@ -62,13 +72,6 @@ export const isSharedPage = (path: string): boolean => {
   return false;
 };
 
-const restrictedPatternsToDelete: Array<RegExp> = [
-  /^\/user\/[^/]+$/, // user page
-];
-export const isDeletablePage = (path: string): boolean => {
-  return !restrictedPatternsToDelete.some(pattern => path.match(pattern));
-};
-
 const restrictedPatternsToCreate: Array<RegExp> = [
   /\^|\$|\*|\+|#|%|\?/,
   /^\/-\/.*/,