FIX: Admin can't see user sidebar preferences of other users (#19570)

app/assets/javascripts/discourse/app/templates/preferences/sidebar.hbs

@@ -40,7 +40,7 @@
 
   <div class="controls controls-dropdown">
     <label>{{i18n "user.experimental_sidebar.list_destination_instruction"}}</label>
-    <ComboBox @valueProperty="value" @content={{this.sidebarListDestinations}} @value={{this.newSidebarListDestination}} @onChange={{action (mut this.newSidebarListDestination)}} />
+    <ComboBox @class="preferences-sidebar-navigation__list-destination-selector" @valueProperty="value" @content={{this.sidebarListDestinations}} @value={{this.newSidebarListDestination}} @onChange={{action (mut this.newSidebarListDestination)}} />
   </div>
 </div>
 

app/serializers/concerns/user_sidebar_mixin.rb

@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+module UserSidebarMixin
+  def sidebar_tags
+    object.visible_sidebar_tags(scope)
+      .pluck(:name, :topic_count, :pm_topic_count)
+      .reduce([]) do |tags, sidebar_tag|
+        tags.push(
+          name: sidebar_tag[0],
+          pm_only: sidebar_tag[1] == 0 && sidebar_tag[2] > 0
+        )
+      end
+  end
+
+  def display_sidebar_tags
+    DiscourseTagging.filter_visible(Tag, scope).exists?
+  end
+
+  def include_display_sidebar_tags?
+    include_sidebar_tags?
+  end
+
+  def include_sidebar_tags?
+    SiteSetting.tagging_enabled && sidebar_navigation_menu?
+  end
+
+  def sidebar_category_ids
+    object.category_sidebar_section_links.pluck(:linkable_id) & scope.allowed_category_ids
+  end
+
+  def include_sidebar_category_ids?
+    sidebar_navigation_menu?
+  end
+
+  def sidebar_list_destination
+    object.user_option.sidebar_list_none_selected? ? SiteSetting.default_sidebar_list_destination : object.user_option.sidebar_list_destination
+  end
+
+  def include_sidebar_list_destination?
+    sidebar_navigation_menu?
+  end
+
+  private
+
+  def sidebar_navigation_menu?
+    !SiteSetting.legacy_navigation_menu?
+  end
+end

app/serializers/concerns/user_sidebar_tags_mixin.rb

@@ -1,31 +0,0 @@
-# frozen_string_literal: true
-
-module UserSidebarTagsMixin
-  def self.included(base)
-    base.attributes :display_sidebar_tags,
-                    :sidebar_tags
-  end
-
-  def sidebar_tags
-    object.visible_sidebar_tags(scope)
-      .pluck(:name, :topic_count, :pm_topic_count)
-      .reduce([]) do |tags, sidebar_tag|
-        tags.push(
-          name: sidebar_tag[0],
-          pm_only: sidebar_tag[1] == 0 && sidebar_tag[2] > 0
-        )
-      end
-  end
-
-  def include_sidebar_tags?
-    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.legacy_navigation_menu?
-  end
-end

app/serializers/current_user_serializer.rb

@@ -2,7 +2,7 @@
 
 class CurrentUserSerializer < BasicUserSerializer
   include UserTagNotificationsMixin
-  include UserSidebarTagsMixin
+  include UserSidebarMixin
 
   attributes :name,
              :unread_notifications,
@@ -62,12 +62,14 @@ class CurrentUserSerializer < BasicUserSerializer
              :draft_count,
              :pending_posts_count,
              :status,
-             :sidebar_category_ids,
              :grouped_unread_notifications,
              :redesigned_user_menu_enabled,
              :redesigned_user_page_nav_enabled,
-             :sidebar_list_destination,
-             :redesigned_topic_timeline_enabled
+             :redesigned_topic_timeline_enabled,
+             :display_sidebar_tags,
+             :sidebar_tags,
+             :sidebar_category_ids,
+             :sidebar_list_destination
 
   delegate :user_stat, to: :object, private: true
   delegate :any_posts, :draft_count, :pending_posts_count, :read_faq?, to: :user_stat
@@ -100,10 +102,6 @@ class CurrentUserSerializer < BasicUserSerializer
     scope.can_create_group?
   end
 
