Merge pull request #30 from apostrophecms/pro-5712

PRO-5712: add migration for targetLocale
apostrophecms · Mar 6, 2024 · 23478c5 · 23478c5
2 parents ce28a11 + 23575ce
commit 23478c5
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Changelog
 
+## UNRELEASED
+
+* Bug fix to properly migrate older redirects missing a `targetLocale` property, and to tolerate situations where this property is irrelevant or makes reference to a locale that no longer exists in the system.
+
 ## 1.4.0 (2024-02-23)
 
 Several fixes and improvements contributed by Stéphane Maccari of Michelin:

diff --git a/index.js b/index.js
@@ -22,6 +22,7 @@ module.exports = {
   },
   init(self) {
     self.addUnlocalizedMigration();
+    self.addTargetLocaleMigration();
     self.createIndexes();
   },
   handlers(self) {
@@ -36,8 +37,8 @@ module.exports = {
         },
         setCurrentLocale(req, doc) {
           const internalPage = doc._newPage && doc._newPage[0];
-
-          doc.targetLocale = internalPage && doc.urlType === 'internal'
+          // Watch out for unlocalized types and missing documents
+          doc.targetLocale = (doc.urlType === 'internal' && internalPage?.aposLocale)
             ? internalPage.aposLocale.replace(/:.*$/, '')
             : null;
         }
@@ -210,8 +211,11 @@ module.exports = {
 
             const shouldForwardQueryString = foundTarget && foundTarget.forwardQueryString;
 
-            const localizedReq = foundTarget.urlType === 'internal' &&
-              req.locale !== foundTarget.targetLocale
+            const localizedReq = (
+              (foundTarget.urlType === 'internal') &&
+              Object.keys(self.apos.i18n.locales).includes(foundTarget.targetLocale) &&
+              (req.locale !== foundTarget.targetLocale)
+            )
               ? req.clone({ locale: foundTarget.targetLocale })
               : req;
 
@@ -305,6 +309,33 @@ module.exports = {
           }
         });
       },
+
+      addTargetLocaleMigration() {
+        self.apos.migration.add('@apostrophecms/redirect:addTargetLocale', async () => {
+          await self.apos.migration.eachDoc({
+            type: self.__meta.name,
+            urlType: 'internal',
+            targetLocale: {
+              $exists: 0
+            }
+          }, async redirect => {
+            await self.apos.doc.db.updateOne({
+              _id: redirect._id
+            }, {
+              $set: {
+                // It is in the nature of the original bug that we can't tell exactly
+                // what locale this should have been (relationships use aposDocId which
+                // is cross-locale and we were not saving any information about the
+                // target locale). However the default locale is the most
+                // likely to be useful and prevents a crash, and if it is not useful
+                // the user can just edit or remove the redirect
+                targetLocale: self.apos.i18n.defaultLocale
+              }
+            });
+          });
+        });
+      },
+
       createIndexes() {
         self.apos.doc.db.createIndex({ redirectSlug: 1 });
       }

diff --git a/test/index.js b/test/index.js
@@ -62,17 +62,18 @@ describe('@apostrophecms/redirect', function () {
     const reqFr = apos.task.getReq({ locale: 'fr' });
     const instance = redirectModule.newInstance();
     const pageFr = await apos.page.find(reqFr, { title: 'page fr' }).toObject();
-    await redirectModule.insert(req, {
+    const inserted = await redirectModule.insert(req, {
       ...instance,
       title: 'internal redirect',
       urlType: 'internal',
       redirectSlug: '/page-1',
       _newPage: [ pageFr ]
     });
-
+    assert.strictEqual(inserted.targetLocale, 'fr');
     const redirected = await apos.http.get('http://localhost:3000/page-1');
     assert.equal(redirected, '<title>page fr</title>\n');
   });
+
 });
 
 async function insertPages(apos) {