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

Merge pull request #5698 from weseek/feat/determine-if-empty-page-is-deletable

feat: Implement method to find non-empty ancestor page to determine whether empty page is deletable
Yohei Shiina 3 лет назад
Родитель
Сommit
d5ffe8b626

+ 4 - 3
packages/app/src/components/PageContentFooter.tsx

@@ -1,14 +1,15 @@
 import React, { FC, memo } from 'react';
 
-import AuthorInfo from './Navbar/AuthorInfo';
-
 import { Ref } from '../interfaces/common';
 import { IUser } from '../interfaces/user';
 
+import AuthorInfo from './Navbar/AuthorInfo';
+
+
 type Props = {
   createdAt: Date,
   updatedAt: Date,
-  creator: Ref<IUser>,
+  creator: any,
   revisionAuthor: Ref<IUser>,
 }
 

+ 1 - 1
packages/app/src/interfaces/page.ts

@@ -12,7 +12,7 @@ export interface IPage {
   status: string,
   revision: Ref<IRevision>,
   tags: Ref<ITag>[],
-  creator: Ref<IUser>,
+  creator: any,
   createdAt: Date,
   updatedAt: Date,
   seenUsers: Ref<IUser>[],

+ 13 - 0
packages/app/src/server/models/page.ts

@@ -915,6 +915,19 @@ export function generateGrantCondition(
 
 schema.statics.generateGrantCondition = generateGrantCondition;
 
+schema.statics.findNotEmptyClosestAncestor = async function(path: string): Promise<PageDocument> {
+  const builderForAncestors = new PageQueryBuilder(this.find(), false); // empty page not included
+
+  const ancestors = await builderForAncestors
+    .addConditionToListOnlyAncestors(path) // only ancestor paths
+    .addConditionToSortPagesByDescPath() // sort by path in Desc. Long to Short.
+    .query
+    .exec();
+
+  return ancestors[0];
+};
+
+
 export type PageCreateOptions = {
   format?: string
   grantUserGroupId?: ObjectIdLike

+ 3 - 2
packages/app/src/server/routes/apiv3/page-listing.ts

@@ -131,14 +131,15 @@ export default (crowi: Crowi): Router => {
 
       for (const page of pages) {
         // construct isIPageInfoForListing
-        const basicPageInfo = pageService.constructBasicPageInfo(page);
+        // eslint-disable-next-line no-await-in-loop
+        const basicPageInfo = await pageService.constructBasicPageInfo(page, req.user);
 
         const pageInfo = (!isIPageInfoForEntity(basicPageInfo))
           ? basicPageInfo
           // create IPageInfoForListing
           : {
             ...basicPageInfo,
-            isAbleToDeleteCompletely: pageService.canDeleteCompletely((page.creator as IUserHasId)?._id, req.user, false), // use normal delete config
+            isAbleToDeleteCompletely: pageService.canDeleteCompletely(page.path, (page.creator as IUserHasId)?._id, req.user, false), // use normal delete config
             bookmarkCount: bookmarkCountMap != null ? bookmarkCountMap[page._id] : undefined,
             revisionShortBody: shortBodiesMap != null ? shortBodiesMap[page._id] : undefined,
           } as IPageInfoForListing;

+ 2 - 2
packages/app/src/server/routes/page.js

@@ -1190,7 +1190,7 @@ module.exports = function(crowi, app) {
 
     try {
       if (isCompletely) {
-        if (!crowi.pageService.canDeleteCompletely(page.creator, req.user, isRecursively)) {
+        if (!crowi.pageService.canDeleteCompletely(page.path, page.creator, req.user, isRecursively)) {
           return res.json(ApiResponse.error('You can not delete this page completely', 'user_not_admin'));
         }
         await crowi.pageService.deleteCompletely(page, req.user, options, isRecursively);
@@ -1206,7 +1206,7 @@ module.exports = function(crowi, app) {
           return res.json(ApiResponse.error('Someone could update this page, so couldn\'t delete.', 'outdated'));
         }
 
-        if (!crowi.pageService.canDelete(page.creator, req.user, isRecursively)) {
+        if (!crowi.pageService.canDelete(page.path, page.creator, req.user, isRecursively)) {
           return res.json(ApiResponse.error('You can not delete this page', 'user_not_admin'));
         }
 

+ 30 - 15
packages/app/src/server/service/page.ts

@@ -40,7 +40,7 @@ const debug = require('debug')('growi:services:page');
 const logger = loggerFactory('growi:services:page');
 const {
   isTrashPage, isTopPage, omitDuplicateAreaPageFromPages,
-  collectAncestorPaths, isMovablePage, canMoveByPath, hasSlash, generateChildrenRegExp,
+  collectAncestorPaths, isMovablePage, canMoveByPath, isUsersProtectedPages, hasSlash, generateChildrenRegExp,
 } = pagePathUtils;
 
 const { addTrailingSlash } = pathUtils;
@@ -238,7 +238,9 @@ class PageService {
     });
   }
 
-  canDeleteCompletely(creatorId: ObjectIdLike, operator, isRecursively: boolean): boolean {
+  canDeleteCompletely(path: string, creatorId: ObjectIdLike, operator: any | null, isRecursively: boolean): boolean {
+    if (operator == null || isTopPage(path) || isUsersProtectedPages(path)) return false;
+
     const pageCompleteDeletionAuthority = this.crowi.configManager.getConfig('crowi', 'security:pageCompleteDeletionAuthority');
     const pageRecursiveCompleteDeletionAuthority = this.crowi.configManager.getConfig('crowi', 'security:pageRecursiveCompleteDeletionAuthority');
 
@@ -247,7 +249,9 @@ class PageService {
     return this.canDeleteLogic(creatorId, operator, isRecursively, singleAuthority, recursiveAuthority);
   }
 
-  canDelete(creatorId: ObjectIdLike, operator, isRecursively: boolean): boolean {
+  canDelete(path: string, creatorId: ObjectIdLike, operator: any | null, isRecursively: boolean): boolean {
+    if (operator == null || isUsersProtectedPages(path) || isTopPage(path)) return false;
+
     const pageDeletionAuthority = this.crowi.configManager.getConfig('crowi', 'security:pageDeletionAuthority');
     const pageRecursiveDeletionAuthority = this.crowi.configManager.getConfig('crowi', 'security:pageRecursiveDeletionAuthority');
 
@@ -289,11 +293,11 @@ class PageService {
   }
 
   filterPagesByCanDeleteCompletely(pages, user, isRecursively: boolean) {
-    return pages.filter(p => p.isEmpty || this.canDeleteCompletely(p.creator, user, isRecursively));
+    return pages.filter(p => p.isEmpty || this.canDeleteCompletely(p.path, p.creator, user, isRecursively));
   }
 
   filterPagesByCanDelete(pages, user, isRecursively: boolean) {
-    return pages.filter(p => p.isEmpty || this.canDelete(p.creator, user, isRecursively));
+    return pages.filter(p => p.isEmpty || this.canDelete(p.path, p.creator, user, isRecursively));
   }
 
   // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
@@ -327,8 +331,7 @@ class PageService {
       };
     }
 
-    const isGuestUser = user == null;
-    const pageInfo = this.constructBasicPageInfo(page, isGuestUser);
+    const pageInfo = await this.constructBasicPageInfo(page, user);
 
     const Bookmark = this.crowi.model('Bookmark');
     const bookmarkCount = await Bookmark.countByPageId(pageId);
@@ -337,7 +340,7 @@ class PageService {
       ...pageInfo,
       bookmarkCount,
     };
-
+    const isGuestUser = user == null;
     if (isGuestUser) {
       return {
         data: page,
@@ -347,7 +350,6 @@ class PageService {
 
     const isBookmarked: boolean = (await Bookmark.findByPageIdAndUserId(pageId, user._id)) != null;
     const isLiked: boolean = page.isLiked(user);
-    const isAbleToDeleteCompletely: boolean = this.canDeleteCompletely((page.creator as IUserHasId)?._id, user, false); // use normal delete config
 
     const subscription = await Subscription.findByUserIdAndTargetId(user._id, pageId);
 
@@ -355,7 +357,6 @@ class PageService {
       data: page,
       meta: {
         ...metadataForGuest,
-        isAbleToDeleteCompletely,
         isBookmarked,
         isLiked,
         subscriptionStatus: subscription?.status,
@@ -2168,16 +2169,27 @@ class PageService {
     });
   }
 
-  constructBasicPageInfo(page: IPage, isGuestUser?: boolean): IPageInfo | IPageInfoForEntity {
+  async constructBasicPageInfo(page: IPage, operator): Promise<IPageInfo | IPageInfoForEntity> {
+    const Page = mongoose.model('Page') as unknown as PageModel;
+
+    const isGuestUser = operator == null;
     const isMovable = isGuestUser ? false : isMovablePage(page.path);
 
     if (page.isEmpty) {
+      // Need non-empty ancestor page to get its creator id because empty page does NOT have it.
+      // Use creator id of ancestor page to determine whether the empty page is deletable
+      const notEmptyClosestAncestor = await Page.findNotEmptyClosestAncestor(page.path);
+      const creatorId = notEmptyClosestAncestor.creator;
+
+      const isDeletable = this.canDelete(page.path, creatorId, operator, false);
+      const isAbleToDeleteCompletely = this.canDeleteCompletely(page.path, creatorId, operator, false); // use normal delete config
+
       return {
         isV5Compatible: true,
         isEmpty: true,
         isMovable,
-        isDeletable: true,
-        isAbleToDeleteCompletely: true,
+        isDeletable,
+        isAbleToDeleteCompletely,
         isRevertible: false,
       };
     }
@@ -2185,6 +2197,9 @@ class PageService {
     const likers = page.liker.slice(0, 15) as Ref<IUserHasId>[];
     const seenUsers = page.seenUsers.slice(0, 15) as Ref<IUserHasId>[];
 
+    const isDeletable = this.canDelete(page.path, page.creator, operator, false);
+    const isAbleToDeleteCompletely = this.canDeleteCompletely(page.path, page.creator, operator, false); // use normal delete config
+
     return {
       isV5Compatible: isTopPage(page.path) || page.parent != null,
       isEmpty: false,
@@ -2193,8 +2208,8 @@ class PageService {
       seenUserIds: this.extractStringIds(seenUsers),
       sumOfSeenUsers: page.seenUsers.length,
       isMovable,
-      isDeletable: isMovable,
-      isAbleToDeleteCompletely: false,
+      isDeletable,
+      isAbleToDeleteCompletely,
       isRevertible: isTrashPage(page.path),
     };