-  def sidebar_list_destination
-    object.user_option.sidebar_list_none_selected? ? SiteSetting.default_sidebar_list_destination : object.user_option.sidebar_list_destination
-  end
-
   def can_send_private_email_messages
     scope.can_send_private_messages_to_email?
   end
@@ -253,14 +251,6 @@ class CurrentUserSerializer < BasicUserSerializer
     Draft.has_topic_draft(object)
   end
 
-  def sidebar_category_ids
-    object.category_sidebar_section_links.pluck(:linkable_id) & scope.allowed_category_ids
-  end
-
-  def include_sidebar_category_ids?
-    !SiteSetting.legacy_navigation_menu?
-  end
-
   def include_status?
     SiteSetting.enable_user_status && object.has_status?
   end

app/serializers/user_serializer.rb

@@ -2,7 +2,7 @@
 
 class UserSerializer < UserCardSerializer
   include UserTagNotificationsMixin
-  include UserSidebarTagsMixin
+  include UserSidebarMixin
 
   attributes :bio_raw,
              :bio_cooked,
@@ -64,7 +64,10 @@ class UserSerializer < UserCardSerializer
                      :user_auth_tokens,
                      :user_notification_schedule,
                      :use_logo_small_as_avatar,
-                     :sidebar_tags
+                     :sidebar_tags,
+                     :sidebar_category_ids,
+                     :sidebar_list_destination,
+                     :display_sidebar_tags
 
   untrusted_attributes :bio_raw,
                        :bio_cooked,

spec/serializers/current_user_serializer_spec.rb

@@ -220,96 +220,6 @@ RSpec.describe CurrentUserSerializer do
     end
   end
 
-  describe '#sidebar_tags' do
-    fab!(:tag) { Fabricate(:tag, name: "foo") }
-    fab!(:pm_tag) { Fabricate(:tag, name: "bar", pm_topic_count: 5, topic_count: 0) }
-    fab!(:hidden_tag) { Fabricate(:tag, name: "secret") }
-    fab!(:staff_tag_group) { Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: ["secret"]) }
-    fab!(:tag_sidebar_section_link) { Fabricate(:tag_sidebar_section_link, user: user, linkable: tag) }
-    fab!(:tag_sidebar_section_link_2) { Fabricate(:tag_sidebar_section_link, user: user, linkable: pm_tag) }
-    fab!(:tag_sidebar_section_link_3) { Fabricate(:tag_sidebar_section_link, user: user, linkable: hidden_tag) }
-
-    it "is not included when navigation menu is legacy" do
-      SiteSetting.navigation_menu = "legacy"
-      SiteSetting.tagging_enabled = true
-
-      json = serializer.as_json
-
-      expect(json[:sidebar_tags]).to eq(nil)
-    end
-
-    it "is not included when tagging has not been enabled" do
-      SiteSetting.navigation_menu = "sidebar"
-      SiteSetting.tagging_enabled = false
-
-      json = serializer.as_json
-
-      expect(json[:sidebar_tags]).to eq(nil)
-    end
-
-    it "serializes only the tags that the user can see when sidebar and tagging has been enabled" do
-      SiteSetting.navigation_menu = "sidebar"
-      SiteSetting.tagging_enabled = true
-
-      json = serializer.as_json
-
-      expect(json[:sidebar_tags]).to contain_exactly(
-        { name: tag.name, pm_only: false },
-        { name: pm_tag.name, pm_only: true }
-      )
-
-      user.update!(admin: true)
-
-      json = serializer.as_json
-
-      expect(json[:sidebar_tags]).to contain_exactly(
-        { name: tag.name, pm_only: false },
-        { name: pm_tag.name, pm_only: true },
-        { name: hidden_tag.name, pm_only: false }
-      )
-    end
-  end
-
-  describe '#sidebar_category_ids' do
-    fab!(:group) { Fabricate(:group) }
-    fab!(:category) { Fabricate(:category) }
-    fab!(:category_2) { Fabricate(:category) }
-    fab!(:private_category) { Fabricate(:private_category, group: group) }
-    fab!(:category_sidebar_section_link) { Fabricate(:category_sidebar_section_link, user: user, linkable: category) }
-    fab!(:category_sidebar_section_link_2) { Fabricate(:category_sidebar_section_link, user: user, linkable: category_2) }
-    fab!(:category_sidebar_section_link_3) { Fabricate(:category_sidebar_section_link, user: user, linkable: private_category) }
-
-    it "is not included when navigation menu is legacy" do
-      category_sidebar_section_link
-      SiteSetting.navigation_menu = "legacy"
-
-      json = serializer.as_json
-
-      expect(json[:sidebar_category_ids]).to eq(nil)
-    end
-
-    it 'serializes only the categories that the user can see when sidebar and tagging has been enabled"' do
-      SiteSetting.navigation_menu = "sidebar"
-
-      json = serializer.as_json
-
-      expect(json[:sidebar_category_ids]).to eq([
-        category.id,
-        category_2.id
-      ])
-
-      group.add(user)
-      serializer = described_class.new(user, scope: Guardian.new(user), root: false)
-      json = serializer.as_json
-
-      expect(json[:sidebar_category_ids]).to eq([
-        category.id,
-        category_2.id,
-        private_category.id
-      ])
-    end
-  end
-
   describe "#likes_notifications_disabled" do
     it "is true if the user disables likes notifications" do
       user.user_option.update!(like_notification_frequency: UserOption.like_notification_frequency_type[:never])
