Merge pull request #2525 from weseek/fix/2523-delete-page-grant_restricted

fix #2523 delete pages which grant is GRANT_RESTRICTED

CHANGES.md

@@ -2,7 +2,7 @@
 
 ## v4.0.10-RC
 
-* 
+* Fix: Fail to rename/delete a page set as "Anyone with the link"
 
 ## v4.0.9
 

src/client/js/components/PageManagement/ApiErrorMessage.jsx

@@ -42,12 +42,10 @@ const ApiErrorMessage = (props) => {
         return (
           <strong><i className="icon-fw icon-ban"></i> Invalid path</strong>
         );
-      case 'unknown':
+      default:
         return (
           <strong><i className="icon-fw icon-ban"></i> Unknown error occured</strong>
         );
-      default:
-        return null;
     }
   }
 

src/server/models/page.js

@@ -693,7 +693,7 @@ module.exports = function(crowi) {
   /**
    * find pages that is match with `path` and its descendants
    */
-  pageSchema.statics.findListWithDescendants = async function(path, user, option) {
+  pageSchema.statics.findListWithDescendants = async function(path, user, option = {}) {
     const builder = new PageQueryBuilder(this.find());
     builder.addConditionToListWithDescendants(path, option);
 
@@ -1094,21 +1094,18 @@ module.exports = function(crowi) {
       throw new Error('This method does NOT supports deleting trashed pages.');
     }
 
-    const findOpts = { includeRedirect: true };
-    const result = await this.findListWithDescendants(targetPage.path, user, findOpts);
+    // find descendants (this array does not include GRANT_RESTRICTED)
+    const result = await this.findListWithDescendants(targetPage.path, user);
     const pages = result.pages;
+    // add targetPage if 'grant' is GRANT_RESTRICTED
+    //  because findListWithDescendants excludes GRANT_RESTRICTED pages
+    if (targetPage.grant === GRANT_RESTRICTED) {
+      pages.push(targetPage);
+    }
 
-    let updatedPage = null;
     await Promise.all(pages.map((page) => {
-      const isParent = (page.path === targetPage.path);
-      const p = this.deletePage(page, user, options);
-      if (isParent) {
-        updatedPage = p;
-      }
-      return p;
+      return this.deletePage(page, user, options);
     }));
-
-    return updatedPage;
   };
 
   pageSchema.statics.revertDeletedPage = async function(page, user, options = {}) {
@@ -1135,7 +1132,7 @@ module.exports = function(crowi) {
   };
 
   pageSchema.statics.revertDeletedPageRecursively = async function(targetPage, user, options = {}) {
-    const findOpts = { includeRedirect: true, includeTrashed: true };
+    const findOpts = { includeTrashed: true };
     const result = await this.findListWithDescendants(targetPage.path, user, findOpts);
     const pages = result.pages;
 
@@ -1185,17 +1182,23 @@ module.exports = function(crowi) {
   /**
    * Delete Bookmarks, Attachments, Revisions, Pages and emit delete
    */
-  pageSchema.statics.completelyDeletePageRecursively = async function(pagePath, user, options = {}) {
+  pageSchema.statics.completelyDeletePageRecursively = async function(targetPage, user, options = {}) {
+    const pagePath = targetPage.path;
 
-    const findOpts = { includeRedirect: true, includeTrashed: true };
+    const findOpts = { includeTrashed: true };
+
+    // find descendants (this array does not include GRANT_RESTRICTED)
     const result = await this.findListWithDescendants(pagePath, user, findOpts);
     const pages = result.pages;
+    // add targetPage if 'grant' is GRANT_RESTRICTED
+    //  because findListWithDescendants excludes GRANT_RESTRICTED pages
+    if (targetPage.grant === GRANT_RESTRICTED) {
+      pages.push(targetPage);
+    }
 
     await Promise.all(pages.map((page) => {
       return this.completelyDeletePage(page, user, options);
     }));
-
-    return pagePath;
   };
 
   pageSchema.statics.removeByPath = function(path) {
@@ -1263,22 +1266,30 @@ module.exports = function(crowi) {
     return updatedPageData;
   };
 
-  pageSchema.statics.renameRecursively = async function(pageData, newPagePathPrefix, user, options) {
+  pageSchema.statics.renameRecursively = async function(targetPage, newPagePathPrefix, user, options) {
     validateCrowi();
 
-    const path = pageData.path;
+    const path = targetPage.path;
     const pathRegExp = new RegExp(`^${escapeStringRegexp(path)}`, 'i');
 
     // sanitize path
     newPagePathPrefix = crowi.xss.process(newPagePathPrefix); // eslint-disable-line no-param-reassign
 
+    // find descendants (this array does not include GRANT_RESTRICTED)
     const result = await this.findListWithDescendants(path, user, options);
-    await Promise.all(result.pages.map((page) => {
+    const pages = result.pages;
+    // add targetPage if 'grant' is GRANT_RESTRICTED
+    //  because findListWithDescendants excludes GRANT_RESTRICTED pages
+    if (targetPage.grant === GRANT_RESTRICTED) {
+      pages.push(targetPage);
+    }
+
+    await Promise.all(pages.map((page) => {
       const newPagePath = page.path.replace(pathRegExp, newPagePathPrefix);
       return this.rename(page, newPagePath, user, options);
     }));
-    pageData.path = newPagePathPrefix;
-    return pageData;
+    targetPage.path = newPagePathPrefix;
+    return targetPage;
   };
 
   pageSchema.statics.handlePrivatePagesForDeletedGroup = async function(deletedGroup, action, transferToUserGroupId) {

src/server/routes/page.js

@@ -1231,7 +1231,7 @@ module.exports = function(crowi, app) {
 
     const options = { socketClientId };
 
-    let page = await Page.findByIdAndViewer(pageId, req.user);
+    const page = await Page.findByIdAndViewer(pageId, req.user);
 
     if (page == null) {
       return res.json(ApiResponse.error(`Page '${pageId}' is not found or forbidden`, 'notfound_or_forbidden'));
@@ -1245,10 +1245,10 @@ module.exports = function(crowi, app) {
           return res.json(ApiResponse.error('You can not delete completely', 'user_not_admin'));
         }
         if (isRecursively) {
-          await Page.completelyDeletePageRecursively(page.path, req.user, options);
+          await Page.completelyDeletePageRecursively(page, req.user, options);
         }
         else {
-          page = await Page.completelyDeletePage(page, req.user, options);
+          await Page.completelyDeletePage(page, req.user, options);
         }
       }
       else {
@@ -1257,16 +1257,16 @@ module.exports = function(crowi, app) {
         }
 
         if (isRecursively) {
-          page = await Page.deletePageRecursively(page, req.user, options);
+          await Page.deletePageRecursively(page, req.user, options);
         }
         else {
-          page = await Page.deletePage(page, req.user, options);
+          await Page.deletePage(page, req.user, options);
         }
       }
     }
     catch (err) {
       logger.error('Error occured while get setting', err);
-      return res.json(ApiResponse.error('Failed to delete page.', 'unknown'));
+      return res.json(ApiResponse.error('Failed to delete page.', err.message));
     }
 
     debug('Page deleted', page.path);