diff --git a/app/assets/javascripts/discourse/app/components/sidebar/section-link.hbs b/app/assets/javascripts/discourse/app/components/sidebar/section-link.hbs index 4d3ba543b5b..790ea1a7f76 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/section-link.hbs +++ b/app/assets/javascripts/discourse/app/components/sidebar/section-link.hbs @@ -7,7 +7,7 @@ {{#each section.links as |link|}} - + {{#if link.external}} + + {{else}} + + {{/if}} {{/each}} {{/each}} diff --git a/app/assets/javascripts/discourse/app/components/sidebar/user/custom-sections.js b/app/assets/javascripts/discourse/app/components/sidebar/user/custom-sections.js index 1d3f943bbc5..96d69dffd14 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/user/custom-sections.js +++ b/app/assets/javascripts/discourse/app/components/sidebar/user/custom-sections.js @@ -39,10 +39,12 @@ export default class SidebarUserCustomSections extends Component { ? htmlSafe(`${iconHTML("globe")} ${section.title}`) : section.title; section.links.forEach((link) => { - const routeInfoHelper = new RouteInfoHelper(this.router, link.value); - link.route = routeInfoHelper.route; - link.models = routeInfoHelper.models; - link.query = routeInfoHelper.query; + if (!link.external) { + const routeInfoHelper = new RouteInfoHelper(this.router, link.value); + link.route = routeInfoHelper.route; + link.models = routeInfoHelper.models; + link.query = routeInfoHelper.query; + } }); }); return this.currentUser.sidebarSections; diff --git a/app/assets/javascripts/discourse/app/controllers/sidebar-section-form.js b/app/assets/javascripts/discourse/app/controllers/sidebar-section-form.js index ec2c8605cbd..6ec376db61a 100644 --- a/app/assets/javascripts/discourse/app/controllers/sidebar-section-form.js +++ b/app/assets/javascripts/discourse/app/controllers/sidebar-section-form.js @@ -45,16 +45,14 @@ class SectionLink { this.router = router; this.icon = icon || "link"; this.name = name; - this.value = value ? `${this.protocolAndHost}${value}` : value; + this.value = value; this.id = id; - } - - get protocolAndHost() { - return window.location.protocol + "//" + window.location.host; + this.httpHost = "http://" + window.location.host; + this.httpsHost = "https://" + window.location.host; } get path() { - return this.value?.replace(this.protocolAndHost, ""); + return this.value?.replace(this.httpHost, "").replace(this.httpsHost, ""); } get valid() { @@ -77,14 +75,35 @@ class SectionLink { return this.name === undefined || this.validName ? "" : "warning"; } + get external() { + return ( + this.value && + !( + this.value.startsWith(this.httpHost) || + this.value.startsWith(this.httpsHost) || + this.value.startsWith("/") + ) + ); + } + + #validExternal() { + try { + return new URL(this.value); + } catch { + return false; + } + } + + #validInternal() { + return this.router.recognize(this.path).name !== "unknown"; + } + get validValue() { return ( !isEmpty(this.value) && - (this.value.startsWith(this.protocolAndHost) || - this.value.startsWith("/")) && this.value.length <= 200 && this.path && - this.router.recognize(this.path).name !== "unknown" + (this.external ? this.#validExternal() : this.#validInternal()) ); } @@ -178,6 +197,7 @@ export default Controller.extend(ModalFunctionality, { icon: link.icon, name: link.name, value: link.path, + external: link.external, _destroy: link._destroy, }; }), diff --git a/app/assets/javascripts/discourse/tests/integration/components/sidebar/section-link-test.js b/app/assets/javascripts/discourse/tests/integration/components/sidebar/section-link-test.js index 28cafd27e8d..d50446288e1 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/sidebar/section-link-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/sidebar/section-link-test.js @@ -54,4 +54,19 @@ module("Integration | Component | sidebar | section-link", function (hooks) { "has the right class attribute for the link" ); }); + + test("target attribute for link", async function (assert) { + const template = hbs``; + await render(template); + + assert.strictEqual(query("a").target, "_self"); + }); + + test("target attribute for link when user set external links in new tab", async function (assert) { + this.currentUser.user_option.external_links_in_new_tab = true; + const template = hbs``; + await render(template); + + assert.strictEqual(query("a").target, "_blank"); + }); }); diff --git a/app/models/sidebar_url.rb b/app/models/sidebar_url.rb index 3558ca0777a..c173fcc8580 100644 --- a/app/models/sidebar_url.rb +++ b/app/models/sidebar_url.rb @@ -7,14 +7,28 @@ class SidebarUrl < ActiveRecord::Base validate :path_validator + before_save :remove_internal_hostname, :set_external + def path_validator - Rails.application.routes.recognize_path(value) + if external? + raise ActionController::RoutingError if value !~ Discourse::Utils::URI_REGEXP + else + Rails.application.routes.recognize_path(value) + end rescue ActionController::RoutingError errors.add( :value, I18n.t("activerecord.errors.models.sidebar_section_link.attributes.linkable_type.invalid"), ) end + + def remove_internal_hostname + self.value = self.value.sub(%r{\Ahttp(s)?://#{Discourse.current_hostname}}, "") + end + + def set_external + self.external = value.start_with?("http://", "https://") + end end # == Schema Information @@ -27,4 +41,5 @@ end # created_at :datetime not null # updated_at :datetime not null # icon :string(40) not null +# external :boolean default(FALSE), not null # diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 63551b6cdf4..289db78cb00 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2399,6 +2399,7 @@ en: enable_new_notifications_menu: "Enables the new notifications menu for the legacy navigation menu." enable_experimental_hashtag_autocomplete: "EXPERIMENTAL: Use the new #hashtag autocompletion system for categories and tags that renders the selected item differently and has improved search" experimental_new_new_view_groups: "EXPERIMENTAL: Enable a new topics list that combines unread and new topics and make the \"Everything\" link in the sidebar link to it." + enable_custom_sidebar_sections: "EXPERIMENTAL: Enable custom sidebar sections" errors: invalid_css_color: "Invalid color. Enter a color name or hex value." diff --git a/config/site_settings.yml b/config/site_settings.yml index 1e5f21c43ca..b27fecb0499 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -2116,7 +2116,6 @@ navigation: default: "" allow_any: false refresh: true - hidden: true embedding: embed_by_username: diff --git a/db/migrate/20230303015952_add_external_to_sidebar_urls.rb b/db/migrate/20230303015952_add_external_to_sidebar_urls.rb new file mode 100644 index 00000000000..fbf1bf5bd51 --- /dev/null +++ b/db/migrate/20230303015952_add_external_to_sidebar_urls.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddExternalToSidebarUrls < ActiveRecord::Migration[7.0] + def change + add_column :sidebar_urls, :external, :boolean, default: false, null: false + end +end diff --git a/spec/requests/sidebar_sections_controller_spec.rb b/spec/requests/sidebar_sections_controller_spec.rb index 4c8ebf12cf3..a975ecb4bb6 100644 --- a/spec/requests/sidebar_sections_controller_spec.rb +++ b/spec/requests/sidebar_sections_controller_spec.rb @@ -54,8 +54,13 @@ RSpec.describe SidebarSectionsController do params: { title: "custom section", links: [ - { icon: "link", name: "categories", value: "/categories" }, + { + icon: "link", + name: "categories", + value: "http://#{Discourse.current_hostname}/categories", + }, { icon: "address-book", name: "tags", value: "/tags" }, + { icon: "external-link-alt", name: "Discourse", value: "https://discourse.org" }, ], } @@ -68,13 +73,19 @@ RSpec.describe SidebarSectionsController do expect(sidebar_section.user).to eq(user) expect(sidebar_section.public).to be false expect(UserHistory.count).to eq(0) - expect(sidebar_section.sidebar_urls.count).to eq(2) + expect(sidebar_section.sidebar_urls.count).to eq(3) expect(sidebar_section.sidebar_urls.first.icon).to eq("link") expect(sidebar_section.sidebar_urls.first.name).to eq("categories") expect(sidebar_section.sidebar_urls.first.value).to eq("/categories") + expect(sidebar_section.sidebar_urls.first.external).to be false expect(sidebar_section.sidebar_urls.second.icon).to eq("address-book") expect(sidebar_section.sidebar_urls.second.name).to eq("tags") expect(sidebar_section.sidebar_urls.second.value).to eq("/tags") + expect(sidebar_section.sidebar_urls.second.external).to be false + expect(sidebar_section.sidebar_urls.third.icon).to eq("external-link-alt") + expect(sidebar_section.sidebar_urls.third.name).to eq("Discourse") + expect(sidebar_section.sidebar_urls.third.value).to eq("https://discourse.org") + expect(sidebar_section.sidebar_urls.third.external).to be true end it "does not allow regular user to create public section" do diff --git a/spec/system/custom_sidebar_sections_spec.rb b/spec/system/custom_sidebar_sections_spec.rb index 4d54a634638..1926e91bc12 100644 --- a/spec/system/custom_sidebar_sections_spec.rb +++ b/spec/system/custom_sidebar_sections_spec.rb @@ -21,7 +21,7 @@ describe "Custom sidebar sections", type: :system, js: true do expect(section_modal).to be_visible expect(section_modal).to have_disabled_save - expect(find("#discourse-modal-title")).to have_content("Add custom section") + expect(sidebar.custom_section_modal_title).to have_content("Add custom section") section_modal.fill_name("My section") @@ -31,7 +31,29 @@ describe "Custom sidebar sections", type: :system, js: true do section_modal.save expect(page).to have_button("My section") - expect(page).to have_link("Sidebar Tags") + expect(sidebar).to have_link("Sidebar Tags") + end + + it "allows the user to create custom section with external link" do + visit("/latest") + sidebar.open_new_custom_section + + expect(section_modal).to be_visible + expect(section_modal).to have_disabled_save + expect(sidebar.custom_section_modal_title).to have_content("Add custom section") + + section_modal.fill_name("My section") + + section_modal.fill_link("Discourse Homepage", "htt") + expect(section_modal).to have_disabled_save + + section_modal.fill_link("Discourse Homepage", "https://discourse.org") + expect(section_modal).to have_enabled_save + + section_modal.save + + expect(page).to have_button("My section") + expect(sidebar).to have_link("Discourse Homepage", href: "https://discourse.org") end it "allows the user to edit custom section" do @@ -53,7 +75,8 @@ describe "Custom sidebar sections", type: :system, js: true do section_modal.save expect(page).to have_button("Edited section") - expect(page).to have_link("Edited Tags") + expect(sidebar).to have_link("Edited Tag") + expect(page).not_to have_link("Sidebar Categories") end @@ -100,7 +123,7 @@ describe "Custom sidebar sections", type: :system, js: true do section_modal.save expect(page).to have_button("Public section") - expect(page).to have_link("Sidebar Tags") + expect(sidebar).to have_link("Sidebar Tags") expect(page).to have_css(".sidebar-section-public-section .d-icon-globe") sidebar.edit_custom_section("Public section") diff --git a/spec/system/page_objects/components/sidebar.rb b/spec/system/page_objects/components/sidebar.rb index 3ddb24cb545..bce6d559d35 100644 --- a/spec/system/page_objects/components/sidebar.rb +++ b/spec/system/page_objects/components/sidebar.rb @@ -23,6 +23,16 @@ module PageObjects find(".sidebar-section-#{name.parameterize}").hover find(".sidebar-section-#{name.parameterize} button.sidebar-section-header-button").click end + + def has_link?(name, href: nil) + attributes = {} + attributes[:href] = href if href + page.has_link?(name, attributes) + end + + def custom_section_modal_title + find("#discourse-modal-title") + end end end end