From f238271f718e878604b344c3530bad376d684a2b Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Thu, 25 Apr 2024 08:21:24 +0800 Subject: [PATCH] FIX: Ensure that custom header links migration do not fail validation This commit is a follow up to 73747938bde3c2d3f6f68350c48a320364be1b04 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. --- .../0002-migrate-custom-header-links.js | 52 ++++++--- settings.yml | 2 +- .../0002-migrate-custom-header-links-test.js | 103 ++++++++++++++++++ 3 files changed, 138 insertions(+), 19 deletions(-) diff --git a/migrations/settings/0002-migrate-custom-header-links.js b/migrations/settings/0002-migrate-custom-header-links.js index ba7ecad..5395782 100644 --- a/migrations/settings/0002-migrate-custom-header-links.js +++ b/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; diff --git a/settings.yml b/settings.yml index 71b6ddc..5f44482 100644 --- a/settings.yml +++ b/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 diff --git a/test/unit/migrations/settings/0002-migrate-custom-header-links-test.js b/test/unit/migrations/settings/0002-migrate-custom-header-links-test.js index 0a02b99..801cc7d 100644 --- a/test/unit/migrations/settings/0002-migrate-custom-header-links-test.js +++ b/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({