Merge branch 'master' into feat/test-code-for-migration

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

@@ -192,36 +192,24 @@ const UserGroupDetailPage: FC = () => {
 
   return (
     <div>
-      <a href="/admin/user-groups" className="btn btn-outline-secondary">
-        <i className="icon-fw ti-arrow-left" aria-hidden="true"></i>
-        {t('admin:user_group_management.back_to_list')}
-      </a>
-
-      {
-        userGroup?.parent != null && ancestorUserGroups != null && ancestorUserGroups.length > 0 && (
-          <div className="btn-group ml-2">
-            <a className="btn btn-outline-secondary" href={`/admin/user-group-detail/${userGroup.parent}`}>
-              <i className="icon-fw ti-arrow-left" aria-hidden="true"></i>
-              {t('admin:user_group_management.back_to_ancestors_group')}
-            </a>
-            <button
-              type="button"
-              className="btn btn-outline-secondary dropdown-toggle dropdown-toggle-split"
-              data-toggle="dropdown"
-              aria-haspopup="true"
-              ria-expanded="false"
-            >
-            </button>
-            <div className="dropdown-menu">
-              {
-                ancestorUserGroups.map(userGroup => (
-                  <a className="dropdown-item" key={userGroup._id} href={`/admin/user-group-detail/${userGroup._id}`}>{userGroup.name}</a>
-                ))
-              }
-            </div>
-          </div>
-        )
-      }
+      <nav aria-label="breadcrumb">
+        <ol className="breadcrumb">
+          <li className="breadcrumb-item"><a href="/admin/user-groups">{t('admin:user_group_management.group_list')}</a></li>
+          {
+            ancestorUserGroups != null && ancestorUserGroups.length > 0 && (
+              ancestorUserGroups.map((ancestorUserGroup: IUserGroupHasId) => (
+                <li key={ancestorUserGroup._id} className={`breadcrumb-item ${ancestorUserGroup._id === userGroup._id ? 'active' : ''}`} aria-current="page">
+                  { ancestorUserGroup._id === userGroup._id ? (
+                    <>{ancestorUserGroup.name}</>
+                  ) : (
+                    <a href={`/admin/user-group-detail/${ancestorUserGroup._id}`}>{ancestorUserGroup.name}</a>
+                  )}
+                </li>
+              ))
+            )
+          }
+        </ol>
+      </nav>
 
       <div className="mt-4 form-box">
         <UserGroupForm

packages/app/src/components/Common/Dropdown/PageItemControl.tsx

@@ -257,7 +257,7 @@ export const PageItemControlSubstance = (props: PageItemControlSubstanceProps):
     <Dropdown isOpen={isOpen} toggle={() => setIsOpen(!isOpen)} data-testid="open-page-item-control-btn">
       { children ?? (
         <DropdownToggle color="transparent" className="border-0 rounded btn-page-item-control d-flex align-items-center justify-content-center">
-          <i className="icon-options text-muted"></i>
+          <i className="icon-options"></i>
         </DropdownToggle>
       ) }
 

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

@@ -451,7 +451,7 @@ const Item: FC<ItemProps> = (props: ItemProps) => {
           >
             {/* pass the color property to reactstrap dropdownToggle props. https://6-4-0--reactstrap.netlify.app/components/dropdowns/  */}
             <DropdownToggle color="transparent" className="border-0 rounded btn-page-item-control p-0 grw-visible-on-hover mr-1">
-              <i className="icon-options fa fa-rotate-90 text-muted p-1"></i>
+              <i className="icon-options fa fa-rotate-90 p-1"></i>
             </DropdownToggle>
           </PageItemControl>
           <button
@@ -459,7 +459,7 @@ const Item: FC<ItemProps> = (props: ItemProps) => {
             className="border-0 rounded btn btn-page-item-control p-0 grw-visible-on-hover"
             onClick={onClickPlusButton}
           >
-            <i className="icon-plus text-muted d-block p-0" />
+            <i className="icon-plus d-block p-0" />
           </button>
         </div>
       </li>

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

@@ -366,16 +366,6 @@ export const getPageSchema = (crowi) => {
     return queryBuilder.query.exec();
   };
 
-  pageSchema.statics.findByIdAndViewerToEdit = async function(id, user, includeEmpty = false) {
-    const baseQuery = this.findOne({ _id: id });
-    const queryBuilder = new this.PageQueryBuilder(baseQuery, includeEmpty);
-
-    // add grant conditions
-    await addConditionToFilteringByViewerToEdit(queryBuilder, user);
-
-    return queryBuilder.query.exec();
-  };
-
   // find page by path
   pageSchema.statics.findByPath = function(path, includeEmpty = false) {
     if (path == null) {

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

@@ -46,7 +46,7 @@ export interface PageModel extends Model<PageDocument> {
   [x: string]: any; // for obsolete methods
   createEmptyPagesByPaths(paths: string[], onlyMigratedAsExistingPages?: boolean, publicOnly?: boolean): Promise<void>
   getParentAndFillAncestors(path: string, user): Promise<PageDocument & { _id: any }>
-  findByIdsAndViewer(pageIds: string[], user, userGroups?, includeEmpty?: boolean): Promise<PageDocument[]>
+  findByIdsAndViewer(pageIds: ObjectIdLike[], user, userGroups?, includeEmpty?: 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[]>
@@ -372,6 +372,17 @@ class PageQueryBuilder {
     return this;
   }
 
+  addConditionToExcludeByPageIdsArray(pageIds) {
+    this.query = this.query
+      .and({
+        _id: {
+          $nin: pageIds,
+        },
+      });
+
+    return this;
+  }
+
   populateDataToList(userPublicFields) {
     this.query = this.query
       .populate({
@@ -829,16 +840,6 @@ schema.statics.removeLeafEmptyPagesRecursively = async function(pageId: ObjectId
   await this.deleteMany({ _id: { $in: pageIdsToRemove } });
 };
 
-schema.statics.findByPageIdsToEdit = async function(ids, user, shouldIncludeEmpty = false) {
-  const builder = new PageQueryBuilder(this.find({ _id: { $in: ids } }), shouldIncludeEmpty);
-
-  await this.addConditionToFilteringByViewerToEdit(builder, user);
-
-  const pages = await builder.query.exec();
-
-  return pages;
-};
-
 schema.statics.normalizeDescendantCountById = async function(pageId) {
   const children = await this.find({ parent: pageId });
 

packages/app/src/server/models/user-group.ts

@@ -101,7 +101,7 @@ schema.statics.findGroupsWithAncestorsRecursively = async function(group, ancest
     return ancestors;
   }
 
-  ancestors.push(parent);
+  ancestors.unshift(parent);
 
   return this.findGroupsWithAncestorsRecursively(parent, ancestors);
 };

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

@@ -497,7 +497,7 @@ module.exports = (crowi) => {
     let renamedPage;
 
     try {
-      page = await Page.findByIdAndViewerToEdit(pageId, req.user, true);
+      page = await Page.findByIdAndViewer(pageId, req.user, null, true);
 
       if (page == null) {
         return res.apiv3Err(new ErrorV3(`Page '${pageId}' is not found or forbidden`, 'notfound_or_forbidden'), 401);
@@ -653,7 +653,7 @@ module.exports = (crowi) => {
       return res.apiv3Err(new ErrorV3(`Page exists '${newPagePath})'`, 'already_exists'), 409);
     }
 
-    const page = await Page.findByIdAndViewerToEdit(pageId, req.user, true);
+    const page = await Page.findByIdAndViewer(pageId, req.user, null, true);
 
     const isEmptyAndNotRecursively = page?.isEmpty && !isRecursively;
     if (page == null || isEmptyAndNotRecursively) {
@@ -748,7 +748,7 @@ module.exports = (crowi) => {
 
     let pagesToDelete;
     try {
-      pagesToDelete = await Page.findByPageIdsToEdit(pageIds, req.user, true);
+      pagesToDelete = await Page.findByIdsAndViewer(pageIds, req.user, null, true);
     }
     catch (err) {
       logger.error('Failed to find pages to delete.', err);

packages/app/src/server/routes/apiv3/user-group.js

@@ -165,7 +165,7 @@ module.exports = (crowi) => {
 
     try {
       const userGroup = await UserGroup.findById(groupId);
-      const ancestorUserGroups = await UserGroup.findGroupsWithAncestorsRecursively(userGroup, []);
+      const ancestorUserGroups = await UserGroup.findGroupsWithAncestorsRecursively(userGroup);
       return res.apiv3({ ancestorUserGroups });
     }
     catch (err) {

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

@@ -1177,7 +1177,7 @@ module.exports = function(crowi, app) {
 
     const options = {};
 
-    const page = await Page.findByIdAndViewerToEdit(pageId, req.user, true);
+    const page = await Page.findByIdAndViewer(pageId, req.user, null, true);
 
     if (page == null) {
       return res.json(ApiResponse.error(`Page '${pageId}' is not found or forbidden`, 'notfound_or_forbidden'));

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

@@ -1,5 +1,5 @@
 import mongoose from 'mongoose';
-import { pagePathUtils, pathUtils } from '@growi/core';
+import { pagePathUtils, pathUtils, pageUtils } from '@growi/core';
 import escapeStringRegexp from 'escape-string-regexp';
 
 import UserGroup from '~/server/models/user-group';
@@ -357,6 +357,8 @@ class PageGrantService {
       throw Error(`The maximum number of pageIds allowed is ${LIMIT_FOR_MULTIPLE_PAGE_OP}.`);
     }
 
+    const Page = mongoose.model('Page') as unknown as PageModel;
+
     const shouldCheckDescendants = true;
     const shouldIncludeNotMigratedPages = true;
 
@@ -368,8 +370,12 @@ class PageGrantService {
         path, grant, grantedUsers: grantedUserIds, grantedGroup: grantedGroupId,
       } = page;
 
-      const isNormalized = await this.isGrantNormalized(path, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants, shouldIncludeNotMigratedPages);
-      if (isNormalized) {
+      if (!pageUtils.isPageNormalized(page)) {
+        nonNormalizable.push(page);
+        continue;
+      }
+
+      if (await this.isGrantNormalized(path, grant, grantedUserIds, grantedGroupId, shouldCheckDescendants, shouldIncludeNotMigratedPages)) {
         normalizable.push(page);
       }
       else {

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

@@ -107,7 +107,6 @@ class PageCursorsForDescendantsFactory {
 
     const builder = new PageQueryBuilder(this.Page.find(), this.shouldIncludeEmpty);
     builder.addConditionToFilteringByParentId(page._id);
-    // await this.Page.addConditionToFilteringByViewerToEdit(builder, this.user);
 
     const cursor = builder.query.lean().cursor({ batchSize: BULK_REINDEX_SIZE }) as QueryCursor<any>;
 
@@ -305,7 +304,7 @@ class PageService {
     const isRoot = isTopPage(page.path);
     const isPageRestricted = page.grant === Page.GRANT_RESTRICTED;
 
-    const shouldUseV4Process = !isRoot && !isPageRestricted && (!isV5Compatible || !isPageMigrated || isTrashPage);
+    const shouldUseV4Process = !isRoot && (!isV5Compatible || !isPageMigrated || isTrashPage || isPageRestricted);
 
     return shouldUseV4Process;
   }
@@ -316,7 +315,7 @@ class PageService {
     const isV5Compatible = this.crowi.configManager.getConfig('crowi', 'app:isV5Compatible');
     const isPageRestricted = page.grant === Page.GRANT_RESTRICTED;
 
-    const shouldUseV4Process = !isPageRestricted && !isV5Compatible;
+    const shouldUseV4Process = !isV5Compatible || isPageRestricted;
 
     return shouldUseV4Process;
   }
@@ -534,18 +533,15 @@ class PageService {
     const newParentPath = pathlib.dirname(toPath);
 
     // local util
-    const collectAncestorPathsUntilFromPath = (path: string, paths: string[] = [path]): string[] => {
-      const nextPath = pathlib.dirname(path);
-      if (nextPath === fromPath) {
-        return [...paths, nextPath];
-      }
-
-      paths.push(nextPath);
+    const collectAncestorPathsUntilFromPath = (path: string, paths: string[] = []): string[] => {
+      if (path === fromPath) return paths;
 
-      return collectAncestorPathsUntilFromPath(nextPath, paths);
+      const parentPath = pathlib.dirname(path);
+      paths.push(parentPath);
+      return collectAncestorPathsUntilFromPath(parentPath, paths);
     };
 
-    const pathsToInsert = collectAncestorPathsUntilFromPath(newParentPath);
+    const pathsToInsert = collectAncestorPathsUntilFromPath(toPath);
     const originalParent = await Page.findById(originalPage.parent);
     if (originalParent == null) {
       throw Error('Original parent not found');
@@ -2192,7 +2188,7 @@ class PageService {
     const Page = mongoose.model('Page') as unknown as PageModel;
 
     if (isRecursively) {
-      const pages = await Page.findByPageIdsToEdit(pageIds, user, false);
+      const pages = await Page.findByIdsAndViewer(pageIds, user, null);
 
       // DO NOT await !!
       this.normalizeParentRecursivelyByPages(pages, user);
@@ -2558,15 +2554,13 @@ class PageService {
         },
       ]);
 
-    // limit pages to get
+    // Limit pages to get
     const total = await Page.countDocuments(filter);
     if (total > PAGES_LIMIT) {
       baseAggregation = baseAggregation.limit(Math.floor(total * 0.3));
     }
 
     const pagesStream = await baseAggregation.cursor({ batchSize: BATCH_SIZE });
-
-    // use batch stream
     const batchStream = createBatchStream(BATCH_SIZE);
 
     let countPages = 0;
@@ -2577,10 +2571,15 @@ class PageService {
       async write(pages, encoding, callback) {
         const parentPaths = Array.from(new Set<string>(pages.map(p => pathlib.dirname(p.path))));
 
-        // Fill parents with empty pages
+        // 1. Remove unnecessary empty pages
+        const pageIdsToNotDelete = pages.map(p => p._id);
+        const emptyPagePathsToDelete = pages.map(p => p.path);
+        await Page.removeEmptyPages(pageIdsToNotDelete, emptyPagePathsToDelete);
+
+        // 2. Create lacking parents as empty pages
         await Page.createEmptyPagesByPaths(parentPaths, user, false);
 
-        // Find parents
+        // 3. Find parents
         const builder = new PageQueryBuilder(Page.find({}, { _id: 1, path: 1 }), true);
         const parents = await builder
           .addConditionToListByPathsArray(parentPaths)
@@ -2635,9 +2634,6 @@ class PageService {
           throw err;
         }
 
-        // Remove unnecessary empty pages
-        await Page.removeEmptyPages(pages.map(p => p._id), pages.map(p => p.path));
-
         callback();
       },
       final(callback) {

packages/app/src/styles/theme/_apply-colors-dark.scss

@@ -38,8 +38,9 @@ $border-color: $border-color-global;
   */
 input.form-control,
 select.form-control,
+select.custom-select,
 textarea.form-control {
-  color: lighten($color-global, 30%);
+  color: $color-global;
   background-color: darken($bgcolor-global, 5%);
   border-color: $border-color-global;
   &:focus {
@@ -69,6 +70,10 @@ textarea.form-control {
   border-color: $border-color-global;
 }
 
+label.custom-control-label::before {
+  background-color: darken($bgcolor-global, 5%);
+}
+
 /*
  * Dropdown
  */

packages/app/src/styles/theme/_apply-colors.scss

@@ -712,6 +712,7 @@ mark.rbt-highlight-text {
 
 // Page Management Dropdown icon
 .btn-page-item-control {
+  color: $gray-500;
   &:hover,
   &:focus {
     background-color: rgba($color-link, 0.15);

packages/app/test/cypress/integration/2-basic-features/switch-sidebar-contents.spec.ts

@@ -0,0 +1,31 @@
+context('Access to page', () => {
+  const ssPrefix = 'switch-sidebar-content';
+
+  let connectSid: string | undefined;
+
+  before(() => {
+    // login
+    cy.fixture("user-admin.json").then(user => {
+      cy.login(user.username, user.password);
+    });
+    cy.getCookie('connect.sid').then(cookie => {
+      connectSid = cookie?.value;
+    });
+  });
+
+  beforeEach(() => {
+    if (connectSid != null) {
+      cy.setCookie('connect.sid', connectSid);
+    }
+  });
+
+  it('PageTree is successfully shown', () => {
+    cy.visit('/');
+    cy.getByTestid('grw-sidebar-nav-primary-page-tree').click();
+    cy.screenshot(`${ssPrefix}-pagetree-before-load`, { capture: 'viewport' });
+    // eslint-disable-next-line cypress/no-unnecessary-waiting
+    cy.wait(1500);
+    cy.screenshot(`${ssPrefix}-pagetree-after-load`, { capture: 'viewport' });
+  });
+
+});

packages/core/src/index.js

@@ -1,6 +1,7 @@
 import * as _pathUtils from './utils/path-utils';
 import * as _envUtils from './utils/env-utils';
 import * as _pagePathUtils from './utils/page-path-utils';
+import * as _pageUtils from './utils/page-utils';
 import * as _templateChecker from './utils/template-checker';
 import * as _customTagUtils from './plugin/util/custom-tag-utils';
 
@@ -8,6 +9,7 @@ import * as _customTagUtils from './plugin/util/custom-tag-utils';
 export const pathUtils = _pathUtils;
 export const envUtils = _envUtils;
 export const pagePathUtils = _pagePathUtils;
+export const pageUtils = _pageUtils;
 export const templateChecker = _templateChecker;
 export const customTagUtils = _customTagUtils;
 

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

@@ -0,0 +1,58 @@
+import { isTopPage } from './page-path-utils';
+
+const GRANT_PUBLIC = 1;
+const GRANT_RESTRICTED = 2;
+const GRANT_SPECIFIED = 3; // DEPRECATED
+const GRANT_OWNER = 4;
+const GRANT_USER_GROUP = 5;
+const PAGE_GRANT_ERROR = 1;
+const STATUS_PUBLISHED = 'published';
+const STATUS_DELETED = 'deleted';
+
+/**
+ * Returns true if the page is on tree including the top page.
+ * @param page Page
+ * @returns boolean
+ */
+// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
+export const isOnTree = (page): boolean => {
+  const { path, parent } = page;
+
+  if (isTopPage(path)) {
+    return true;
+  }
+
+  if (parent != null) {
+    return true;
+  }
+
+  return false;
+};
+
+/**
+ * Returns true if the page meet the condition below.
+ *   - The page is on tree (has parent or the top page)
+ *   - The page's grant is GRANT_RESTRICTED or GRANT_SPECIFIED
+ *   - The page's status is STATUS_DELETED
+ * This does not check grantedUser or grantedGroup.
+ * @param page PageDocument
+ * @returns boolean
+ */
+// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
+export const isPageNormalized = (page): boolean => {
+  const { grant, status } = page;
+
+  if (grant === GRANT_RESTRICTED || grant === GRANT_SPECIFIED) {
+    return true;
+  }
+
+  if (status === STATUS_DELETED) {
+    return true;
+  }
+
+  if (isOnTree(page)) {
+    return true;
+  }
+
+  return true;
+};