Merge pull request #7979 from weseek/fix/consider-empty-page-when-rename-and-duplicate-2

fix: Consider an empty page when renaming and duplicating

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

@@ -529,7 +529,7 @@ module.exports = (crowi) => {
     // check whether path starts slash
     newPagePath = addHeadingSlash(newPagePath);
 
-    const isExist = await Page.count({ path: newPagePath }) > 0;
+    const isExist = await Page.exists({ path: newPagePath, isEmpty: false });
     if (isExist) {
       // if page found, cannot rename to that path
       return res.apiv3Err(new ErrorV3(`${newPagePath} already exists`, 'already_exists'), 409);
@@ -757,7 +757,7 @@ module.exports = (crowi) => {
       }
 
       // check page existence
-      const isExist = (await Page.count({ path: newPagePath })) > 0;
+      const isExist = (await Page.exists({ path: newPagePath, isEmpty: false }));
       if (isExist) {
         return res.apiv3Err(new ErrorV3(`Page exists '${newPagePath})'`, 'already_exists'), 409);
       }

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

@@ -375,7 +375,7 @@ class PageService {
 
     const activity = await this.crowi.activityService.createActivity(parameters);
 
-    const isExist = await Page.exists({ path: newPagePath });
+    const isExist = await Page.exists({ path: newPagePath, isEmpty: false });
     if (isExist) {
       throw Error(`Page already exists at ${newPagePath}`);
     }

apps/app/test/integration/service/v5.public-page.test.ts

@@ -135,9 +135,13 @@ describe('PageService page operations with only public pages', () => {
     const pageIdForRename9 = new mongoose.Types.ObjectId();
     const pageIdForRename10 = new mongoose.Types.ObjectId();
     const pageIdForRename11 = new mongoose.Types.ObjectId();
-    const pageIdForRename12 = new mongoose.Types.ObjectId();
-    const pageIdForRename13 = new mongoose.Types.ObjectId();
-    const pageIdForRename14 = new mongoose.Types.ObjectId();
+
+    const childPageIdForRename1 = new mongoose.Types.ObjectId();
+    const childPageIdForRename2 = new mongoose.Types.ObjectId();
+    const childPageIdForRename3 = new mongoose.Types.ObjectId();
+    const childPageIdForRename4 = new mongoose.Types.ObjectId();
+    const childPageIdForRename5 = new mongoose.Types.ObjectId();
+    const childPageIdForRename7 = new mongoose.Types.ObjectId();
 
     const pageIdForRename16 = new mongoose.Types.ObjectId();
 
@@ -235,7 +239,7 @@ describe('PageService page operations with only public pages', () => {
       },
       {
         _id: pageIdForRename10,
-        path: '/v5_ChildForRename1',
+        path: '/v5_ParentForRename10',
         grant: Page.GRANT_PUBLIC,
         creator: dummyUser1,
         lastUpdateUser: dummyUser1._id,
@@ -243,6 +247,23 @@ describe('PageService page operations with only public pages', () => {
       },
       {
         _id: pageIdForRename11,
+        path: '/v5_ParentForRename11',
+        grant: Page.GRANT_PUBLIC,
+        creator: dummyUser1,
+        lastUpdateUser: dummyUser1._id,
+        parent: rootPage._id,
+        isEmpty: true,
+      },
+      {
+        _id: childPageIdForRename1,
+        path: '/v5_ChildForRename1',
+        grant: Page.GRANT_PUBLIC,
+        creator: dummyUser1,
+        lastUpdateUser: dummyUser1._id,
+        parent: rootPage._id,
+      },
+      {
+        _id: childPageIdForRename2,
         path: '/v5_ChildForRename2',
         grant: Page.GRANT_PUBLIC,
         creator: dummyUser1,
@@ -250,7 +271,7 @@ describe('PageService page operations with only public pages', () => {
         parent: rootPage._id,
       },
       {
-        _id: pageIdForRename12,
+        _id: childPageIdForRename3,
         path: '/v5_ChildForRename3',
         grant: Page.GRANT_PUBLIC,
         creator: dummyUser1,
@@ -259,7 +280,7 @@ describe('PageService page operations with only public pages', () => {
         updatedAt: new Date('2021'),
       },
       {
-        _id: pageIdForRename13,
+        _id: childPageIdForRename4,
         path: '/v5_ChildForRename4',
         grant: Page.GRANT_PUBLIC,
         creator: dummyUser1,
@@ -267,7 +288,7 @@ describe('PageService page operations with only public pages', () => {
         parent: rootPage._id,
       },
       {
-        _id: pageIdForRename14,
+        _id: childPageIdForRename5,
         path: '/v5_ChildForRename5',
         grant: Page.GRANT_PUBLIC,
         creator: dummyUser1,
@@ -275,7 +296,7 @@ describe('PageService page operations with only public pages', () => {
         parent: rootPage._id,
       },
       {
-        _id: pageIdForRename16,
+        _id: childPageIdForRename7,
         path: '/v5_ChildForRename7',
         grant: Page.GRANT_PUBLIC,
         parent: rootPage._id,
@@ -286,7 +307,7 @@ describe('PageService page operations with only public pages', () => {
         grant: Page.GRANT_PUBLIC,
         creator: dummyUser1,
         lastUpdateUser: dummyUser1._id,
-        parent: pageIdForRename14,
+        parent: childPageIdForRename5,
         updatedAt: new Date('2021'),
       },
       {
@@ -294,7 +315,7 @@ describe('PageService page operations with only public pages', () => {
         grant: Page.GRANT_PUBLIC,
         creator: dummyUser1,
         lastUpdateUser: dummyUser1._id,
-        parent: pageIdForRename16,
+        parent: childPageIdForRename7,
       },
       {
         _id: pageIdForRename17,
@@ -470,6 +491,7 @@ describe('PageService page operations with only public pages', () => {
     const pageIdForDuplicate13 = new mongoose.Types.ObjectId();
     const pageIdForDuplicate14 = new mongoose.Types.ObjectId();
     const pageIdForDuplicate15 = new mongoose.Types.ObjectId();
+    const pageIdForDuplicate16 = new mongoose.Types.ObjectId();
 
     // revision ids
     const revisionIdForDuplicate1 = new mongoose.Types.ObjectId();
@@ -606,6 +628,13 @@ describe('PageService page operations with only public pages', () => {
         parent: pageIdForDuplicate14,
         revision: revisionIdForDuplicate12,
       },
+      {
+        _id: pageIdForDuplicate16,
+        path: '/v5_PageForDuplicate16',
+        grant: Page.GRANT_PUBLIC,
+        parent: rootPage._id,
+        isEmpty: true,
+      },
     ]);
 
     await Revision.insertMany([
@@ -1370,6 +1399,24 @@ describe('PageService page operations with only public pages', () => {
 
       expect(isThrown).toBe(true);
     });
+    test('Should rename/move to the path that exists as an empty page', async() => {
+      const page = await Page.findOne({ path: '/v5_ParentForRename10' });
+      const pageDistination = await Page.findOne({ path: '/v5_ParentForRename11', isEmpty: true });
+      expect(page).toBeTruthy();
+      expect(pageDistination).toBeTruthy();
+      expect(pageDistination.isEmpty).toBe(true);
+
+      const newPath = '/v5_ParentForRename11';
+      const renamedPage = await renamePage(page, newPath, dummyUser1, {}, {
+        ip: '::ffff:127.0.0.1',
+        endpoint: '/_api/v3/pages/rename',
+      });
+
+      expect(xssSpy).toHaveBeenCalled();
+      expect(renamedPage.path).toBe(newPath);
+      expect(renamedPage.isEmpty).toBe(false);
+      expect(renamedPage._id).toStrictEqual(page._id);
+    });
     test('Rename non-empty page path to its descendant non-empty page path', async() => {
       const initialPathForPage1 = '/v5_pageForRename17';
       const initialPathForPage2 = '/v5_pageForRename17/v5_pageForRename18';
@@ -1696,6 +1743,24 @@ describe('PageService page operations with only public pages', () => {
       expect(isThrown).toBe(true);
     });
 
+    test('Should duplicate to the path that exists as an empty page', async() => {
+      const page = await Page.findOne({ path: '/v5_PageForDuplicate1' });
+      expect(page).toBeTruthy();
+
+      const newPagePath = '/v5_PageForDuplicate16';
+      const duplicatedPage = await duplicate(page, newPagePath, dummyUser1, false);
+
+      const duplicatedRevision = await Revision.findOne({ pageId: duplicatedPage._id });
+      const baseRevision = await Revision.findOne({ pageId: page._id });
+
+      // new path
+      expect(xssSpy).toHaveBeenCalled();
+      expect(duplicatedPage.path).toBe(newPagePath);
+      expect(duplicatedPage._id).not.toStrictEqual(page._id);
+      expect(duplicatedPage.revision).toStrictEqual(duplicatedRevision._id);
+      expect(duplicatedRevision.body).toEqual(baseRevision.body);
+    });
+
     test('Should duplicate multiple pages', async() => {
       const basePage = await Page.findOne({ path: '/v5_PageForDuplicate3' });
       const revision = await Revision.findOne({ pageId: basePage._id });