From f5af4936d4fad3a073ada108ad109a968eee53ad Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Wed, 21 Jun 2023 14:37:40 +0800 Subject: [PATCH] FIX: Site's top tags not shown for anonymous user (#22219) When a site does not have `default_navigation_menu_tags` site setting set, anonymous users should be shown the site's top tags as a default in the tags section. However, this regressed in 9fad71809c254752479b0faf4f23ba197e742e60 and we ended up showing anonymous users a tags section with only the `All Tags` section link. As part of this commit, I have also refactored the QUnit acceptance tests to system tests which are much easier to work with. --- .../sidebar-anonymous-tags-section-test.js | 84 ------------------- app/serializers/site_serializer.rb | 6 +- .../system/page_objects/components/sidebar.rb | 21 +++++ .../viewing_sidebar_as_anonymous_user_spec.rb | 59 +++++++++++++ 4 files changed, 84 insertions(+), 86 deletions(-) delete mode 100644 app/assets/javascripts/discourse/tests/acceptance/sidebar-anonymous-tags-section-test.js create mode 100644 spec/system/viewing_sidebar_as_anonymous_user_spec.rb diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-anonymous-tags-section-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-anonymous-tags-section-test.js deleted file mode 100644 index b777ad6b4c4..00000000000 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-anonymous-tags-section-test.js +++ /dev/null @@ -1,84 +0,0 @@ -import { test } from "qunit"; -import { visit } from "@ember/test-helpers"; -import { - acceptance, - exists, - queryAll, -} from "discourse/tests/helpers/qunit-helpers"; -import Site from "discourse/models/site"; - -acceptance("Sidebar - Anonymous Tags Section", function (needs) { - needs.settings({ - navigation_menu: "sidebar", - suppress_uncategorized_badge: false, - tagging_enabled: true, - }); - - needs.site({ - top_tags: ["design", "development", "fun"], - }); - - test("tag section links when site has top tags", async function (assert) { - await visit("/"); - - const categories = queryAll( - ".sidebar-section[data-section-name='tags'] .sidebar-section-link-wrapper[data-tag-name]" - ); - - assert.strictEqual(categories.length, 3); - assert.strictEqual(categories[0].textContent.trim(), "design"); - assert.strictEqual(categories[1].textContent.trim(), "development"); - assert.strictEqual(categories[2].textContent.trim(), "fun"); - - assert.ok( - exists("a.sidebar-section-link[data-link-name='all-tags']"), - "all tags link is visible" - ); - }); - - test("tag section links when site has default sidebar tags configured", async function (assert) { - const site = Site.current(); - site.set("anonymous_default_navigation_menu_tags", ["random", "meta"]); - - await visit("/"); - - const categories = queryAll( - ".sidebar-section[data-section-name='tags'] .sidebar-section-link-wrapper" - ); - assert.strictEqual(categories.length, 3); - assert.strictEqual(categories[0].textContent.trim(), "random"); - assert.strictEqual(categories[1].textContent.trim(), "meta"); - - assert.ok( - exists("a.sidebar-section-link[data-link-name='all-tags']"), - "all tags link is visible" - ); - }); - - test("tag section is hidden when tagging is disabled", async function (assert) { - this.siteSettings.tagging_enabled = false; - - await visit("/"); - - assert.ok( - !exists(".sidebar-section[data-section-name='tags']"), - "section is not visible" - ); - }); - - test("tag section is hidden when anonymous has no visible top tags and site has not default sidebar tags configured", async function (assert) { - const site = Site.current(); - - site.setProperties({ - top_tags: [], - anonymous_default_navigation_menu_tags: [], - }); - - await visit("/"); - - assert.ok( - !exists(".sidebar-section[data-section-name='tags']"), - "section is not visible" - ); - }); -}); diff --git a/app/serializers/site_serializer.rb b/app/serializers/site_serializer.rb index 7ba2a793095..394e087298c 100644 --- a/app/serializers/site_serializer.rb +++ b/app/serializers/site_serializer.rb @@ -251,12 +251,14 @@ class SiteSerializer < ApplicationSerializer end def anonymous_default_navigation_menu_tags - SiteSetting.default_navigation_menu_tags.split("|") - DiscourseTagging.hidden_tag_names(scope) + @anonymous_default_navigation_menu_tags ||= + SiteSetting.default_navigation_menu_tags.split("|") - DiscourseTagging.hidden_tag_names(scope) end def include_anonymous_default_navigation_menu_tags? scope.anonymous? && !SiteSetting.legacy_navigation_menu? && SiteSetting.tagging_enabled && - SiteSetting.default_navigation_menu_tags.present? + SiteSetting.default_navigation_menu_tags.present? && + anonymous_default_navigation_menu_tags.present? end def anonymous_sidebar_sections diff --git a/spec/system/page_objects/components/sidebar.rb b/spec/system/page_objects/components/sidebar.rb index ac155f332e1..9536656b279 100644 --- a/spec/system/page_objects/components/sidebar.rb +++ b/spec/system/page_objects/components/sidebar.rb @@ -83,6 +83,27 @@ module PageObjects has_section?("Tags") end + def has_no_tags_section? + has_no_section?("Tags") + end + + def has_all_tags_section_link? + has_section_link?(I18n.t("js.sidebar.all_tags")) + end + + def has_tags_section_links?(tags) + section_selector = ".sidebar-section[data-section-name='tags']" + tag_names = tags.map(&:name) + + has_css?( + "#{section_selector} .sidebar-section-link-wrapper[data-tag-name]", + count: tag_names.length, + ) && + all("#{section_selector} .sidebar-section-link-wrapper[data-tag-name]").all? do |row| + tag_names.include?(row["data-tag-name"].to_s) + end + end + def has_no_section?(name) find(SIDEBAR_WRAPPER_SELECTOR).has_no_button?(name) end diff --git a/spec/system/viewing_sidebar_as_anonymous_user_spec.rb b/spec/system/viewing_sidebar_as_anonymous_user_spec.rb new file mode 100644 index 00000000000..26df95bee23 --- /dev/null +++ b/spec/system/viewing_sidebar_as_anonymous_user_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +describe "Viewing sidebar as anonymous user", type: :system do + fab!(:tag1) { Fabricate(:tag).tap { |tag| Fabricate.times(1, :topic, tags: [tag]) } } + fab!(:tag2) { Fabricate(:tag).tap { |tag| Fabricate.times(2, :topic, tags: [tag]) } } + fab!(:tag3) { Fabricate(:tag).tap { |tag| Fabricate.times(3, :topic, tags: [tag]) } } + fab!(:tag4) { Fabricate(:tag).tap { |tag| Fabricate.times(2, :topic, tags: [tag]) } } + fab!(:tag5) { Fabricate(:tag).tap { |tag| Fabricate.times(2, :topic, tags: [tag]) } } + fab!(:tag6) { Fabricate(:tag).tap { |tag| Fabricate.times(1, :topic, tags: [tag]) } } + + let(:sidebar) { PageObjects::Components::Sidebar.new } + + describe "when viewing the tags section" do + it "should not display the tags section when tagging has been disabled" do + SiteSetting.tagging_enabled = false + + visit("/latest") + + expect(sidebar).to have_no_tags_section + end + + it "should not display the tags section when site has no top tags and `default_navigation_menu_tags` site setting has not been set" do + Tag.delete_all + + visit("/latest") + + expect(sidebar).to have_no_tags_section + end + + it "should display the site's top tags when `default_navigation_menu_tags` site setting has not been set" do + visit("/latest") + + expect(sidebar).to have_tags_section + expect(sidebar).to have_all_tags_section_link + expect(sidebar).to have_tags_section_links([tag1, tag2, tag3, tag4, tag5]) + end + + it "should display the site's top tags when `default_navigation_menu_tags` site setting has been set but the tags configured are hidden to the user" do + SiteSetting.default_navigation_menu_tags = "#{tag5.name}" + Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: [tag5.name]) + + visit("/latest") + + expect(sidebar).to have_tags_section + expect(sidebar).to have_all_tags_section_link + expect(sidebar).to have_tags_section_links([tag1, tag2, tag3, tag4, tag6]) + end + + it "should display the tags configured in `default_navigation_menu_tags` site setting when it has been set" do + SiteSetting.default_navigation_menu_tags = "#{tag3.name}|#{tag4.name}" + + visit("/latest") + + expect(sidebar).to have_tags_section + expect(sidebar).to have_all_tags_section_link + expect(sidebar).to have_tags_section_links([tag3, tag4]) + end + end +end