diff --git a/app/models/user.rb b/app/models/user.rb index 79a6fc22d62..766ef7bf8d0 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -85,7 +85,7 @@ class User < ActiveRecord::Base has_many :topics_allowed, through: :topic_allowed_users, source: :topic has_many :groups, through: :group_users - has_many :secure_categories, through: :groups, source: :categories + has_many :secure_categories, -> { distinct }, through: :groups, source: :categories has_many :associated_groups, through: :user_associated_groups, dependent: :destroy # deleted in user_second_factors relationship @@ -135,10 +135,14 @@ class User < ActiveRecord::Base after_create :ensure_in_trust_level_group after_create :set_default_categories_preferences after_create :set_default_tags_preferences - after_create :set_default_sidebar_section_links + after_create :add_default_sidebar_section_links - after_update :set_default_sidebar_section_links, if: Proc.new { - self.saved_change_to_staged? || self.saved_change_to_admin? + after_update :update_default_sidebar_section_links, if: Proc.new { + self.saved_change_to_admin? + } + + after_update :add_default_sidebar_section_links, if: Proc.new { + self.saved_change_to_staged? } after_update :trigger_user_updated_event, if: Proc.new { @@ -1946,25 +1950,54 @@ class User < ActiveRecord::Base private - def set_default_sidebar_section_links + def set_default_sidebar_section_links(update: false) return if !SiteSetting.enable_experimental_sidebar_hamburger return if staged? || bot? if SiteSetting.default_sidebar_categories.present? + categories_to_update = SiteSetting.default_sidebar_categories.split("|") + + if update + filtered_default_category_ids = Category.secured(self.guardian).where(id: categories_to_update).pluck(:id) + existing_category_ids = SidebarSectionLink.where(user: self, linkable_type: 'Category').pluck(:linkable_id) + + categories_to_update = existing_category_ids + (filtered_default_category_ids & self.secure_category_ids) + end + SidebarSectionLinksUpdater.update_category_section_links( self, - category_ids: SiteSetting.default_sidebar_categories.split("|") + category_ids: categories_to_update ) end if SiteSetting.tagging_enabled && SiteSetting.default_sidebar_tags.present? + tags_to_update = SiteSetting.default_sidebar_tags.split("|") + + if update + default_tag_ids = Tag.where(name: tags_to_update).pluck(:id) + filtered_default_tags = DiscourseTagging.filter_visible(Tag, self.guardian).where(id: default_tag_ids).pluck(:name) + + existing_tag_ids = SidebarSectionLink.where(user: self, linkable_type: 'Tag').pluck(:linkable_id) + existing_tags = DiscourseTagging.filter_visible(Tag, self.guardian).where(id: existing_tag_ids).pluck(:name) + + tags_to_update = existing_tags + (filtered_default_tags & DiscourseTagging.hidden_tag_names) + end + SidebarSectionLinksUpdater.update_tag_section_links( self, - tag_names: SiteSetting.default_sidebar_tags.split("|") + tag_names: tags_to_update ) end end + def add_default_sidebar_section_links + set_default_sidebar_section_links + end + + def update_default_sidebar_section_links + set_default_sidebar_section_links(update: true) + end + def stat user_stat || create_user_stat end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 38540d40173..b7276d5caa0 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -64,19 +64,47 @@ RSpec.describe User do tag.id, hidden_tag.id ) + end - # A user promoted to admin should get secured sidebar records + it 'should create and remove the right sidebar section link records when user is promoted/demoted as an admin' do + user = Fabricate(:user) + another_category = Fabricate(:category) + another_tag = Fabricate(:tag) + + # User has customized their sidebar categories and tags + SidebarSectionLink.where(user: user).delete_all + SidebarSectionLinksUpdater.update_category_section_links(user, category_ids: [another_category.id]) + SidebarSectionLinksUpdater.update_tag_section_links(user, tag_names: [another_tag.name]) + + # A user promoted to admin now has any default categories/tags they didn't previously have access to user.update(admin: true) - expect(SidebarSectionLink.where(linkable_type: 'Category', user_id: user.id).pluck(:linkable_id)).to contain_exactly( - category.id, + another_category.id, secured_category.id ) - expect(SidebarSectionLink.where(linkable_type: 'Tag', user_id: user.id).pluck(:linkable_id)).to contain_exactly( - tag.id, + another_tag.id, hidden_tag.id ) + + # User still has their customized sidebar categories and tags after demotion + user.update(admin: false) + expect(SidebarSectionLink.where(linkable_type: 'Category', user_id: user.id).pluck(:linkable_id)).to contain_exactly( + another_category.id + ) + expect(SidebarSectionLink.where(linkable_type: 'Tag', user_id: user.id).pluck(:linkable_id)).to contain_exactly( + another_tag.id + ) + end + + it 'should not receive any new categories w/ suppress secured categories from admin enabled' do + SiteSetting.suppress_secured_categories_from_admin = true + user = Fabricate(:user) + SidebarSectionLink.where(user: user).delete_all # User has customized their sidebar categories + user.update(admin: true) + expect(SidebarSectionLink.where(linkable_type: 'Category', user_id: user.id).pluck(:linkable_id)).to be_empty + user.update(admin: false) + expect(SidebarSectionLink.where(linkable_type: 'Category', user_id: user.id).pluck(:linkable_id)).to be_empty end it 'should not create any sidebar section link records when experimental sidebar is disabled' do