Merge pull request #6377 from weseek/imprv/re-impl-remove-page-redirect-method

imprv: Re-implement removePageRedirectsByToPath with $graphLookup

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

@@ -56,6 +56,27 @@ schema.statics.retrievePageRedirectEndpoints = async function(fromPath: string):
       },
     },
   ]);
+  /* ---------- aggResult example ----------
+  {
+    "_id" : ObjectId("62e5650d6134d37aa0935e6d"),
+    "fromPath" : "/page1",
+    "toPath" : "/page2",
+    "chains" : [
+        {
+            "_id" : ObjectId("62e5651b6134d37aa0935e7a"),
+            "fromPath" : "/page2",
+            "toPath" : "/page3",
+            "depth" : NumberLong(0)
+        },
+        {
+            "_id" : ObjectId("62e565256134d37aa0935e80"),
+            "fromPath" : "/page3",
+            "toPath" : "/Sandbox",
+            "depth" : NumberLong(1)
+        }
+    ]
+  }
+  */
 
   if (aggResult.length === 0) {
     return null;
@@ -78,14 +99,64 @@ schema.statics.retrievePageRedirectEndpoints = async function(fromPath: string):
   return { start, end };
 };
 
-schema.statics.removePageRedirectByToPath = async function(toPath: string): Promise<void> {
-  await this.deleteMany({ toPath });
+schema.statics.removePageRedirectsByToPath = async function(toPath: string): Promise<void> {
+  const aggResult: IPageRedirectWithChains[] = await this.aggregate([
+    { $match: { toPath } },
+    {
+      $graphLookup: {
+        from: 'pageredirects',
+        startWith: '$fromPath',
+        connectFromField: 'fromPath',
+        connectToField: 'toPath',
+        as: CHAINS_FIELD_NAME,
+      },
+    },
+  ]);
+  /* ---------- aggResult example ----------
+  // 1
+  {
+    "_id" : ObjectId("62e565256134d37aa0935e80"),
+    "fromPath" : "/page3",
+    "toPath" : "/page4",
+    "chains" : [
+        {
+            "_id" : ObjectId("62e5651b6134d37aa0935e7a"),
+            "fromPath" : "/page2",
+            "toPath" : "/page3",
+            "depth" : NumberLong(0)
+        },
+        {
+            "_id" : ObjectId("62e5650d6134d37aa0935e6d"),
+            "fromPath" : "/page1",
+            "toPath" : "/page2",
+            "depth" : NumberLong(1)
+        }
+    ]
+  }
+  // 2
+  {
+    "_id" : ObjectId("62e5937a6134d37aa0936405"),
+    "fromPath" : "/org/page4",
+    "toPath" : "/page4",
+    "chains" : []
+  }
+  */
+
+  if (aggResult.length === 0) {
+    return;
+  }
 
+  const idsToRemove = aggResult
+    .map((redirectWithChains) => {
+      return [
+        redirectWithChains._id,
+        redirectWithChains[CHAINS_FIELD_NAME].map(doc => doc._id),
+      ].flat();
+    })
+    .flat();
+
+  await this.deleteMany({ _id: { $in: idsToRemove } });
   return;
 };
 
-// schema.statics.removePageRedirectsByChains = async function(redirectChains: IPageRedirectChains): Promise<void> {
-//   return;
-// };
-
 export default getOrCreateModel<PageRedirectDocument, PageRedirectModel>('PageRedirect', schema);

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

@@ -1476,7 +1476,7 @@ module.exports = function(crowi, app) {
     const path = req.body.path;
 
     try {
-      await PageRedirect.removePageRedirectByToPath(path);
+      await PageRedirect.removePageRedirectsByToPath(path);
       logger.debug('Redirect Page deleted', path);
     }
     catch (err) {

packages/app/test/integration/models/page-redirect.test.js

@@ -19,33 +19,32 @@ describe('PageRedirect', () => {
     await PageRedirect.deleteMany({});
   });
 
-  describe('.removePageRedirectByToPath', () => {
+  describe('.removePageRedirectsByToPath', () => {
     test('works fine', async() => {
       // setup:
       await PageRedirect.insertMany([
         { fromPath: '/org/path1', toPath: '/path1' },
         { fromPath: '/org/path2', toPath: '/path2' },
         { fromPath: '/org/path3', toPath: '/path3' },
-        { fromPath: '/org/path33', toPath: '/path3' },
+        { fromPath: '/org/path33', toPath: '/org/path333' },
+        { fromPath: '/org/path333', toPath: '/path3' },
       ]);
       expect(await PageRedirect.findOne({ fromPath: '/org/path1' })).not.toBeNull();
       expect(await PageRedirect.findOne({ fromPath: '/org/path2' })).not.toBeNull();
       expect(await PageRedirect.findOne({ fromPath: '/org/path3' })).not.toBeNull();
       expect(await PageRedirect.findOne({ fromPath: '/org/path33' })).not.toBeNull();
+      expect(await PageRedirect.findOne({ fromPath: '/org/path333' })).not.toBeNull();
 
       // when:
       // remove all documents that have { toPath: '/path/3' }
-      await PageRedirect.removePageRedirectByToPath('/path3');
+      await PageRedirect.removePageRedirectsByToPath('/path3');
 
       // then:
-      const r1 = await PageRedirect.findOne({ fromPath: '/org/path1' });
-      const r2 = await PageRedirect.findOne({ fromPath: '/org/path2' });
-      const r3 = await PageRedirect.findOne({ fromPath: '/org/path3' });
-      const r4 = await PageRedirect.findOne({ fromPath: '/org/path33' });
-      expect(r1).not.toBeNull();
-      expect(r2).not.toBeNull();
-      expect(r3).toBeNull();
-      expect(r4).toBeNull();
+      expect(await PageRedirect.findOne({ fromPath: '/org/path1' })).not.toBeNull();
+      expect(await PageRedirect.findOne({ fromPath: '/org/path2' })).not.toBeNull();
+      expect(await PageRedirect.findOne({ fromPath: '/org/path3' })).toBeNull();
+      expect(await PageRedirect.findOne({ fromPath: '/org/path33' })).toBeNull();
+      expect(await PageRedirect.findOne({ fromPath: '/org/path333' })).toBeNull();
     });
   });