From deb0656b63512387cea498ed15c9aaee9adae2f9 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Thu, 13 Oct 2022 08:37:28 +0800 Subject: [PATCH] UX: Hide tags section in sidebar when user has no visible tags (#18539) Also hides the tags configuration for sidebar under user preferences Internal ref: /t/73500 --- .../app/components/sidebar/user/sections.hbs | 2 +- .../app/templates/preferences/sidebar.hbs | 2 +- .../sidebar-user-tags-section-test.js | 12 ++++++ .../tests/acceptance/sidebar-user-test.js | 8 +++- .../user-preferences-sidebar-test.js | 5 ++- .../concerns/user_sidebar_tags_mixin.rb | 12 +++++- .../current_user_serializer_spec.rb | 2 + spec/serializers/user_serializer_spec.rb | 2 + spec/support/user_sidebar_tags_mixin.rb | 41 +++++++++++++++++++ 9 files changed, 79 insertions(+), 7 deletions(-) create mode 100644 spec/support/user_sidebar_tags_mixin.rb diff --git a/app/assets/javascripts/discourse/app/components/sidebar/user/sections.hbs b/app/assets/javascripts/discourse/app/components/sidebar/user/sections.hbs index 0cb880c614d..f3a01651bd6 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/user/sections.hbs +++ b/app/assets/javascripts/discourse/app/components/sidebar/user/sections.hbs @@ -2,7 +2,7 @@ - {{#if this.siteSettings.tagging_enabled}} + {{#if this.currentUser.display_sidebar_tags}} {{/if}} diff --git a/app/assets/javascripts/discourse/app/templates/preferences/sidebar.hbs b/app/assets/javascripts/discourse/app/templates/preferences/sidebar.hbs index dd55ee6942c..61a6286787a 100644 --- a/app/assets/javascripts/discourse/app/templates/preferences/sidebar.hbs +++ b/app/assets/javascripts/discourse/app/templates/preferences/sidebar.hbs @@ -15,7 +15,7 @@
{{i18n "user.experimental_sidebar.categories_section_instruction"}}
-{{#if this.siteSettings.tagging_enabled}} +{{#if this.model.display_sidebar_tags}}
{{i18n "user.experimental_sidebar.tags_section"}} diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-tags-section-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-tags-section-test.js index 29f082fdc71..cdd7e790d9e 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-tags-section-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-tags-section-test.js @@ -58,6 +58,7 @@ acceptance("Sidebar - Logged on user - Tags section", function (needs) { pm_only: false, }, ], + display_sidebar_tags: true, }); needs.pretender((server, helper) => { @@ -90,6 +91,17 @@ acceptance("Sidebar - Logged on user - Tags section", function (needs) { }); }); + test("section is not displayed when display_sidebar_tags property is false", async function (assert) { + updateCurrentUser({ display_sidebar_tags: false }); + + await visit("/"); + + assert.notOk( + exists(".sidebar-section-tags"), + "tags section is not displayed" + ); + }); + test("clicking on section header button", async function (assert) { await visit("/"); await click(".sidebar-section-tags .sidebar-section-header-button"); diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-test.js index 10ed04ae0f1..24e7473b181 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-test.js @@ -1,7 +1,11 @@ import I18n from "I18n"; import { test } from "qunit"; import { click, visit } from "@ember/test-helpers"; -import { acceptance, exists } from "discourse/tests/helpers/qunit-helpers"; +import { + acceptance, + exists, + updateCurrentUser, +} from "discourse/tests/helpers/qunit-helpers"; acceptance( "Sidebar - Logged on user - Experimental sidebar and hamburger setting disabled", @@ -171,7 +175,7 @@ acceptance( }); test("clean up topic tracking state state changed callbacks when sidebar is destroyed", async function (assert) { - this.siteSettings.tagging_enabled = true; + updateCurrentUser({ display_sidebar_tags: true }); await visit("/"); diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-sidebar-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-sidebar-test.js index 5e48f669bdc..4637783240d 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-sidebar-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-sidebar-test.js @@ -13,6 +13,7 @@ acceptance("User Preferences - Sidebar", function (needs) { needs.user({ sidebar_category_ids: [], sidebar_tags: [], + display_sidebar_tags: true, }); needs.settings({ @@ -51,8 +52,8 @@ acceptance("User Preferences - Sidebar", function (needs) { }); }); - test("user should not see tag chooser when tagging is disabled", async function (assert) { - this.siteSettings.tagging_enabled = false; + test("user should not see tag chooser when display_sidebar_tags property is false", async function (assert) { + updateCurrentUser({ display_sidebar_tags: false }); await visit("/u/eviltrout/preferences/sidebar"); diff --git a/app/serializers/concerns/user_sidebar_tags_mixin.rb b/app/serializers/concerns/user_sidebar_tags_mixin.rb index f9ec79fbeea..e387f01c310 100644 --- a/app/serializers/concerns/user_sidebar_tags_mixin.rb +++ b/app/serializers/concerns/user_sidebar_tags_mixin.rb @@ -2,10 +2,20 @@ module UserSidebarTagsMixin def self.included(base) + base.attributes :display_sidebar_tags + base.has_many :sidebar_tags, serializer: Sidebar::TagSerializer, embed: :objects end def include_sidebar_tags? - SiteSetting.enable_experimental_sidebar_hamburger && SiteSetting.tagging_enabled + include_display_sidebar_tags? + end + + def display_sidebar_tags + DiscourseTagging.filter_visible(Tag, scope).exists? + end + + def include_display_sidebar_tags? + SiteSetting.tagging_enabled && SiteSetting.enable_experimental_sidebar_hamburger end end diff --git a/spec/serializers/current_user_serializer_spec.rb b/spec/serializers/current_user_serializer_spec.rb index db40891fa51..267eedac163 100644 --- a/spec/serializers/current_user_serializer_spec.rb +++ b/spec/serializers/current_user_serializer_spec.rb @@ -387,4 +387,6 @@ RSpec.describe CurrentUserSerializer do expect(serializer.as_json[:associated_account_ids]).to eq({ "twitter" => "1" }) end end + + include_examples "#display_sidebar_tags", described_class end diff --git a/spec/serializers/user_serializer_spec.rb b/spec/serializers/user_serializer_spec.rb index 966d5f225fb..b5abe360d24 100644 --- a/spec/serializers/user_serializer_spec.rb +++ b/spec/serializers/user_serializer_spec.rb @@ -424,4 +424,6 @@ RSpec.describe UserSerializer do end end end + + include_examples "#display_sidebar_tags", UserSerializer end diff --git a/spec/support/user_sidebar_tags_mixin.rb b/spec/support/user_sidebar_tags_mixin.rb new file mode 100644 index 00000000000..0b874c59e6d --- /dev/null +++ b/spec/support/user_sidebar_tags_mixin.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +RSpec.shared_examples "#display_sidebar_tags" do |serializer_klass| + fab!(:tag) { Fabricate(:tag) } + fab!(:user) { Fabricate(:user) } + let(:serializer) { serializer_klass.new(user, scope: Guardian.new(user), root: false) } + + before do + SiteSetting.enable_experimental_sidebar_hamburger = true + end + + it 'should not be included in serialised object when experimental hamburger and sidebar has been disabled' do + SiteSetting.tagging_enabled = true + SiteSetting.enable_experimental_sidebar_hamburger = false + + expect(serializer.as_json[:display_sidebar_tags]).to eq(nil) + end + + it 'should not be included in serialised object when tagging has been disabled' do + SiteSetting.tagging_enabled = false + + expect(serializer.as_json[:display_sidebar_tags]).to eq(nil) + end + + it 'should be true when user has visible tags' do + SiteSetting.tagging_enabled = true + + Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: [tag.name]) + user.update!(admin: true) + + expect(serializer.as_json[:display_sidebar_tags]).to eq(true) + end + + it 'should be false when user has no visible tags' do + SiteSetting.tagging_enabled = true + + Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: [tag.name]) + + expect(serializer.as_json[:display_sidebar_tags]).to eq(false) + end +end