@@ -363,15 +273,6 @@ RSpec.describe CurrentUserSerializer do
     end
   end
 
-  describe "#sidebar_list_destination" do
-    it "returns choosen value or default" do
-      expect(serializer.as_json[:sidebar_list_destination]).to eq(SiteSetting.default_sidebar_list_destination)
-
-      user.user_option.update!(sidebar_list_destination: "unread_new")
-      expect(serializer.as_json[:sidebar_list_destination]).to eq("unread_new")
-    end
-  end
-
   describe "#new_personal_messages_notifications_count" do
     fab!(:notification) { Fabricate(:notification, user: user, read: false, notification_type: Notification.types[:private_message]) }
 
@@ -388,5 +289,5 @@ RSpec.describe CurrentUserSerializer do
     end
   end
 
-  include_examples "#display_sidebar_tags", described_class
+  include_examples "User Sidebar Serializer Attributes", described_class
 end

spec/serializers/user_serializer_spec.rb

@@ -372,53 +372,17 @@ RSpec.describe UserSerializer do
     end
   end
 
-  describe '#sidebar_tags' do
-    fab!(:tag_sidebar_section_link) { Fabricate(:tag_sidebar_section_link, user: user) }
-    fab!(:tag_sidebar_section_link_2) { Fabricate(:tag_sidebar_section_link, user: user) }
+  context 'for user sidebar attributes' do
+    include_examples "User Sidebar Serializer Attributes", described_class
 
-    context 'when viewing self' do
-      subject(:json) { UserSerializer.new(user, scope: Guardian.new(user), root: false).as_json }
+    it "does not include attributes when scoped to user that cannot edit user" do
+      user2 = Fabricate(:user)
+      serializer = described_class.new(user, scope: Guardian.new(user2), root: false)
 
-      it "is not included when navigation menu is set to legacy" do
-        SiteSetting.navigation_menu = "legacy"
-        SiteSetting.tagging_enabled = true
-
-        expect(json[:sidebar_tags]).to eq(nil)
-      end
-
-      it "is not included when SiteSetting.tagging_enabled is false" do
-        SiteSetting.navigation_menu = "sidebar"
-        SiteSetting.tagging_enabled = false
-
-        expect(json[:sidebar_tags]).to eq(nil)
-      end
-
-      it "is present when sidebar and tagging has been enabled" do
-        SiteSetting.navigation_menu = "sidebar"
-        SiteSetting.tagging_enabled = true
-
-        tag_sidebar_section_link_2.linkable.update!(pm_topic_count: 5, topic_count: 0)
-
-        expect(json[:sidebar_tags]).to contain_exactly(
-          { name: tag_sidebar_section_link.linkable.name, pm_only: false },
-          { name: tag_sidebar_section_link_2.linkable.name, pm_only: true }
-        )
-      end
-    end
-
-    context 'when viewing another user' do
-      fab!(:user2) { Fabricate(:user) }
-
-      subject(:json) { UserSerializer.new(user, scope: Guardian.new(user2), root: false).as_json }
-
-      it "is not present even when sidebar and tagging has been enabled" do
-        SiteSetting.navigation_menu = "sidebar"
-        SiteSetting.tagging_enabled = true
-
-        expect(json[:sidebar_tags]).to eq(nil)
+      expect(serializer.as_json[:sidebar_category_ids]).to eq(nil)
+      expect(serializer.as_json[:sidebar_tags]).to eq(nil)
+      expect(serializer.as_json[:sidebar_list_destination]).to eq(nil)
+      expect(serializer.as_json[:display_sidebar_tags]).to eq(nil)
     end
   end
 end
