Merge pull request #4926 from weseek/imprv/add-condition-to-omit-empty-pages-from-api

imprv: Add condition to omit empty pages from api

packages/app/src/client/services/ContextExtractor.tsx

@@ -60,7 +60,7 @@ const ContextExtractorOnce: FC = () => {
   const hasDraftOnHackmd = !!mainContent?.getAttribute('data-page-has-draft-on-hackmd');
   const creator = JSON.parse(mainContent?.getAttribute('data-page-creator') || jsonNull);
   const revisionAuthor = JSON.parse(mainContent?.getAttribute('data-page-revision-author') || jsonNull);
-  const targetAndAncestors = JSON.parse(mainContent?.getAttribute('data-target-and-ancestors') || jsonNull);
+  const targetAndAncestors = JSON.parse(document.getElementById('growi-pagetree-target-and-ancestors')?.textContent || jsonNull);
   const slackChannels = mainContent?.getAttribute('data-slack-channels') || '';
 
   /*

packages/app/src/client/services/PageContainer.js

@@ -82,7 +82,6 @@ export default class PageContainer extends Container {
       templateTagData: mainContent.getAttribute('data-template-tags') || null,
       shareLinksNumber: mainContent.getAttribute('data-share-links-number'),
       shareLinkId: JSON.parse(mainContent.getAttribute('data-share-link-id') || null),
-      targetAndAncestors: JSON.parse(mainContent.getAttribute('data-target-and-ancestors') || null),
 
       // latest(on remote) information
       remoteRevisionId: revisionId,

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

@@ -1,4 +1,4 @@
-import React, { FC, useState } from 'react';
+import React, { FC } from 'react';
 
 import { IPageHasId } from '../../../interfaces/page';
 import { ItemNode } from './ItemNode';
@@ -93,8 +93,6 @@ const ItemsTree: FC<ItemsTreeProps> = (props: ItemsTreeProps) => {
   const { data: ancestorsChildrenData, error: error1 } = useSWRxPageAncestorsChildren(targetPath);
   const { data: rootPageData, error: error2 } = useSWRxRootPage();
 
-  const [isRenderedCompletely, setRenderedCompletely] = useState(false);
-
   const DeleteModal = (
     <PageDeleteModal
       isOpen={isDeleteModalOpen}
@@ -114,9 +112,8 @@ const ItemsTree: FC<ItemsTreeProps> = (props: ItemsTreeProps) => {
   /*
    * Render completely
    */
-  if (!isRenderedCompletely && ancestorsChildrenData != null && rootPageData != null) {
+  if (ancestorsChildrenData != null && rootPageData != null) {
     const initialNode = generateInitialNodeAfterResponse(ancestorsChildrenData.ancestorsChildren, new ItemNode(rootPageData.rootPage));
-    setRenderedCompletely(true); // render once
     return renderByInitialNode(initialNode, DeleteModal, isEnableActions, targetId, onClickDeleteByPage);
   }
 

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

@@ -79,8 +79,17 @@ const populateDataToShowRevision = (page, userPublicFields) => {
 
 export class PageQueryBuilder {
 
-  constructor(query) {
+  constructor(query, includeEmpty = false) {
     this.query = query;
+    if (!includeEmpty) {
+      this.query = this.query
+        .and({
+          $or: [
+            { isEmpty: false },
+            { isEmpty: null }, // for v4 compatibility
+          ],
+        });
+    }
   }
 
   addConditionToExcludeTrashed() {
@@ -592,7 +601,7 @@ export const getPageSchema = (crowi) => {
    * @param {User} user User instance
    * @param {UserGroup[]} userGroups List of UserGroup instances
    */
-  pageSchema.statics.findByIdAndViewer = async function(id, user, userGroups) {
+  pageSchema.statics.findByIdAndViewer = async function(id, user, userGroups, includeEmpty = false) {
     const baseQuery = this.findOne({ _id: id });
 
     let relatedUserGroups = userGroups;
@@ -602,18 +611,21 @@ export const getPageSchema = (crowi) => {
       relatedUserGroups = await UserGroupRelation.findAllUserGroupIdsRelatedToUser(user);
     }
 
-    const queryBuilder = new PageQueryBuilder(baseQuery);
+    const queryBuilder = new PageQueryBuilder(baseQuery, includeEmpty);
     queryBuilder.addConditionToFilteringByViewer(user, relatedUserGroups, true);
 
-    return await queryBuilder.query.exec();
+    return queryBuilder.query.exec();
   };
 
   // find page by path
-  pageSchema.statics.findByPath = function(path) {
+  pageSchema.statics.findByPath = function(path, includeEmpty = false) {
     if (path == null) {
       return null;
     }
-    return this.findOne({ path });
+
+    const builder = new PageQueryBuilder(this.findOne({ path }), includeEmpty);
+
+    return builder.query.exec();
   };
 
   /**
@@ -621,7 +633,7 @@ export const getPageSchema = (crowi) => {
    * @param {User} user User instance
    * @param {UserGroup[]} userGroups List of UserGroup instances
    */
-  pageSchema.statics.findAncestorByPathAndViewer = async function(path, user, userGroups) {
+  pageSchema.statics.findAncestorByPathAndViewer = async function(path, user, userGroups, includeEmpty = false) {
     if (path == null) {
       throw new Error('path is required.');
     }
@@ -642,10 +654,10 @@ export const getPageSchema = (crowi) => {
       relatedUserGroups = await UserGroupRelation.findAllUserGroupIdsRelatedToUser(user);
     }
 
-    const queryBuilder = new PageQueryBuilder(baseQuery);
+    const queryBuilder = new PageQueryBuilder(baseQuery, includeEmpty);
     queryBuilder.addConditionToFilteringByViewer(user, relatedUserGroups);
 
-    return await queryBuilder.query.exec();
+    return queryBuilder.query.exec();
   };
 
   pageSchema.statics.findByRedirectTo = function(path) {
@@ -655,22 +667,22 @@ export const getPageSchema = (crowi) => {
   /**
    * find pages that is match with `path` and its descendants
    */
-  pageSchema.statics.findListWithDescendants = async function(path, user, option = {}) {
-    const builder = new PageQueryBuilder(this.find());
+  pageSchema.statics.findListWithDescendants = async function(path, user, option = {}, includeEmpty = false) {
+    const builder = new PageQueryBuilder(this.find(), includeEmpty);
     builder.addConditionToListWithDescendants(path, option);
 
-    return await findListFromBuilderAndViewer(builder, user, false, option);
+    return findListFromBuilderAndViewer(builder, user, false, option);
   };
 
   /**
    * find pages that is match with `path` and its descendants whitch user is able to manage
    */
-  pageSchema.statics.findManageableListWithDescendants = async function(page, user, option = {}) {
+  pageSchema.statics.findManageableListWithDescendants = async function(page, user, option = {}, includeEmpty = false) {
     if (user == null) {
       return null;
     }
 
-    const builder = new PageQueryBuilder(this.find());
+    const builder = new PageQueryBuilder(this.find(), includeEmpty);
     builder.addConditionToListWithDescendants(page.path, option);
     builder.addConditionToExcludeRedirect();
 
@@ -691,11 +703,11 @@ export const getPageSchema = (crowi) => {
   /**
    * find pages that start with `path`
    */
-  pageSchema.statics.findListByStartWith = async function(path, user, option) {
-    const builder = new PageQueryBuilder(this.find());
+  pageSchema.statics.findListByStartWith = async function(path, user, option, includeEmpty = false) {
+    const builder = new PageQueryBuilder(this.find(), includeEmpty);
     builder.addConditionToListByStartWith(path, option);
 
-    return await findListFromBuilderAndViewer(builder, user, false, option);
+    return findListFromBuilderAndViewer(builder, user, false, option);
   };
 
   /**
@@ -1102,8 +1114,8 @@ export const getPageSchema = (crowi) => {
     await this.removeRedirectOriginPageByPath(redirectPage.path);
   };
 
-  pageSchema.statics.findListByPathsArray = async function(paths) {
-    const queryBuilder = new PageQueryBuilder(this.find());
+  pageSchema.statics.findListByPathsArray = async function(paths, includeEmpty = false) {
+    const queryBuilder = new PageQueryBuilder(this.find(), includeEmpty);
     queryBuilder.addConditionToListByPathsArray(paths);
 
     return await queryBuilder.query.exec();

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

@@ -41,7 +41,7 @@ export interface PageModel extends Model<PageDocument> {
   [x: string]: any; // for obsolete methods
   createEmptyPagesByPaths(paths: string[], publicOnly?: boolean): Promise<void>
   getParentIdAndFillAncestors(path: string): Promise<string | null>
-  findByPathAndViewer(path: string | null, user, userGroups?, useFindOne?: boolean): Promise<PageDocument[]>
+  findByPathAndViewer(path: string | null, user, userGroups?, useFindOne?: boolean, includeEmpty?: boolean): Promise<PageDocument[]>
   findTargetAndAncestorsByPathOrId(pathOrId: string): Promise<TargetAndAncestorsResult>
   findChildrenByParentPathOrIdAndViewer(parentPathOrId: string, user, userGroups?): Promise<PageDocument[]>
   findAncestorsChildrenByPathAndViewer(path: string, user, userGroups?): Promise<Record<string, PageDocument[]>>
@@ -143,7 +143,7 @@ const generateChildrenRegExp = (path: string): RegExp => {
  */
 schema.statics.createEmptyPagesByPaths = async function(paths: string[], publicOnly = false): Promise<void> {
   // find existing parents
-  const builder = new PageQueryBuilder(this.find(publicOnly ? { grant: GRANT_PUBLIC } : {}, { _id: 0, path: 1 }));
+  const builder = new PageQueryBuilder(this.find(publicOnly ? { grant: GRANT_PUBLIC } : {}, { _id: 0, path: 1 }), true);
   const existingPages = await builder
     .addConditionToListByPathsArray(paths)
     .query
@@ -165,7 +165,7 @@ schema.statics.createEmptyPagesByPaths = async function(paths: string[], publicO
 };
 
 /*
- * Find the pages parent and update if the parent exists.
+ * Find the parent and update if the parent exists.
  * If not,
  *   - first   run createEmptyPagesByPaths with ancestor's paths to ensure all the ancestors exist
  *   - second  update ancestor pages' parent
@@ -175,17 +175,20 @@ schema.statics.getParentIdAndFillAncestors = async function(path: string): Promi
   const parentPath = nodePath.dirname(path);
 
   const parent = await this.findOne({ path: parentPath }); // find the oldest parent which must always be the true parent
-  if (parent != null) { // fill parents if parent is null
+  if (parent != null) {
     return parent._id;
   }
 
+  /*
+   * Fill parents if parent is null
+   */
   const ancestorPaths = collectAncestorPaths(path); // paths of parents need to be created
 
   // just create ancestors with empty pages
   await this.createEmptyPagesByPaths(ancestorPaths);
 
   // find ancestors
-  const builder = new PageQueryBuilder(this.find({}, { _id: 1, path: 1 }));
+  const builder = new PageQueryBuilder(this.find({}, { _id: 1, path: 1 }), true);
   const ancestors = await builder
     .addConditionToListByPathsArray(ancestorPaths)
     .addConditionToSortPagesByDescPath()
@@ -193,7 +196,6 @@ schema.statics.getParentIdAndFillAncestors = async function(path: string): Promi
     .lean()
     .exec();
 
-
   const ancestorsMap = new Map(); // Map<path, _id>
   ancestors.forEach(page => ancestorsMap.set(page.path, page._id));
 
@@ -234,14 +236,14 @@ const addViewerCondition = async(queryBuilder: PageQueryBuilder, user, userGroup
  * Find a page by path and viewer. Pass false to useFindOne to use findOne method.
  */
 schema.statics.findByPathAndViewer = async function(
-    path: string | null, user, userGroups = null, useFindOne = true,
+    path: string | null, user, userGroups = null, useFindOne = true, includeEmpty = false,
 ): Promise<PageDocument | PageDocument[] | null> {
   if (path == null) {
     throw new Error('path is required.');
   }
 
   const baseQuery = useFindOne ? this.findOne({ path }) : this.find({ path });
-  const queryBuilder = new PageQueryBuilder(baseQuery);
+  const queryBuilder = new PageQueryBuilder(baseQuery, includeEmpty);
   await addViewerCondition(queryBuilder, user, userGroups);
 
   return queryBuilder.query.exec();
@@ -269,7 +271,7 @@ schema.statics.findTargetAndAncestorsByPathOrId = async function(pathOrId: strin
   ancestorPaths.push(path); // include target
 
   // Do not populate
-  const queryBuilder = new PageQueryBuilder(this.find());
+  const queryBuilder = new PageQueryBuilder(this.find(), true);
   await addViewerCondition(queryBuilder, user, userGroups);
 
   const _targetAndAncestors: PageDocument[] = await queryBuilder
@@ -298,11 +300,11 @@ schema.statics.findChildrenByParentPathOrIdAndViewer = async function(parentPath
   if (hasSlash(parentPathOrId)) {
     const path = parentPathOrId;
     const regexp = generateChildrenRE2(path);
-    queryBuilder = new PageQueryBuilder(this.find({ path: { $regex: regexp.source } }));
+    queryBuilder = new PageQueryBuilder(this.find({ path: { $regex: regexp.source } }), true);
   }
   else {
     const parentId = parentPathOrId;
-    queryBuilder = new PageQueryBuilder(this.find({ parent: parentId }));
+    queryBuilder = new PageQueryBuilder(this.find({ parent: parentId }), true);
   }
   await addViewerCondition(queryBuilder, user, userGroups);
 
@@ -318,7 +320,7 @@ schema.statics.findAncestorsChildrenByPathAndViewer = async function(path: strin
   const regexps = ancestorPaths.map(path => new RegExp(generateChildrenRegExp(path))); // cannot use re2
 
   // get pages at once
-  const queryBuilder = new PageQueryBuilder(this.find({ path: { $in: regexps } }));
+  const queryBuilder = new PageQueryBuilder(this.find({ path: { $in: regexps } }), true);
   await addViewerCondition(queryBuilder, user, userGroups);
   const _pages = await queryBuilder
     .addConditionAsMigrated()

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

@@ -264,10 +264,10 @@ module.exports = function(crowi, app) {
     renderVars.pages = result.pages;
   }
 
-  async function addRenderVarsForPageTree(renderVars, path, user) {
-    const { targetAndAncestors, rootPage } = await Page.findTargetAndAncestorsByPathOrId(path, user);
+  async function addRenderVarsForPageTree(renderVars, pathOrId, user) {
+    const { targetAndAncestors, rootPage } = await Page.findTargetAndAncestorsByPathOrId(pathOrId, user);
 
-    if (targetAndAncestors.length === 0 && !isTopPage(path)) {
+    if (targetAndAncestors.length === 0 && pathOrId.includes('/') && !isTopPage(pathOrId)) {
       throw new Error('Ancestors must have at least one page.');
     }
 
@@ -291,6 +291,7 @@ module.exports = function(crowi, app) {
 
   async function _notFound(req, res) {
     const path = getPathFromRequest(req);
+    const pathOrId = req.params.id || path;
 
     let view;
     const renderVars = { path };
@@ -326,6 +327,7 @@ module.exports = function(crowi, app) {
     const limit = 50;
     const offset = parseInt(req.query.offset) || 0;
     await addRenderVarsForDescendants(renderVars, path, req.user, offset, limit, true);
+    await addRenderVarsForPageTree(renderVars, pathOrId, req.user);
 
     return res.render(view, renderVars);
   }
@@ -334,7 +336,7 @@ module.exports = function(crowi, app) {
     const id = req.params.id;
     const { revisionId } = req.query;
 
-    let page = await Page.findByIdAndViewer(id, req.user);
+    let page = await Page.findByIdAndViewer(id, req.user, null, true, true);
 
     if (page == null) {
       next();
@@ -395,7 +397,7 @@ module.exports = function(crowi, app) {
     const id = req.params.id;
     const revisionId = req.query.revision;
 
-    let page = await Page.findByIdAndViewer(id, req.user);
+    let page = await Page.findByIdAndViewer(id, req.user, null, true, true);
 
     if (page == null) {
       // check the page is forbidden or just does not exist.
@@ -592,7 +594,7 @@ module.exports = function(crowi, app) {
    * redirector
    */
   async function redirector(req, res, next, path) {
-    const pages = await Page.findByPathAndViewer(path, req.user, null, false);
+    const pages = await Page.findByPathAndViewer(path, req.user, null, false, true);
     const { redirectFrom } = req.query;
 
     if (pages.length >= 2) {

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

@@ -1001,7 +1001,7 @@ class PageService {
         await Page.createEmptyPagesByPaths(parentPaths, publicOnly);
 
         // find parents again
-        const builder = new PageQueryBuilder(Page.find({}, { _id: 1, path: 1 }));
+        const builder = new PageQueryBuilder(Page.find({}, { _id: 1, path: 1 }), true);
         const parents = await builder
           .addConditionToListByPathsArray(parentPaths)
           .query

packages/app/src/server/views/layout/layout.html

@@ -125,6 +125,11 @@
   {{ userUISettings|json|safe }}
   </script>
 {% endif %}
+{% if targetAndAncestors != null %}
+  <script type="application/json" id="growi-pagetree-target-and-ancestors">
+  {{ targetAndAncestors|json|safe }}
+  </script>
+{% endif %}
 
 
 {% block custom_script %}

packages/app/src/server/views/widget/page_content.html

@@ -27,7 +27,6 @@
   data-page-user="{% if pageUser %}{{ pageUser|json }}{% else %}null{% endif %}"
   data-share-links-number="{% if page %}{{ sharelinksNumber }}{% endif %}"
   data-share-link-id="{% if sharelink %}{{ sharelink._id|json }}{% endif %}"
-  data-target-and-ancestors="{% if targetAndAncestors %}{{ targetAndAncestors|json }}{% endif %}"
   >
 {% else %}
 <div id="content-main" class="content-main d-flex"