Merge pull request #5231 from weseek/feat/page-redirect-route

feat: Page redirect route

packages/app/src/interfaces/page.ts

@@ -17,7 +17,6 @@ export interface IPage {
   parent: Ref<IPage> | null,
   descendantCount: number,
   isEmpty: boolean,
-  redirectTo: string,
   grant: number,
   grantedUsers: Ref<IUser>[],
   grantedGroup: Ref<any>,

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

@@ -104,11 +104,6 @@ export class PageQueryBuilder {
     return this;
   }
 
-  addConditionToExcludeRedirect() {
-    this.query = this.query.and({ redirectTo: null });
-    return this;
-  }
-
   /**
    * generate the query to find the pages '{path}/*' and '{path}' self.
    * If top page, return without doing anything.
@@ -690,10 +685,6 @@ export const getPageSchema = (crowi) => {
     return queryBuilder.query.exec();
   };
 
-  pageSchema.statics.findByRedirectTo = function(path) {
-    return this.findOne({ redirectTo: path });
-  };
-
   /**
    * find pages that is match with `path` and its descendants
    */
@@ -714,7 +705,6 @@ export const getPageSchema = (crowi) => {
 
     const builder = new PageQueryBuilder(this.find(), includeEmpty);
     builder.addConditionToListWithDescendants(page.path, option);
-    builder.addConditionToExcludeRedirect();
 
     // add grant conditions
     await addConditionToFilteringByViewerToEdit(builder, user);
@@ -765,9 +755,6 @@ export const getPageSchema = (crowi) => {
     const opt = Object.assign({}, option);
     const builder = new PageQueryBuilder(this.find({ _id: { $in: ids } }));
 
-    if (excludeRedirect) {
-      builder.addConditionToExcludeRedirect();
-    }
     builder.addConditionToPagenate(opt.offset, opt.limit);
 
     // count
@@ -804,10 +791,6 @@ export const getPageSchema = (crowi) => {
     if (!opt.includeTrashed) {
       builder.addConditionToExcludeTrashed();
     }
-    // exclude redirect pages
-    if (!opt.includeRedirect) {
-      builder.addConditionToExcludeRedirect();
-    }
 
     // add grant conditions
     await addConditionToFilteringByViewerForList(builder, user, showAnyoneKnowsLink);
@@ -1009,7 +992,6 @@ export const getPageSchema = (crowi) => {
     const Page = this;
     const Revision = crowi.model('Revision');
     const format = options.format || 'markdown';
-    const redirectTo = options.redirectTo || null;
     const grantUserGroupId = options.grantUserGroupId || null;
 
     // sanitize path
@@ -1031,7 +1013,6 @@ export const getPageSchema = (crowi) => {
     page.path = path;
     page.creator = user;
     page.lastUpdateUser = user;
-    page.redirectTo = redirectTo;
     page.status = STATUS_PUBLISHED;
 
     await validateAppliedScope(user, grant, grantUserGroupId);
@@ -1077,8 +1058,6 @@ export const getPageSchema = (crowi) => {
     const builder = new PageQueryBuilder(this.find());
     builder.addConditionToListWithDescendants(parentPage.path);
 
-    builder.addConditionToExcludeRedirect();
-
     // add grant conditions
     await addConditionToFilteringByViewerToEdit(builder, user);
 
@@ -1103,29 +1082,6 @@ export const getPageSchema = (crowi) => {
     return this.findOneAndRemove({ path }).exec();
   };
 
-  /**
-   * remove the page that is redirecting to specified `pagePath` recursively
-   *  ex: when
-   *    '/page1' redirects to '/page2' and
-   *    '/page2' redirects to '/page3'
-   *    and given '/page3',
-   *    '/page1' and '/page2' will be removed
-   *
-   * @param {string} pagePath
-   */
-  pageSchema.statics.removeRedirectOriginPageByPath = async function(pagePath) {
-    const redirectPage = await this.findByRedirectTo(pagePath);
-
-    if (redirectPage == null) {
-      return;
-    }
-
-    // remove
-    await this.findByIdAndRemove(redirectPage.id);
-    // remove recursive
-    await this.removeRedirectOriginPageByPath(redirectPage.path);
-  };
-
   pageSchema.statics.findListByPathsArray = async function(paths, includeEmpty = false) {
     const queryBuilder = new PageQueryBuilder(this.find(), includeEmpty);
     queryBuilder.addConditionToListByPathsArray(paths);

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

@@ -74,7 +74,6 @@ const schema = new Schema<PageDocument, PageModel>({
     type: String, required: true, index: true,
   },
   revision: { type: ObjectId, ref: 'Revision' },
-  redirectTo: { type: String, index: true },
   status: { type: String, default: STATUS_PUBLISHED, index: true },
   grant: { type: Number, default: GRANT_PUBLIC, index: true },
   grantedUsers: [{ type: ObjectId, ref: 'User' }],

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

@@ -74,10 +74,6 @@ const ErrorV3 = require('../../models/vo/error-apiv3');
  *            type: string
  *            description: page path
  *            example: /
- *          redirectTo:
- *            type: string
- *            description: redirect path
- *            example: ""
  *          revision:
  *            type: string
  *            description: page revision

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

@@ -110,10 +110,6 @@ const LIMIT_FOR_LIST = 10;
  *            type: string
  *            description: page path
  *            example: /Sandbox/Math
- *          redirectTo:
- *            type: string
- *            description: redirect path
- *            example: ""
  *          revision:
  *            type: string
  *            description: revision ID

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

@@ -1,9 +1,11 @@
 import { pagePathUtils } from '@growi/core';
 import urljoin from 'url-join';
 import { body } from 'express-validator';
+import mongoose from 'mongoose';
 
 import loggerFactory from '~/utils/logger';
 import UpdatePost from '../models/update-post';
+import { PageRedirectModel } from '../models/page-redirect';
 
 const { isCreatablePage, isTopPage } = pagePathUtils;
 const { serializePageSecurely } = require('../models/serializers/page-serializer');
@@ -71,10 +73,6 @@ const { serializeUserSecurely } = require('../models/serializers/user-serializer
  *            type: string
  *            description: page path
  *            example: /
- *          redirectTo:
- *            type: string
- *            description: redirect path
- *            example: ""
  *          revision:
  *            $ref: '#/components/schemas/Revision'
  *          status:
@@ -147,6 +145,7 @@ module.exports = function(crowi, app) {
   const PageTagRelation = crowi.model('PageTagRelation');
   const GlobalNotificationSetting = crowi.model('GlobalNotificationSetting');
   const ShareLink = crowi.model('ShareLink');
+  const PageRedirect = mongoose.model('PageRedirect');
 
   const ApiResponse = require('../util/apiResponse');
   const getToday = require('../util/getToday');
@@ -443,11 +442,6 @@ module.exports = function(crowi, app) {
 
     const { path } = page; // this must exist
 
-    if (page.redirectTo) {
-      debug(`Redirect to '${page.redirectTo}'`);
-      return res.redirect(`${encodeURI(page.redirectTo)}?redirectFrom=${encodeURIComponent(path)}`);
-    }
-
     logger.debug('Page is found when processing pageShowForGrowiBehavior', page._id, path);
 
     const limit = 50;
@@ -624,6 +618,7 @@ module.exports = function(crowi, app) {
    */
   async function redirector(req, res, next, path) {
     const pages = await Page.findByPathAndViewer(path, req.user, null, false, true);
+
     const { redirectFrom } = req.query;
 
     if (pages.length >= 2) {
@@ -652,7 +647,18 @@ module.exports = function(crowi, app) {
       return res.safeRedirect(urljoin(url.pathname, url.search));
     }
 
-    req.isForbidden = await Page.count({ path }) > 0;
+    const isForbidden = await Page.exists({ path });
+    if (isForbidden) {
+      req.isForbidden = true;
+      return _notFound(req, res);
+    }
+
+    // redirect by PageRedirect
+    const pageRedirect = await PageRedirect.findOne({ fromPath: path });
+    if (pageRedirect != null) {
+      return res.safeRedirect(`${encodeURI(pageRedirect.toPath)}?redirectFrom=${encodeURIComponent(path)}`);
+    }
+
     return _notFound(req, res);
   }
 

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

@@ -278,7 +278,6 @@ class PageService {
 
     const builder = new PageQueryBuilder(Page.find(), true)
       .addConditionAsNotMigrated() // to avoid affecting v5 pages
-      .addConditionToExcludeRedirect()
       .addConditionToListOnlyDescendants(targetPagePath);
 
     await Page.addConditionToFilteringByViewerToEdit(builder, userToOperate);

packages/app/src/server/service/search-delegator/elasticsearch.ts

@@ -99,7 +99,7 @@ class ElasticsearchDelegator implements SearchDelegator<Data> {
   }
 
   shouldIndexed(page) {
-    return page.revision != null && page.redirectTo == null;
+    return page.revision != null;
   }
 
   initClient() {
@@ -441,8 +441,8 @@ class ElasticsearchDelegator implements SearchDelegator<Data> {
     const shouldIndexed = this.shouldIndexed.bind(this);
     const bulkWrite = this.client.bulk.bind(this.client);
 
-    const findQuery = new PageQueryBuilder(queryFactory()).addConditionToExcludeRedirect().query;
-    const countQuery = new PageQueryBuilder(queryFactory()).addConditionToExcludeRedirect().query;
+    const findQuery = new PageQueryBuilder(queryFactory()).query;
+    const countQuery = new PageQueryBuilder(queryFactory()).query;
 
     const totalCount = await countQuery.count();
 

packages/plugin-attachment-refs/src/server/routes/refs.js

@@ -166,8 +166,7 @@ module.exports = (crowi) => {
     if (prefix != null) {
       builder = new PageQueryBuilder(Page.find())
         .addConditionToListWithDescendants(prefix)
-        .addConditionToExcludeTrashed()
-        .addConditionToExcludeRedirect();
+        .addConditionToExcludeTrashed();
     }
     // builder to get single page
     else {

packages/plugin-lsx/src/server/routes/lsx.js

@@ -195,8 +195,7 @@ module.exports = (crowi, app) => {
     }
 
     builder
-      .addConditionToExcludeTrashed()
-      .addConditionToExcludeRedirect();
+      .addConditionToExcludeTrashed();
 
     return Page.addConditionToFilteringByViewerForList(builder, user);
   }