-
-  include_examples "#display_sidebar_tags", UserSerializer
-end

spec/support/user_sidebar_serializer_attributes.rb

@@ -0,0 +1,150 @@
+# frozen_string_literal: true
+
+RSpec.shared_examples "User Sidebar Serializer Attributes" do |serializer_klass|
+  fab!(:user) { Fabricate(:user) }
+
+  let(:serializer) { serializer_klass.new(user, scope: Guardian.new(user), root: false) }
+
+  before do
+    SiteSetting.navigation_menu = "sidebar"
+  end
+
+  describe "#sidebar_list_destination" do
+    it 'is not included when navigation menu is legacy' do
+      SiteSetting.navigation_menu = "legacy"
+
+      expect(serializer.as_json[:sidebar_list_destination]).to eq(nil)
+    end
+
+    it "returns choosen value or default" do
+      expect(serializer.as_json[:sidebar_list_destination]).to eq(SiteSetting.default_sidebar_list_destination)
+
+      user.user_option.update!(sidebar_list_destination: "unread_new")
+
+      expect(serializer.as_json[:sidebar_list_destination]).to eq("unread_new")
+    end
+  end
+
+  describe '#sidebar_category_ids' do
+    fab!(:group) { Fabricate(:group) }
+    fab!(:category) { Fabricate(:category) }
+    fab!(:category_2) { Fabricate(:category) }
+    fab!(:private_category) { Fabricate(:private_category, group: group) }
+    fab!(:category_sidebar_section_link) { Fabricate(:category_sidebar_section_link, user: user, linkable: category) }
+    fab!(:category_sidebar_section_link_2) { Fabricate(:category_sidebar_section_link, user: user, linkable: category_2) }
+    fab!(:category_sidebar_section_link_3) { Fabricate(:category_sidebar_section_link, user: user, linkable: private_category) }
+
+    it "is not included when navigation menu is legacy" do
+      SiteSetting.navigation_menu = "legacy"
+
+      json = serializer.as_json
+
+      expect(json[:sidebar_category_ids]).to eq(nil)
+    end
+
+    it 'serializes only the categories that the user can see when sidebar has been enabled"' do
+      SiteSetting.navigation_menu = "sidebar"
+
+      json = serializer.as_json
+
+      expect(json[:sidebar_category_ids]).to eq([
+        category.id,
+        category_2.id
+      ])
+
+      group.add(user)
+      serializer = serializer_klass.new(user, scope: Guardian.new(user), root: false)
+      json = serializer.as_json
+
+      expect(json[:sidebar_category_ids]).to eq([
+        category.id,
+        category_2.id,
+        private_category.id
+      ])
+    end
+  end
+
+  describe '#sidebar_tags' do
+    fab!(:tag) { Fabricate(:tag, name: "foo") }
+    fab!(:pm_tag) { Fabricate(:tag, name: "bar", pm_topic_count: 5, topic_count: 0) }
+    fab!(:hidden_tag) { Fabricate(:tag, name: "secret") }
+    fab!(:staff_tag_group) { Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: ["secret"]) }
+    fab!(:tag_sidebar_section_link) { Fabricate(:tag_sidebar_section_link, user: user, linkable: tag) }
+    fab!(:tag_sidebar_section_link_2) { Fabricate(:tag_sidebar_section_link, user: user, linkable: pm_tag) }
+    fab!(:tag_sidebar_section_link_3) { Fabricate(:tag_sidebar_section_link, user: user, linkable: hidden_tag) }
+
+    it "is not included when navigation menu is legacy" do
+      SiteSetting.navigation_menu = "legacy"
+      SiteSetting.tagging_enabled = true
+
+      json = serializer.as_json
+
+      expect(json[:sidebar_tags]).to eq(nil)
+    end
+
+    it "is not included when tagging has not been enabled" do
+      SiteSetting.navigation_menu = "sidebar"
+      SiteSetting.tagging_enabled = false
+
+      json = serializer.as_json
+
+      expect(json[:sidebar_tags]).to eq(nil)
+    end
+
+    it "serializes only the tags that the user can see when sidebar and tagging has been enabled" do
+      SiteSetting.navigation_menu = "sidebar"
+      SiteSetting.tagging_enabled = true
+
+      json = serializer.as_json
+
+      expect(json[:sidebar_tags]).to contain_exactly(
+        { name: tag.name, pm_only: false },
+        { name: pm_tag.name, pm_only: true }
+      )
+
+      user.update!(admin: true)
+
+      json = serializer.as_json
+
+      expect(json[:sidebar_tags]).to contain_exactly(
+        { name: tag.name, pm_only: false },
+        { name: pm_tag.name, pm_only: true },
+        { name: hidden_tag.name, pm_only: false }
+      )
+    end
+  end
+
+  describe "#display_sidebar_tags" do
+    fab!(:tag) { Fabricate(:tag) }
+
+    it 'should not be included in serialised object when navigation menu is legacy' do
+      SiteSetting.tagging_enabled = true
+      SiteSetting.navigation_menu = "legacy"
+
+      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
+end

