FIX: Ensure that custom header links migration do not fail validation

This commit is a follow up to 7374793
where the migration will fail because the objects created by the
migration will end up failing to save because the objects will fail the
validation given the new schema. This commit updates the migration to
ensure that we do not end up with invalid objects.

migrations/settings/0002-migrate-custom-header-links.js

@@ -2,31 +2,47 @@ export default function migrate(settings) {
   const oldSetting = settings.get("custom_header_links");
 
   if (oldSetting) {
-    const newSetting = oldSetting.split("|").map((link) => {
-      const [text, title, url, view, target, hide_on_scroll, locale] = link
+    const newLinks = [];
+
+    oldSetting.split("|").forEach((link) => {
+      const [text, title, url, view, target, hideOnScroll, locale] = link
         .split(",")
         .map((s) => s.trim());
 
-      const newLink = {
-        text,
-        title,
-        url,
-        view,
-        target,
-        hide_on_scroll,
-        locale,
-      };
-
-      Object.keys(newLink).forEach((key) => {
-        if (newLink[key] === undefined) {
-          delete newLink[key];
+      if (text && title && url) {
+        const newLink = {
+          text,
+          title,
+          url,
+        };
+
+        if (["vdm", "vdo", "vmo"].includes(view)) {
+          newLink.view = view;
+        } else {
+          newLink.view = "vdm";
+        }
+
+        if (["blank", "self"].includes(target)) {
+          newLink.target = target;
+        } else {
+          newLink.target = "blank";
+        }
+
+        if (["remove", "keep"].includes(hideOnScroll)) {
+          newLink.hide_on_scroll = hideOnScroll;
+        } else {
+          newLink.hide_on_scroll = "keep";
+        }
+
+        if (locale) {
+          newLink.locale = locale;
         }
-      });
 
-      return newLink;
+        newLinks.push(newLink);
+      }
     });
 
-    settings.set("custom_header_links", newSetting);
+    settings.set("custom_header_links", newLinks);
   }
 
   return settings;

settings.yml

@@ -21,6 +21,7 @@ custom_header_links:
       hide_on_scroll: "keep"
   schema:
     name: "link"
+    identifier: text
     properties:
       text:
         type: string
@@ -30,7 +31,6 @@ custom_header_links:
           max_length: 100
       title:
         type: string
-        required: true
         validations:
           min_length: 1
           max_length: 1000

test/unit/migrations/settings/0002-migrate-custom-header-links-test.js

@@ -4,6 +4,109 @@ import migrate from "../../../../migrations/settings/0002-migrate-custom-header-
 module(
   "Unit | Migrations | Settings | 0002-migrate-custom-header-links",
   function () {
+    test("migrate when old setting value is invalid", function (assert) {
+      const settings = new Map(
+        Object.entries({
+          custom_header_links:
+            "Invalid Link|Invalid Link 2, some title|Invalid Link 3, , ,",
+        })
+      );
+      const result = migrate(settings);
+
+      const expectedResult = new Map(
+        Object.entries({ custom_header_links: [] })
+      );
+
+      assert.deepEqual(
+        Object.fromEntries(result.entries()),
+        Object.fromEntries(expectedResult.entries())
+      );
+    });
+
+    test("migrate when `hide_on_scroll` attribute is invalid", function (assert) {
+      const settings = new Map(
+        Object.entries({
+          custom_header_links:
+            "External link, this link will open in a new tab, https://meta.discourse.org, vdo, blank, invalid",
+        })
+      );
+
+      const result = migrate(settings);
+
+      const expectedResult = new Map(
+        Object.entries({
+          custom_header_links: [
+            {
+              text: "External link",
+              title: "this link will open in a new tab",
+              url: "https://meta.discourse.org",
+              view: "vdo",
+              target: "blank",
+              hide_on_scroll: "keep",
+            },
+          ],
+        })
+      );
+
+      assert.deepEqual(Array.from(result), Array.from(expectedResult));
+    });
+
+    test("migrate when `target` attribute is invalid", function (assert) {
+      const settings = new Map(
+        Object.entries({
+          custom_header_links:
+            "External link, this link will open in a new tab, https://meta.discourse.org, vdo, invalid, remove",
+        })
+      );
+
+      const result = migrate(settings);
+
+      const expectedResult = new Map(
+        Object.entries({
+          custom_header_links: [
+            {
+              text: "External link",
+              title: "this link will open in a new tab",
+              url: "https://meta.discourse.org",
+              view: "vdo",
+              target: "blank",
+              hide_on_scroll: "remove",
+            },
+          ],
+        })
+      );
+
+      assert.deepEqual(Array.from(result), Array.from(expectedResult));
+    });
+
+    test("migrate when `view` attribute is invalid", function (assert) {
+      const settings = new Map(
+        Object.entries({
+          custom_header_links:
+            "External link, this link will open in a new tab, https://meta.discourse.org, invalid, blank, remove",
+        })
+      );
+
+      const result = migrate(settings);
+
+      const expectedResult = new Map(
+        Object.entries({
+          custom_header_links: [
+            {
+              text: "External link",
+              title: "this link will open in a new tab",
+              url: "https://meta.discourse.org",
+              view: "vdm",
+              target: "blank",
+              hide_on_scroll: "remove",
+            },
+          ],
+        })
+      );
+
+      assert.deepEqual(Array.from(result), Array.from(expectedResult));
+    });
+
     test("migrate", function (assert) {
       const settings = new Map(
         Object.entries({