FEATURE: allow external links in custom sidebar sections (#20503)

Originally, only Discourse site links were available. After feedback, it was decided to extend this feature to external URLs.

/t/93491

app/assets/javascripts/discourse/app/components/sidebar/section-link.hbs

@@ -7,7 +7,7 @@
       <a
         href={{@href}}
         rel="noopener noreferrer"
-        target="_blank"
+        target={{this.target}}
         class={{this.classNames}}
         title={{@title}}
         ...attributes

app/assets/javascripts/discourse/app/components/sidebar/section-link.js

@@ -1,6 +1,9 @@
 import Component from "@glimmer/component";
+import { inject as service } from "@ember/service";
 
 export default class SectionLink extends Component {
+  @service currentUser;
+
   willDestroy() {
     if (this.args.willDestroy) {
       this.args.willDestroy();
@@ -35,6 +38,12 @@ export default class SectionLink extends Component {
     return classNames.join(" ");
   }
 
+  get target() {
+    return this.currentUser.user_option.external_links_in_new_tab
+      ? "_blank"
+      : "_self";
+  }
+
   get models() {
     if (this.args.model) {
       return [this.args.model];

app/assets/javascripts/discourse/app/components/sidebar/user/custom-sections.hbs

@@ -8,15 +8,25 @@
       @headerActionsIcon="pencil-alt"
     >
       {{#each section.links as |link|}}
-        <Sidebar::SectionLink
-          @linkName={{link.name}}
-          @route={{link.route}}
-          @models={{link.models}}
-          @query={{link.query}}
-          @content={{replace-emoji link.name}}
-          @prefixType="icon"
-          @prefixValue={{link.icon}}
-        />
+        {{#if link.external}}
+          <Sidebar::SectionLink
+            @linkName={{link.name}}
+            @content={{replace-emoji link.name}}
+            @prefixType="icon"
+            @prefixValue={{link.icon}}
+            @href={{link.value}}
+          />
+        {{else}}
+          <Sidebar::SectionLink
+            @linkName={{link.name}}
+            @route={{link.route}}
+            @models={{link.models}}
+            @query={{link.query}}
+            @content={{replace-emoji link.name}}
+            @prefixType="icon"
+            @prefixValue={{link.icon}}
+          />
+        {{/if}}
       {{/each}}
     </Sidebar::Section>
   {{/each}}

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;

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,
           };
         }),

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`<Sidebar::SectionLink @linkName="test" @href="https://discourse.org" />`;
+    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`<Sidebar::SectionLink @linkName="test" @href="https://discourse.org" />`;
+    await render(template);
+
+    assert.strictEqual(query("a").target, "_blank");
+  });
 });

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
 #

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."

config/site_settings.yml

@@ -2116,7 +2116,6 @@ navigation:
     default: ""
     allow_any: false
     refresh: true
-    hidden: true
 
 embedding:
   embed_by_username:

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

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

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")

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