From e586f6052fc75fa7e65b93a1304316ae4aff4851 Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Thu, 6 Apr 2023 08:55:47 +1000 Subject: [PATCH] FEATURE: public custom sidebar sections visible to anonymous (#20931) Previously, public custom sections were only visible to logged-in users. In this PR, we are making them visible to anonymous as well. The reason is that Community Section will be moved into custom section model to be easily editable by admins. --- .../sidebar/anonymous/custom-sections.hbs | 31 +++++++++ .../sidebar/anonymous/custom-sections.js | 17 +++++ .../components/sidebar/anonymous/sections.hbs | 1 + .../app/components/sidebar/user/section.js | 4 +- .../sidebar_sections_controller.rb | 2 + app/models/sidebar_section.rb | 2 + app/serializers/current_user_serializer.rb | 3 +- app/serializers/site_serializer.rb | 12 ++++ spec/serializers/site_serializer_spec.rb | 64 +++++++++++++++++++ spec/system/custom_sidebar_sections_spec.rb | 42 ++++++++---- .../system/page_objects/components/sidebar.rb | 4 ++ 11 files changed, 168 insertions(+), 14 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/components/sidebar/anonymous/custom-sections.hbs create mode 100644 app/assets/javascripts/discourse/app/components/sidebar/anonymous/custom-sections.js diff --git a/app/assets/javascripts/discourse/app/components/sidebar/anonymous/custom-sections.hbs b/app/assets/javascripts/discourse/app/components/sidebar/anonymous/custom-sections.hbs new file mode 100644 index 00000000000..80da8163d41 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/sidebar/anonymous/custom-sections.hbs @@ -0,0 +1,31 @@ +
+ {{#each this.sections as |section|}} + + {{#each section.links as |link|}} + {{#if link.external}} + + {{else}} + + {{/if}} + {{/each}} + + {{/each}} +
\ No newline at end of file diff --git a/app/assets/javascripts/discourse/app/components/sidebar/anonymous/custom-sections.js b/app/assets/javascripts/discourse/app/components/sidebar/anonymous/custom-sections.js new file mode 100644 index 00000000000..6d7fa593dd1 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/sidebar/anonymous/custom-sections.js @@ -0,0 +1,17 @@ +import Component from "@glimmer/component"; +import { inject as service } from "@ember/service"; +import Section from "discourse/components/sidebar/user/section"; + +export default class SidebarAnonymousCustomSections extends Component { + @service router; + @service site; + + get sections() { + return this.site.anonymous_sidebar_sections?.map((section) => { + return new Section({ + section, + router: this.router, + }); + }); + } +} diff --git a/app/assets/javascripts/discourse/app/components/sidebar/anonymous/sections.hbs b/app/assets/javascripts/discourse/app/components/sidebar/anonymous/sections.hbs index aee22d9ebbf..ced0920e93c 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/anonymous/sections.hbs +++ b/app/assets/javascripts/discourse/app/components/sidebar/anonymous/sections.hbs @@ -1,5 +1,6 @@
+ diff --git a/app/assets/javascripts/discourse/app/components/sidebar/user/section.js b/app/assets/javascripts/discourse/app/components/sidebar/user/section.js index bafa6b37f59..da4a2b324f5 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/user/section.js +++ b/app/assets/javascripts/discourse/app/components/sidebar/user/section.js @@ -23,13 +23,13 @@ export default class Section { } get decoratedTitle() { - return this.section.public && this.currentUser.staff + return this.section.public && this.currentUser?.staff ? htmlSafe(`${iconHTML("globe")} ${this.section.title}`) : this.section.title; } get headerActions() { - if (!this.section.public || this.currentUser.staff) { + if (!this.section.public || this.currentUser?.staff) { return [ { action: () => { diff --git a/app/controllers/sidebar_sections_controller.rb b/app/controllers/sidebar_sections_controller.rb index e0607d4ffd6..32deeba6f4f 100644 --- a/app/controllers/sidebar_sections_controller.rb +++ b/app/controllers/sidebar_sections_controller.rb @@ -25,6 +25,7 @@ class SidebarSectionsController < ApplicationController nil, group_ids: SiteSetting.enable_custom_sidebar_sections_map, ) + Site.clear_anon_cache! end render json: SidebarSectionSerializer.new(sidebar_section) @@ -48,6 +49,7 @@ class SidebarSectionsController < ApplicationController nil, group_ids: SiteSetting.enable_custom_sidebar_sections_map, ) + Site.clear_anon_cache! end render json: SidebarSectionSerializer.new(sidebar_section) diff --git a/app/models/sidebar_section.rb b/app/models/sidebar_section.rb index 22549ed6b06..c59d9c0919a 100644 --- a/app/models/sidebar_section.rb +++ b/app/models/sidebar_section.rb @@ -23,6 +23,8 @@ class SidebarSection < ActiveRecord::Base maximum: MAX_TITLE_LENGTH, } + scope :public_sections, -> { where("public") } + private def set_system_user_for_public_section diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb index ddd2d0dfa5b..fa4664b4259 100644 --- a/app/serializers/current_user_serializer.rb +++ b/app/serializers/current_user_serializer.rb @@ -79,8 +79,9 @@ class CurrentUserSerializer < BasicUserSerializer def sidebar_sections SidebarSection + .public_sections + .or(SidebarSection.where(user_id: object.id)) .includes(sidebar_section_links: :linkable) - .where("public OR user_id = ?", object.id) .order("(public IS TRUE) DESC") .map { |section| SidebarSectionSerializer.new(section, root: false) } end diff --git a/app/serializers/site_serializer.rb b/app/serializers/site_serializer.rb index 4aa21862328..c518e0cf9b1 100644 --- a/app/serializers/site_serializer.rb +++ b/app/serializers/site_serializer.rb @@ -39,6 +39,7 @@ class SiteSerializer < ApplicationSerializer :displayed_about_plugin_stat_groups, :show_welcome_topic_banner, :anonymous_default_sidebar_tags, + :anonymous_sidebar_sections, :whispers_allowed_groups_names, ) @@ -260,6 +261,17 @@ class SiteSerializer < ApplicationSerializer SiteSetting.default_sidebar_tags.present? end + def anonymous_sidebar_sections + SidebarSection + .public_sections + .includes(sidebar_section_links: :linkable) + .map { |section| SidebarSectionSerializer.new(section, root: false) } + end + + def include_anonymous_sidebar_sections? + scope.anonymous? + end + def whispers_allowed_groups_names Group.where(id: SiteSetting.whispers_allowed_groups_map).pluck(:name) end diff --git a/spec/serializers/site_serializer_spec.rb b/spec/serializers/site_serializer_spec.rb index 0fb21110d35..46713b9139b 100644 --- a/spec/serializers/site_serializer_spec.rb +++ b/spec/serializers/site_serializer_spec.rb @@ -192,6 +192,70 @@ RSpec.describe SiteSerializer do end end + describe "#anonymous_sidebar_sections" do + fab!(:user) { Fabricate(:user) } + fab!(:public_sidebar_section) do + Fabricate(:sidebar_section, title: "Public section", public: true) + end + fab!(:private_sidebar_section) do + Fabricate(:sidebar_section, title: "Private section", user: user, public: false) + end + + it "is not included in the serialised object when user is not anonymous" do + guardian = Guardian.new(user) + + serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json + end + + it "includes only public sidebar sections serialised object when user is anonymous" do + serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json + expect(serialized[:anonymous_sidebar_sections].map(&:title)).to eq(["Public section"]) + end + + it "eager loads sidebar_urls" do + public_section_link = + Fabricate(:custom_sidebar_section_link, user: user, sidebar_section: public_sidebar_section) + # warmup + described_class.new(Site.new(guardian), scope: guardian, root: false).as_json + + initial_count = + track_sql_queries do + serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json + + expect( + serialized[:anonymous_sidebar_sections].map { |sidebar_section| sidebar_section.id }, + ).to eq([public_sidebar_section.id]) + + expect(serialized[:anonymous_sidebar_sections].first.links.map { |link| link.id }).to eq( + [public_section_link.linkable.id], + ) + end.count + + public_section_link_2 = + Fabricate(:custom_sidebar_section_link, user: user, sidebar_section: public_sidebar_section) + public_section_link_3 = + Fabricate(:custom_sidebar_section_link, user: user, sidebar_section: public_sidebar_section) + + final_count = + track_sql_queries do + serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json + + expect( + serialized[:anonymous_sidebar_sections].map { |sidebar_section| sidebar_section.id }, + ).to eq([public_sidebar_section.id]) + + expect(serialized[:anonymous_sidebar_sections].first.links.map { |link| link.id }).to eq( + [ + public_section_link.linkable.id, + public_section_link_2.linkable.id, + public_section_link_3.linkable.id, + ], + ) + end.count + expect(final_count).to eq(initial_count) + end + end + describe "#top_tags" do fab!(:tag) { Fabricate(:tag) } diff --git a/spec/system/custom_sidebar_sections_spec.rb b/spec/system/custom_sidebar_sections_spec.rb index 20bd1114eef..2ce6949ab2a 100644 --- a/spec/system/custom_sidebar_sections_spec.rb +++ b/spec/system/custom_sidebar_sections_spec.rb @@ -12,10 +12,10 @@ describe "Custom sidebar sections", type: :system, js: true do Fabricate(:group_user, group: group, user: user) Fabricate(:group_user, group: group, user: admin) SiteSetting.enable_custom_sidebar_sections = group.id.to_s - sign_in user end it "allows the user to create custom section" do + sign_in user visit("/latest") sidebar.open_new_custom_section @@ -30,11 +30,12 @@ describe "Custom sidebar sections", type: :system, js: true do section_modal.save - expect(page).to have_button("My section") + expect(sidebar).to have_section("My section") expect(sidebar).to have_link("Sidebar Tags") end it "allows the user to create custom section with /my link" do + sign_in user visit("/latest") sidebar.open_new_custom_section @@ -49,11 +50,12 @@ describe "Custom sidebar sections", type: :system, js: true do section_modal.save - expect(page).to have_button("My section") + expect(sidebar).to have_section("My section") expect(sidebar).to have_link("My preferences") end it "allows the user to create custom section with external link" do + sign_in user visit("/latest") sidebar.open_new_custom_section @@ -71,7 +73,7 @@ describe "Custom sidebar sections", type: :system, js: true do section_modal.save - expect(page).to have_button("My section") + expect(sidebar).to have_section("My section") expect(sidebar).to have_link("Discourse Homepage", href: "https://discourse.org") end @@ -82,6 +84,7 @@ describe "Custom sidebar sections", type: :system, js: true do sidebar_url_2 = Fabricate(:sidebar_url, name: "Sidebar Categories", value: "/categories") Fabricate(:sidebar_section_link, sidebar_section: sidebar_section, linkable: sidebar_url_2) + sign_in user visit("/latest") sidebar.edit_custom_section("My section") @@ -93,7 +96,7 @@ describe "Custom sidebar sections", type: :system, js: true do section_modal.save - expect(page).to have_button("Edited section") + expect(sidebar).to have_section("Edited section") expect(sidebar).to have_link("Edited Tag") expect(page).not_to have_link("Sidebar Categories") @@ -106,6 +109,7 @@ describe "Custom sidebar sections", type: :system, js: true do sidebar_url_2 = Fabricate(:sidebar_url, name: "Sidebar Categories", value: "/categories") Fabricate(:sidebar_section_link, sidebar_section: sidebar_section, linkable: sidebar_url_2) + sign_in user visit("/latest") within(".sidebar-custom-sections .sidebar-section-link-wrapper:nth-child(1)") do expect(page).to have_css(".sidebar-section-link-sidebar-tags") @@ -127,15 +131,16 @@ describe "Custom sidebar sections", type: :system, js: true do end it "does not allow the user to edit public section" do - sidebar_section = Fabricate(:sidebar_section, title: "Public section", user: user, public: true) + sidebar_section = Fabricate(:sidebar_section, title: "Public section", public: true) sidebar_url_1 = Fabricate(:sidebar_url, name: "Sidebar Tags", value: "/tags") Fabricate(:sidebar_section_link, sidebar_section: sidebar_section, linkable: sidebar_url_1) sidebar_url_2 = Fabricate(:sidebar_url, name: "Sidebar Categories", value: "/categories") Fabricate(:sidebar_section_link, sidebar_section: sidebar_section, linkable: sidebar_url_2) + sign_in user visit("/latest") - expect(page).to have_button("Public section") + expect(sidebar).to have_section("Public section") find(".sidebar-section[data-section-name='public-section']").hover @@ -153,6 +158,7 @@ describe "Custom sidebar sections", type: :system, js: true do sidebar_url_1 = Fabricate(:sidebar_url, name: "tags", value: "/tags") Fabricate(:sidebar_section_link, sidebar_section: sidebar_section, linkable: sidebar_url_1) + sign_in user visit("/latest") sidebar.edit_custom_section("My section") @@ -160,7 +166,7 @@ describe "Custom sidebar sections", type: :system, js: true do section_modal.delete section_modal.confirm_delete - expect(page).not_to have_button("My section") + expect(sidebar).not_to have_section("My section") end it "allows admin to create, edit and delete public section" do @@ -173,7 +179,7 @@ describe "Custom sidebar sections", type: :system, js: true do section_modal.mark_as_public section_modal.save - expect(page).to have_button("Public section") + expect(sidebar).to have_section("Public section") expect(sidebar).to have_link("Sidebar Tags") expect(page).to have_css(".sidebar-section[data-section-name='public-section'] .d-icon-globe") @@ -181,16 +187,30 @@ describe "Custom sidebar sections", type: :system, js: true do section_modal.fill_name("Edited public section") section_modal.save - expect(page).to have_button("Edited public section") + expect(sidebar).to have_section("Edited public section") sidebar.edit_custom_section("Edited public section") section_modal.delete section_modal.confirm_delete - expect(page).not_to have_button("Edited public section") + expect(sidebar).not_to have_section("Edited public section") + end + + it "shows anonymous public sections" do + sidebar_section = Fabricate(:sidebar_section, title: "Public section", public: true) + sidebar_url_1 = Fabricate(:sidebar_url, name: "Sidebar Tags", value: "/tags") + Fabricate(:sidebar_section_link, sidebar_section: sidebar_section, linkable: sidebar_url_1) + sidebar_url_2 = Fabricate(:sidebar_url, name: "Sidebar Categories", value: "/categories") + Fabricate(:sidebar_section_link, sidebar_section: sidebar_section, linkable: sidebar_url_2) + + visit("/latest") + expect(sidebar).to have_section("Public section") + expect(sidebar).to have_link("Sidebar Tags") + expect(sidebar).to have_link("Sidebar Categories") end it "validates custom section fields" do + sign_in user visit("/latest") sidebar.open_new_custom_section diff --git a/spec/system/page_objects/components/sidebar.rb b/spec/system/page_objects/components/sidebar.rb index 73c1e2e0e24..dd4231ceb4f 100644 --- a/spec/system/page_objects/components/sidebar.rb +++ b/spec/system/page_objects/components/sidebar.rb @@ -36,6 +36,10 @@ module PageObjects def custom_section_modal_title find("#discourse-modal-title") end + + def has_section?(name) + find(".sidebar-wrapper").has_button?(name) + end end end end