From ce6748fad5c8385ad1cc9bff5668730f50ba24d8 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Wed, 24 Apr 2024 14:24:52 +0800 Subject: [PATCH] DEV: Migrate `social_links` theme setting to new objects setting type This commit migrates the `social_links` theme setting to `type: objects`. Since [discourse/discourse@a440e15](https://github.com/discourse/discourse/commit/a440e15291e98a866f80a6566d5ec0597ab77e2e), we have started to support objects typed theme setting so we are switching this theme component to use it instead as it provides a much better UX for configuring the settings required for the theme component. --- .../discourse/components/custom-footer.hbs | 8 +- .../discourse/components/custom-footer.js | 23 -- locales/en.yml | 20 +- .../0004-migrate-social-links-setting.js | 47 ++++ settings.yml | 55 ++++- .../0004_migrate_social_links_spec.rb | 212 ++++++++++++++++++ spec/system/footer_spec.rb | 2 +- 7 files changed, 335 insertions(+), 32 deletions(-) create mode 100644 migrations/settings/0004-migrate-social-links-setting.js create mode 100644 spec/migrations/0004_migrate_social_links_spec.rb diff --git a/javascripts/discourse/components/custom-footer.hbs b/javascripts/discourse/components/custom-footer.hbs index 9b4f4f1..c8fe1ac 100644 --- a/javascripts/discourse/components/custom-footer.hbs +++ b/javascripts/discourse/components/custom-footer.hbs @@ -60,15 +60,15 @@
- {{#each this.socialLinks as |link|}} + {{#each (theme-setting "social_links") as |link|}} {{/each}}
diff --git a/javascripts/discourse/components/custom-footer.js b/javascripts/discourse/components/custom-footer.js index bd1c283..376af00 100644 --- a/javascripts/discourse/components/custom-footer.js +++ b/javascripts/discourse/components/custom-footer.js @@ -1,29 +1,6 @@ import Component from "@glimmer/component"; -import { dasherize } from "@ember/string"; export default class extends Component { mainHeading = settings.heading; blurb = settings.blurb; - - socialLinks = settings.social_links - .split("|") - .filter(Boolean) - .map((link) => { - const fragments = link.split(",").map((fragment) => fragment.trim()); - const text = fragments[0]; - const dataName = dasherize(text); - const title = fragments[1]; - const href = fragments[2]; - const target = fragments[3] === "blank" ? "_blank" : ""; - const icon = fragments[4].toLowerCase(); - - return { - text, - dataName, - title, - href, - target, - icon, - }; - }); } diff --git a/locales/en.yml b/locales/en.yml index c556a71..9133f72 100644 --- a/locales/en.yml +++ b/locales/en.yml @@ -47,7 +47,25 @@ en: description: The target attribute of the link social_links: - description: "Enter the social links you'd like to add to the footer in this format:
provider, title, URL, target
Provider: is the name of the provider like Facebook or Twitter
Title: The text that shows when the link is hovered
URL: The path you'd like the link to have
Target: Use blank to open the link in a new tab and use self to open it in the same tab
Icon: use a FontAwesome5 icon name (brand icons need a 'fab-' prefix)." + description: "Social links you'd like to add to the footer" + schema: + properties: + text: + label: Text + description: Text to be displayed for the link + title: + label: Title + description: The title attribute of the link + url: + label: URL + description: The URL the link points to + target: + label: Target + description: The target attribute of the link + icon_name: + label: Icon Name + description: FontAwesome5 icon name (brand icons need a 'fab-' prefix) + show_footer_on_login_required_page: description: "Check this setting if you want the footer to be displayed on the login-required page (only applies if your site is private)" svg_icons: diff --git a/migrations/settings/0004-migrate-social-links-setting.js b/migrations/settings/0004-migrate-social-links-setting.js new file mode 100644 index 0000000..4d4578e --- /dev/null +++ b/migrations/settings/0004-migrate-social-links-setting.js @@ -0,0 +1,47 @@ +export default function migrate(settings, helpers) { + const oldSocialLinks = settings.get("social_links"); + + if (oldSocialLinks) { + const newSocialLinks = []; + + oldSocialLinks.split("|").forEach((oldSocialLink) => { + const [linkText, linkTitle, linkUrl, linkTarget, linkIcon] = oldSocialLink + .split(",") + .map((value) => value.trim()); + + if (linkText) { + const newLink = { + text: linkText, + }; + + if (linkTitle) { + newLink.title = linkTitle.substring(0, 1000); + } + + if (linkUrl && helpers.isValidUrl(linkUrl) && linkUrl.length <= 2048) { + newLink.url = linkUrl; + } else { + newLink.url = "#"; + } + + if (linkTarget === "self") { + newLink.target = "_self"; + } else { + newLink.target = "_blank"; + } + + if (linkIcon && linkIcon.length <= 200) { + newLink.icon_name = linkIcon.toLowerCase(); + } + + newSocialLinks.push(newLink); + } + }); + + settings.set("social_links", newSocialLinks); + } else if (oldSocialLinks?.trim() === "") { + settings.set("social_links", []); + } + + return settings; +} diff --git a/settings.yml b/settings.yml index ce84f55..b1c6b97 100644 --- a/settings.yml +++ b/settings.yml @@ -104,9 +104,58 @@ small_links: - _top social_links: - type: list - list_type: simple - default: "Facebook, Join us on Facebook, #, blank,fab-facebook|Twitter, show some love on Twitter, #, blank,fab-twitter| Youtube, Check out our latest videos on Youtube, #, blank,fab-youtube" + type: objects + default: + - text: Facebook + title: Join us on Facebook + url: "#" + target: _blank + icon_name: fab-facebook + - text: Twitter + title: Show some love on Twitter + url: "#" + target: _blank + icon_name: fab-twitter + - text: Youtube + title: Check out our latest videos on Youtube + url: "#" + target: _blank + icon_name: fab-youtube + schema: + name: social link + identifier: text + properties: + text: + type: string + required: true + validations: + min_length: 1 + max_length: 1000 + title: + type: string + validations: + min_length: 1 + max_length: 1000 + url: + type: string + required: true + validations: + min_length: 1 + max_length: 2048 + url: true + target: + type: string + default: _blank + choices: + - _blank + - _self + - _parent + - _top + icon_name: + type: string + validations: + min_length: 1 + max_length: 200 show_footer_on_login_required_page: default: true diff --git a/spec/migrations/0004_migrate_social_links_spec.rb b/spec/migrations/0004_migrate_social_links_spec.rb new file mode 100644 index 0000000..d079e9f --- /dev/null +++ b/spec/migrations/0004_migrate_social_links_spec.rb @@ -0,0 +1,212 @@ +# frozen_string_literal: true + +RSpec.describe "0004-migrate-social-links-setting migration" do + let!(:theme) { upload_theme_component } + + it "should handle an empty string for old setting" do + theme.theme_settings.create!( + name: "social_links", + theme:, + data_type: ThemeSetting.types[:string], + value: " ", + ) + + run_theme_migration(theme, "0004-migrate-social-links-setting") + + expect(theme.settings[:social_links].value).to eq([]) + end + + it "does not migrate links which do not have a text property" do + theme.theme_settings.create!( + name: "social_links", + theme:, + data_type: ThemeSetting.types[:string], + value: " ,some title, /some/url", + ) + + run_theme_migration(theme, "0004-migrate-social-links-setting") + + expect(theme.settings[:social_links].value).to eq([]) + end + + it "should migrate title property correctly if previous title property is valid" do + theme.theme_settings.create!( + name: "social_links", + theme:, + data_type: ThemeSetting.types[:string], + value: "Some Text, Some Title", + ) + + run_theme_migration(theme, "0004-migrate-social-links-setting") + + expect(theme.settings[:social_links].value).to eq( + [{ "text" => "Some Text", "title" => "Some Title", "url" => "#", "target" => "_blank" }], + ) + end + + it "should truncate title property to 1000 chars if previous title property is more than 1000 chars" do + theme.theme_settings.create!( + name: "social_links", + theme:, + data_type: ThemeSetting.types[:string], + value: "Some Text, #{"a" * 1001}", + ) + + run_theme_migration(theme, "0004-migrate-social-links-setting") + + expect(theme.settings[:social_links].value).to eq( + [{ "text" => "Some Text", "title" => "#{"a" * 1000}", "url" => "#", "target" => "_blank" }], + ) + end + + it "should set URL property to `#` if previous URL property is invalid, empty or is more than 2048 chars" do + theme.theme_settings.create!( + name: "social_links", + theme:, + data_type: ThemeSetting.types[:string], + value: + "Some Text, Some Title|Some Text 2, Some Title 2, not a URL|Some Text 3, Some Title 3, #{"a" * 2049}", + ) + + run_theme_migration(theme, "0004-migrate-social-links-setting") + + expect(theme.settings[:social_links].value).to eq( + [ + { "text" => "Some Text", "title" => "Some Title", "url" => "#", "target" => "_blank" }, + { "text" => "Some Text 2", "title" => "Some Title 2", "url" => "#", "target" => "_blank" }, + { "text" => "Some Text 3", "title" => "Some Title 3", "url" => "#", "target" => "_blank" }, + ], + ) + end + + it "should migrate URL property correctly if previous URL property is valid" do + theme.theme_settings.create!( + name: "social_links", + theme:, + data_type: ThemeSetting.types[:string], + value: "Some Text, Some Title, /some/url|Some Text 2, Some Title 2, http://example.com", + ) + + run_theme_migration(theme, "0004-migrate-social-links-setting") + + expect(theme.settings[:social_links].value).to eq( + [ + { + "text" => "Some Text", + "title" => "Some Title", + "url" => "/some/url", + "target" => "_blank", + }, + { + "text" => "Some Text 2", + "title" => "Some Title 2", + "url" => "http://example.com", + "target" => "_blank", + }, + ], + ) + end + + it "sets target property to `_self` if previous target property is `self`" do + theme.theme_settings.create!( + name: "social_links", + theme:, + data_type: ThemeSetting.types[:string], + value: "Some Text, Some Title, /some/url, self", + ) + + run_theme_migration(theme, "0004-migrate-social-links-setting") + + expect(theme.settings[:social_links].value).to eq( + [ + { + "text" => "Some Text", + "title" => "Some Title", + "url" => "/some/url", + "target" => "_self", + }, + ], + ) + end + + it "does not set target property if previous target property is invalid or empty" do + theme.theme_settings.create!( + name: "social_links", + theme:, + data_type: ThemeSetting.types[:string], + value: + "Some Text, Some Title, /some/url|Some Text 2, Some Title 2, http://example.com, not a target", + ) + + run_theme_migration(theme, "0004-migrate-social-links-setting") + + expect(theme.settings[:social_links].value).to eq( + [ + { + "text" => "Some Text", + "title" => "Some Title", + "url" => "/some/url", + "target" => "_blank", + }, + { + "text" => "Some Text 2", + "title" => "Some Title 2", + "url" => "http://example.com", + "target" => "_blank", + }, + ], + ) + end + + it "does not set icon_name property if previous icon_name property is empty or more than 200 chars" do + theme.theme_settings.create!( + name: "social_links", + theme:, + data_type: ThemeSetting.types[:string], + value: + "Some Text, Some Title, /some/url, self|Some Text 2, Some Title 2, http://example.com, self, #{"a" * 201}", + ) + + run_theme_migration(theme, "0004-migrate-social-links-setting") + + expect(theme.settings[:social_links].value).to eq( + [ + { + "text" => "Some Text", + "title" => "Some Title", + "url" => "/some/url", + "target" => "_self", + }, + { + "text" => "Some Text 2", + "title" => "Some Title 2", + "url" => "http://example.com", + "target" => "_self", + }, + ], + ) + end + + it "sets the icon_name property correctly if previous icon_name property is present" do + theme.theme_settings.create!( + name: "social_links", + theme:, + data_type: ThemeSetting.types[:string], + value: "Some Text, Some Title, /some/url, self, some-icon", + ) + + run_theme_migration(theme, "0004-migrate-social-links-setting") + + expect(theme.settings[:social_links].value).to eq( + [ + { + "text" => "Some Text", + "title" => "Some Title", + "url" => "/some/url", + "target" => "_self", + "icon_name" => "some-icon", + }, + ], + ) + end +end diff --git a/spec/system/footer_spec.rb b/spec/system/footer_spec.rb index c8f5f6c..4c588eb 100644 --- a/spec/system/footer_spec.rb +++ b/spec/system/footer_spec.rb @@ -95,7 +95,7 @@ ) expect(page).to have_css( - "a.social-link[data-easyfooter-social-link='twitter'][title='show some love on Twitter'][href='#'][target='_blank'] .d-icon-fab-twitter", + "a.social-link[data-easyfooter-social-link='twitter'][title='Show some love on Twitter'][href='#'][target='_blank'] .d-icon-fab-twitter", ) expect(page).to have_css(