FIX: my links in sidebar section (#20754)
Links like `/my/preferences` were invalid in custom section. The reason is that `/my` links are just redirects from backend, and they are not recognized as valid Ember paths. https://github.com/discourse/discourse/blob/main/config/routes.rb#L433 Therefore, regex match allowlist was added - similar to backend check: https://github.com/discourse/discourse/blob/main/app/controllers/users_controller.rb#L471 /safe-mode is same case
This commit is contained in:
parent
ae939f4111
commit
1859025228
|
@ -9,6 +9,8 @@ import { sanitize } from "discourse/lib/text";
|
||||||
import { tracked } from "@glimmer/tracking";
|
import { tracked } from "@glimmer/tracking";
|
||||||
import { A } from "@ember/array";
|
import { A } from "@ember/array";
|
||||||
|
|
||||||
|
const FULL_RELOAD_LINKS_REGEX = [/^\/my\/[a-z_\-\/]+$/, /^\/safe-mode$/];
|
||||||
|
|
||||||
class Section {
|
class Section {
|
||||||
@tracked title;
|
@tracked title;
|
||||||
@tracked links;
|
@tracked links;
|
||||||
|
@ -95,7 +97,10 @@ class SectionLink {
|
||||||
}
|
}
|
||||||
|
|
||||||
#validInternal() {
|
#validInternal() {
|
||||||
return this.router.recognize(this.path).name !== "unknown";
|
return (
|
||||||
|
this.router.recognize(this.path).name !== "unknown" ||
|
||||||
|
FULL_RELOAD_LINKS_REGEX.some((regex) => this.path.match(regex))
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
get validValue() {
|
get validValue() {
|
||||||
|
@ -200,7 +205,6 @@ export default Controller.extend(ModalFunctionality, {
|
||||||
icon: link.icon,
|
icon: link.icon,
|
||||||
name: link.name,
|
name: link.name,
|
||||||
value: link.path,
|
value: link.path,
|
||||||
external: link.external,
|
|
||||||
_destroy: link._destroy,
|
_destroy: link._destroy,
|
||||||
};
|
};
|
||||||
}),
|
}),
|
||||||
|
|
|
@ -1,6 +1,8 @@
|
||||||
# frozen_string_literal: true
|
# frozen_string_literal: true
|
||||||
|
|
||||||
class SidebarUrl < ActiveRecord::Base
|
class SidebarUrl < ActiveRecord::Base
|
||||||
|
FULL_RELOAD_LINKS_REGEX = [%r{\A/my/[a-z_\-/]+\z}, %r{\A/safe-mode\z}]
|
||||||
|
|
||||||
validates :icon, presence: true, length: { maximum: 40 }
|
validates :icon, presence: true, length: { maximum: 40 }
|
||||||
validates :name, presence: true, length: { maximum: 80 }
|
validates :name, presence: true, length: { maximum: 80 }
|
||||||
validates :value, presence: true, length: { maximum: 200 }
|
validates :value, presence: true, length: { maximum: 200 }
|
||||||
|
@ -11,7 +13,7 @@ class SidebarUrl < ActiveRecord::Base
|
||||||
|
|
||||||
def path_validator
|
def path_validator
|
||||||
if external?
|
if external?
|
||||||
raise ActionController::RoutingError if value !~ Discourse::Utils::URI_REGEXP
|
raise ActionController::RoutingError.new("Not Found") if value !~ Discourse::Utils::URI_REGEXP
|
||||||
else
|
else
|
||||||
Rails.application.routes.recognize_path(value)
|
Rails.application.routes.recognize_path(value)
|
||||||
end
|
end
|
||||||
|
@ -29,6 +31,10 @@ class SidebarUrl < ActiveRecord::Base
|
||||||
def set_external
|
def set_external
|
||||||
self.external = value.start_with?("http://", "https://")
|
self.external = value.start_with?("http://", "https://")
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def full_reload?
|
||||||
|
FULL_RELOAD_LINKS_REGEX.any? { |regex| value =~ regex }
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
# == Schema Information
|
# == Schema Information
|
||||||
|
|
|
@ -4,7 +4,7 @@ class SidebarSectionSerializer < ApplicationSerializer
|
||||||
attributes :id, :title, :links, :slug, :public
|
attributes :id, :title, :links, :slug, :public
|
||||||
|
|
||||||
def links
|
def links
|
||||||
object.sidebar_section_links.map(&:linkable)
|
object.sidebar_section_links.map { |link| SidebarUrlSerializer.new(link.linkable, root: false) }
|
||||||
end
|
end
|
||||||
|
|
||||||
def slug
|
def slug
|
||||||
|
|
|
@ -0,0 +1,9 @@
|
||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
|
class SidebarUrlSerializer < ApplicationSerializer
|
||||||
|
attributes :id, :name, :value, :icon, :external
|
||||||
|
|
||||||
|
def external
|
||||||
|
object.external? || object.full_reload?
|
||||||
|
end
|
||||||
|
end
|
|
@ -61,6 +61,7 @@ RSpec.describe SidebarSectionsController do
|
||||||
},
|
},
|
||||||
{ icon: "address-book", name: "tags", value: "/tags" },
|
{ icon: "address-book", name: "tags", value: "/tags" },
|
||||||
{ icon: "external-link-alt", name: "Discourse", value: "https://discourse.org" },
|
{ icon: "external-link-alt", name: "Discourse", value: "https://discourse.org" },
|
||||||
|
{ icon: "external-link-alt", name: "My preferences", value: "/my/preferences" },
|
||||||
],
|
],
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -73,7 +74,7 @@ RSpec.describe SidebarSectionsController do
|
||||||
expect(sidebar_section.user).to eq(user)
|
expect(sidebar_section.user).to eq(user)
|
||||||
expect(sidebar_section.public).to be false
|
expect(sidebar_section.public).to be false
|
||||||
expect(UserHistory.count).to eq(0)
|
expect(UserHistory.count).to eq(0)
|
||||||
expect(sidebar_section.sidebar_urls.count).to eq(3)
|
expect(sidebar_section.sidebar_urls.count).to eq(4)
|
||||||
expect(sidebar_section.sidebar_urls.first.icon).to eq("link")
|
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.name).to eq("categories")
|
||||||
expect(sidebar_section.sidebar_urls.first.value).to eq("/categories")
|
expect(sidebar_section.sidebar_urls.first.value).to eq("/categories")
|
||||||
|
@ -86,6 +87,10 @@ RSpec.describe SidebarSectionsController do
|
||||||
expect(sidebar_section.sidebar_urls.third.name).to eq("Discourse")
|
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.value).to eq("https://discourse.org")
|
||||||
expect(sidebar_section.sidebar_urls.third.external).to be true
|
expect(sidebar_section.sidebar_urls.third.external).to be true
|
||||||
|
expect(sidebar_section.sidebar_urls.fourth.icon).to eq("external-link-alt")
|
||||||
|
expect(sidebar_section.sidebar_urls.fourth.name).to eq("My preferences")
|
||||||
|
expect(sidebar_section.sidebar_urls.fourth.value).to eq("/my/preferences")
|
||||||
|
expect(sidebar_section.sidebar_urls.fourth.external).to be false
|
||||||
end
|
end
|
||||||
|
|
||||||
it "does not allow regular user to create public section" do
|
it "does not allow regular user to create public section" do
|
||||||
|
|
|
@ -34,6 +34,25 @@ describe "Custom sidebar sections", type: :system, js: true do
|
||||||
expect(sidebar).to have_link("Sidebar Tags")
|
expect(sidebar).to have_link("Sidebar Tags")
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "allows the user to create custom section with /my 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("My preferences", "/my/preferences")
|
||||||
|
expect(section_modal).to have_enabled_save
|
||||||
|
|
||||||
|
section_modal.save
|
||||||
|
|
||||||
|
expect(page).to have_button("My section")
|
||||||
|
expect(sidebar).to have_link("My preferences")
|
||||||
|
end
|
||||||
|
|
||||||
it "allows the user to create custom section with external link" do
|
it "allows the user to create custom section with external link" do
|
||||||
visit("/latest")
|
visit("/latest")
|
||||||
sidebar.open_new_custom_section
|
sidebar.open_new_custom_section
|
||||||
|
|
Loading…
Reference in New Issue