diff --git a/migrations/settings/0002-migrate-custom-header-links.js b/migrations/settings/0002-migrate-custom-header-links.js index ba7ecad..46648f0 100644 --- a/migrations/settings/0002-migrate-custom-header-links.js +++ b/migrations/settings/0002-migrate-custom-header-links.js @@ -1,32 +1,55 @@ export default function migrate(settings) { const oldSetting = settings.get("custom_header_links"); + // Do nothing if setting is already an array which means user has saved the setting in the new format + // This is required because there was a bug in core where this migration would fail but the theme was still updated + // allowing the users to save the setting in the new format. + if (Array.isArray(oldSetting)) { + return settings; + } + 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, - }; + if (text && title && url) { + const newLink = { + text, + title, + url, + }; - Object.keys(newLink).forEach((key) => { - if (newLink[key] === undefined) { - delete newLink[key]; + if (["vdm", "vdo", "vmo"].includes(view)) { + newLink.view = view; + } else { + newLink.view = "vdm"; } - }); - return newLink; + 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; + } + + 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..3bb8161 100644 --- a/settings.yml +++ b/settings.yml @@ -1,26 +1,9 @@ custom_header_links: type: objects - default: - - 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" - - text: "Most Liked" - title: "Posts with the most amount of likes" - url: "/latest/?order=op_likes" - view: "vdo" - target: "self" - hide_on_scroll: "keep" - - text: "Privacy" - title: "Our Privacy Policy" - url: "/privacy" - view: "vdm" - target: "self" - hide_on_scroll: "keep" + default: [] schema: name: "link" + identifier: text properties: text: type: string @@ -30,7 +13,6 @@ custom_header_links: max_length: 100 title: type: string - required: true validations: min_length: 1 max_length: 1000 diff --git a/spec/system/viewing_custom_header_links_spec.rb b/spec/system/viewing_custom_header_links_spec.rb index 857f3b7..a1ec1d8 100644 --- a/spec/system/viewing_custom_header_links_spec.rb +++ b/spec/system/viewing_custom_header_links_spec.rb @@ -6,6 +6,40 @@ RSpec.describe "Viewing Custom Header Links", system: true do fab!(:theme) { upload_theme_component } let!(:custom_header_link) { PageObjects::Components::CustomHeaderLink.new } + before do + theme.update_setting( + :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", + }, + { + text: "Most Liked", + title: "Posts with the most amount of likes", + url: "/latest/?order=op_likes", + view: "vdo", + target: "self", + hide_on_scroll: "keep", + }, + { + text: "Privacy", + title: "Our Privacy Policy", + url: "/privacy", + view: "vdm", + target: "self", + hide_on_scroll: "keep", + }, + ], + ) + + theme.save! + end + context "when glimmer headers are enabled" do before do if SiteSetting.respond_to?(:experimental_glimmer_header_groups) 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..bde6ef1 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,158 @@ import migrate from "../../../../migrations/settings/0002-migrate-custom-header- module( "Unit | Migrations | Settings | 0002-migrate-custom-header-links", function () { + test("migrate when value of setting is already an array", function (assert) { + const settings = new Map( + Object.entries({ + custom_header_links: [ + { + text: "Some link", + title: "Some link title", + url: "https://some.url.com", + view: "vdo", + target: "blank", + hide_on_scroll: "remove", + }, + ], + }) + ); + + const result = migrate(settings); + + const expectedResult = new Map( + Object.entries({ + custom_header_links: [ + { + text: "Some link", + title: "Some link title", + url: "https://some.url.com", + view: "vdo", + target: "blank", + hide_on_scroll: "remove", + }, + ], + }) + ); + + assert.deepEqual( + Object.fromEntries(result.entries()), + Object.fromEntries(expectedResult.entries()) + ); + }); + + 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( + Object.fromEntries(result.entries()), + Object.fromEntries(expectedResult.entries()) + ); + }); + + 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( + Object.fromEntries(result.entries()), + Object.fromEntries(expectedResult.entries()) + ); + }); + + 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( + Object.fromEntries(result.entries()), + Object.fromEntries(expectedResult.entries()) + ); + }); + test("migrate", function (assert) { const settings = new Map( Object.entries({