spec/support/user_sidebar_tags_mixin.rb

@@ -1,41 +0,0 @@
-# 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.navigation_menu = "sidebar"
-  end
-
-  it 'should not be included in serialised object when navigation menu is legacy' do
-    SiteSetting.tagging_enabled = true
-    SiteSetting.navigation_menu = "legacy"
-
-    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

spec/system/page_objects/pages/user_preferences_sidebar.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+module PageObjects
+  module Pages
+    class UserPreferencesSidebar < PageObjects::Pages::Base
+      def visit(user)
+        page.visit("/u/#{user.username}/preferences/sidebar")
+        self
+      end
+
+      def has_sidebar_categories_preference?(*categories)
+        category_selector_header = page.find(".category-selector .select-kit-header-wrapper")
+        category_selector_header.has_content?(categories.map(&:name).join(", "))
+      end
+
+      def has_sidebar_tags_preference?(*tags)
+        tag_selector_header = page.find(".tag-chooser .select-kit-header-wrapper")
+        tag_selector_header.has_content?(tags.map(&:name).join(", "))
+      end
+
+      def has_sidebar_list_destination_preference?(type)
+        list_selector_header = page.find(".preferences-sidebar-navigation__list-destination-selector .select-kit-header-wrapper")
+        list_selector_header.has_content?(I18n.t("js.user.experimental_sidebar.list_destination_#{type}"))
+      end
+    end
+  end
+end

spec/system/viewing_sidebar_preferences_spec.rb

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+describe 'Viewing sidebar preferences', type: :system, js: true do
+  let(:user_preferences_sidebar_page) { PageObjects::Pages::UserPreferencesSidebar.new }
+
+  before do
+    SiteSetting.navigation_menu = "sidebar"
+  end
+
+  context 'as an admin' do
+    fab!(:admin) { Fabricate(:admin) }
+    fab!(:user) { Fabricate(:user) }
+    fab!(:category) { Fabricate(:category) }
+    fab!(:category2) { Fabricate(:category) }
+    fab!(:category_sidebar_section_link) { Fabricate(:category_sidebar_section_link, user: user, linkable: category) }
+    fab!(:category2_sidebar_section_link) { Fabricate(:category_sidebar_section_link, user: user, linkable: category2) }
+    fab!(:tag) { Fabricate(:tag) }
+    fab!(:tag2) { Fabricate(:tag) }
+    fab!(:tag_sidebar_section_link) { Fabricate(:tag_sidebar_section_link, user: user, linkable: tag) }
+    fab!(:tag2_sidebar_section_link) { Fabricate(:tag_sidebar_section_link, user: user, linkable: tag2) }
+
+    before do
+      sign_in(admin)
+    end
+
+    it 'should be able to view sidebar preferences of another user' do
+      user.user_option.update!(sidebar_list_destination: "unread_new")
+
+      user_preferences_sidebar_page.visit(user)
+
+      expect(user_preferences_sidebar_page).to have_sidebar_categories_preference(category, category2)
+      expect(user_preferences_sidebar_page).to have_sidebar_tags_preference(tag, tag2)
+      expect(user_preferences_sidebar_page).to have_sidebar_list_destination_preference("unread_new")
+    end
+  end
+end