DEV: Sidebar default tags and categories are determined at user creation (#18620)

The previous sidebar default tags and categories implementation did not
allow for a user to configure their sidebar to have no categories or
tags. This commit changes how the defaults are applied. When a user is being created,
we create the SidebarSectionLink records based on the `default_sidebar_categories` and
`default_sidebar_tags` site settings. SidebarSectionLink records are
only created for categories and tags which the user has visibility on at
the point of user creation.

With this change, we're also adding the ability for admins to apply
changes to the `default_sidebar_categories` and `default_sidebar_tags`
site settings historically when changing their site setting. When a new
category/tag has been added to the default, the new category/tag will be
added to the sidebar for all users if the admin elects to apply the changes historically.
Like wise when a tag/category is removed, the tag/category will be
removed from the sidebar for all users if the admin elects to apply the
changes historically.

Internal Ref: /t/73500

app/assets/javascripts/admin/addon/mixins/setting-component.js

@@ -157,7 +157,10 @@ export default Mixin.create({
       "default_tags_watching_first_post",
       "default_text_size",
       "default_title_count_mode",
+      "default_sidebar_categories",
+      "default_sidebar_tags",
     ];
+
     const key = this.buffered.get("setting");
 
     if (defaultUserPreferences.includes(key)) {

app/controllers/admin/site_settings_controller.rb

@@ -23,9 +23,11 @@ class Admin::SiteSettingsController < Admin::AdminController
         if !override
           raise Discourse::InvalidParameters, "You cannot change this site setting because it is deprecated, use #{new_name} instead."
         end
+
         break new_name
       end
     end
+
     id = new_setting_name if new_setting_name
 
     raise_access_hidden_setting(id)
@@ -101,6 +103,8 @@ class Admin::SiteSettingsController < Admin::AdminController
             TagUser.insert_all!(tag_users)
           end
         end
+      elsif is_sidebar_default_setting?(id)
+        Jobs.enqueue(:backfill_sidebar_site_settings, setting_name: id, previous_value: previous_value, new_value: new_value)
       end
     end
 
@@ -159,6 +163,8 @@ class Admin::SiteSettingsController < Admin::AdminController
         .pluck("users.id")
 
       json[:user_count] = user_ids.uniq.count
+    elsif is_sidebar_default_setting?(id)
+      json[:user_count] = SidebarSiteSettingsBackfiller.new(id, previous_value: previous_value, new_value: new_value).number_of_users_to_backfill
     end
 
     render json: json
@@ -166,6 +172,10 @@ class Admin::SiteSettingsController < Admin::AdminController
 
   private
 
+  def is_sidebar_default_setting?(setting_name)
+    %w{default_sidebar_categories default_sidebar_tags}.include?(setting_name.to_s)
+  end
+
   def user_options
     {
       default_email_mailing_list_mode: "mailing_list_mode",

app/jobs/regular/backfill_sidebar_site_settings.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class Jobs::BackfillSidebarSiteSettings < Jobs::Base
+  def execute(args)
+    SidebarSiteSettingsBackfiller
+      .new(args[:setting_name], previous_value: args[:previous_value], new_value: args[:new_value])
+      .backfill!
+  end
+end

app/models/user.rb

@@ -135,6 +135,11 @@ 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_update :set_default_sidebar_section_links, if: Proc.new  {
+    self.saved_change_to_staged?
+  }
 
   after_update :trigger_user_updated_event, if: Proc.new {
     self.human? && self.saved_change_to_uploaded_avatar_id?
@@ -279,6 +284,11 @@ class User < ActiveRecord::Base
 
   MAX_STAFF_DELETE_POST_COUNT ||= 5
 
+  def visible_sidebar_tags(user_guardian = nil)
+    user_guardian ||= guardian
+    DiscourseTagging.filter_visible(custom_sidebar_tags, user_guardian)
+  end
+
   def self.max_password_length
     200
   end
@@ -1670,27 +1680,6 @@ class User < ActiveRecord::Base
     SiteSetting.enable_experimental_sidebar_hamburger
   end
 
-  def sidebar_categories_ids
-    categories_ids = category_sidebar_section_links.pluck(:linkable_id)
-
-    if categories_ids.blank? && SiteSetting.default_sidebar_categories.present?
-      return SiteSetting.default_sidebar_categories.split("|").map(&:to_i) & guardian.allowed_category_ids
-    end
-
-    categories_ids
-  end
-
-  def sidebar_tags
-    return custom_sidebar_tags if custom_sidebar_tags.present?
-
-    if SiteSetting.default_sidebar_tags.present?
-      tag_names = SiteSetting.default_sidebar_tags.split("|") - DiscourseTagging.hidden_tag_names(guardian)
-      return Tag.where(name: tag_names)
-    end
-
-    Tag.none
-  end
-
   protected
 
   def badge_grant
@@ -1919,6 +1908,45 @@ class User < ActiveRecord::Base
 
   private
 
+  def set_default_sidebar_section_links
+    return if !SiteSetting.enable_experimental_sidebar_hamburger
+    return if staged? || bot?
+
+    records = []
+
+    if SiteSetting.default_sidebar_categories.present?
+      category_ids = SiteSetting.default_sidebar_categories.split("|")
+
+      # Filters out categories that user does not have access to or do not exist anymore
+      category_ids = Category.secured(self.guardian).where(id: category_ids).pluck(:id)
+
+      category_ids.each do |category_id|
+        records.push(
+          linkable_type: 'Category',
+          linkable_id: category_id,
+          user_id: self.id
+        )
+      end
+    end
+
+    if SiteSetting.tagging_enabled && SiteSetting.default_sidebar_tags.present?
+      tag_names = SiteSetting.default_sidebar_tags.split("|")
+
+      # Filters out tags that user cannot see or do not exist anymore
+      tag_ids = DiscourseTagging.filter_visible(Tag, self.guardian).where(name: tag_names).pluck(:id)
+
+      tag_ids.each do |tag_id|
+        records.push(
+          linkable_type: 'Tag',
+          linkable_id: tag_id,
+          user_id: self.id
+        )
+      end
+    end
+
+    SidebarSectionLink.insert_all!(records) if records.present?
+  end
+
   def stat
     user_stat || create_user_stat
   end

app/serializers/concerns/user_sidebar_tags_mixin.rb

@@ -2,9 +2,19 @@
 
 module UserSidebarTagsMixin
   def self.included(base)
-    base.attributes :display_sidebar_tags
+    base.attributes :display_sidebar_tags,
+                    :sidebar_tags
+  end
 
-    base.has_many :sidebar_tags, serializer: Sidebar::TagSerializer, embed: :objects
+  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?

app/serializers/current_user_serializer.rb

@@ -339,7 +339,7 @@ class CurrentUserSerializer < BasicUserSerializer
   end
 
   def sidebar_category_ids
-    object.sidebar_categories_ids
+    object.category_sidebar_section_links.pluck(:linkable_id) & scope.allowed_category_ids
   end
 
   def include_sidebar_category_ids?

app/serializers/sidebar/tag_serializer.rb

@@ -1,11 +0,0 @@
-# frozen_string_literal: true
-
-module Sidebar
-  class TagSerializer < ::ApplicationSerializer
-    attributes :name, :pm_only
-
-    def pm_only
-      object.topic_count == 0 && object.pm_topic_count > 0
-    end
-  end
-end

app/serializers/site_serializer.rb

@@ -229,11 +229,11 @@ class SiteSerializer < ApplicationSerializer
   end
 
   def anonymous_default_sidebar_tags
-    User.new.sidebar_tags.pluck(:name)
+    SiteSetting.default_sidebar_tags.split("|") - DiscourseTagging.hidden_tag_names(scope)
   end
 
   def include_anonymous_default_sidebar_tags?
-    scope.anonymous? && SiteSetting.tagging_enabled && SiteSetting.default_sidebar_tags.present?
+    scope.anonymous? && SiteSetting.enable_experimental_sidebar_hamburger && SiteSetting.tagging_enabled && SiteSetting.default_sidebar_tags.present?
   end
 
   private

app/services/sidebar_site_settings_backfiller.rb

@@ -0,0 +1,93 @@
+# frozen_string_literal: true
+
+# A service class that backfills the changes to the default sidebar categories and tags site settings.
+#
+# When a category/tag is removed from the site settings, the `SidebarSectionLink` records associated with the category/tag
+# are deleted.
+#
+# When a category/tag is added to the site settings, a `SidebarSectionLink` record for the associated category/tag are
+# created for all users that do not already have a `SidebarSectionLink` record for the category/tag.
+class SidebarSiteSettingsBackfiller
+  def initialize(setting_name, previous_value:, new_value:)
+    @setting_name = setting_name
+
+    @linkable_klass, previous_ids, new_ids =
+      case setting_name
+      when "default_sidebar_categories"
+        [
+          Category,
+          previous_value.split("|"),
+          new_value.split("|")
+        ]
+      when "default_sidebar_tags"
+        klass = Tag
+
+        [
+          klass,
+          klass.where(name: previous_value.split("|")).pluck(:id),
+          klass.where(name: new_value.split("|")).pluck(:id)
+        ]
+      else
+        raise 'Invalid setting_name'
+      end
+
+    @added_ids = new_ids - previous_ids
+    @removed_ids = previous_ids - new_ids
+  end
+
+  def backfill!
+    DistributedMutex.synchronize("backfill_sidebar_site_settings_#{@setting_name}") do
+      SidebarSectionLink.where(linkable_type: @linkable_klass.to_s, linkable_id: @removed_ids).delete_all
+
+      User.real.where(staged: false).select(:id).find_in_batches do |users|
+        rows = []
+
+        users.each do |user|
+          @added_ids.each do |linkable_id|
+            rows << { user_id: user[:id], linkable_type: @linkable_klass.to_s, linkable_id: linkable_id }
+          end
+        end
+
+        SidebarSectionLink.insert_all!(rows) if rows.present?
+      end
+    end
+  end
+
+  def number_of_users_to_backfill
+    select_statements = []
+
+    if @removed_ids.present?
+      select_statements.push(<<~SQL)
+      SELECT
+        sidebar_section_links.user_id
+      FROM sidebar_section_links
+      WHERE sidebar_section_links.linkable_type = '#{@linkable_klass.to_s}'
+      AND sidebar_section_links.linkable_id IN (#{@removed_ids.join(",")})
+      SQL
+    end
+
+    if @added_ids.present?
+      # Returns the ids of users that will receive the new additions by excluding the users that already have the additions
+      # Note that we want to avoid doing a left outer join against the "sidebar_section_links" table as PG will end up having
+      # to do a full table join for both tables first which is less efficient and can be slow on large sites.
+      select_statements.push(<<~SQL)
+      SELECT
+        users.id
+      FROM users
+      WHERE users.id NOT IN (
+        SELECT
+          sidebar_section_links.user_id
+        FROM sidebar_section_links
+        WHERE sidebar_section_links.linkable_type = '#{@linkable_klass.to_s}'
+        AND sidebar_section_links.linkable_id IN (#{@added_ids.join(",")})
+      ) AND users.id > 0 AND NOT users.staged
+      SQL
+    end
+
+    DB.query_single(<<~SQL)[0]
+    SELECT
+      COUNT(*)
+    FROM (#{select_statements.join("\nUNION DISTINCT\n")}) AS user_ids
+    SQL
+  end
+end

spec/models/user_spec.rb

@@ -20,6 +20,90 @@ RSpec.describe User do
     end
   end
 
+  describe 'Callbacks' do
+    describe 'default sidebar section links' do
+      fab!(:category) { Fabricate(:category) }
+
+      fab!(:secured_category) do |category|
+        category = Fabricate(:category)
+        category.permissions = { "staff" => :full }
+        category.save!
+        category
+      end
+
+      fab!(:tag) { Fabricate(:tag) }
+      fab!(:hidden_tag) { Fabricate(:tag) }
+      fab!(:staff_tag_group) { Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: [hidden_tag.name]) }
+
+      before do
+        SiteSetting.enable_experimental_sidebar_hamburger = true
+        SiteSetting.tagging_enabled = true
+        SiteSetting.default_sidebar_categories = "#{category.id}|#{secured_category.id}"
+        SiteSetting.default_sidebar_tags = "#{tag.name}|#{hidden_tag.name}"
+      end
+
+      it 'creates the right sidebar section link records for categories and tags that a user can see' do
+        user = Fabricate(:user)
+
+        expect(SidebarSectionLink.where(linkable_type: 'Category', user_id: user.id).pluck(:linkable_id)).to contain_exactly(
+          category.id
+        )
+
+        expect(SidebarSectionLink.where(linkable_type: 'Tag', user_id: user.id).pluck(:linkable_id)).to contain_exactly(
+          tag.id
+        )
+
+        user = Fabricate(:admin)
+
+        expect(SidebarSectionLink.where(linkable_type: 'Category', user_id: user.id).pluck(:linkable_id)).to contain_exactly(
+          category.id,
+          secured_category.id
+        )
+
+        expect(SidebarSectionLink.where(linkable_type: 'Tag', user_id: user.id).pluck(:linkable_id)).to contain_exactly(
+          tag.id,
+          hidden_tag.id
+        )
+      end
+
+      it 'should not create any sidebar section link records when experimental sidebar is disabled' do
+        SiteSetting.enable_experimental_sidebar_hamburger = false
+
+        user = Fabricate(:user)
+
+        expect(SidebarSectionLink.exists?(user_id: user.id)).to eq(false)
+      end
+
+      it 'should not create any sidebar section link records for staged users' do
+        user = Fabricate(:user, staged: true)
+
+        expect(SidebarSectionLink.exists?).to eq(false)
+      end
+
+      it 'should create sidebar section link records when user has been unstaged' do
+        user = Fabricate(:user, staged: true)
+        user.unstage!
+
+        expect(SidebarSectionLink.exists?(user: user)).to eq(true)
+      end
+
+      it 'should not create any sidebar section link records for non human users' do
+        user = Fabricate(:user, id: -Time.now.to_i)
+
+        expect(SidebarSectionLink.exists?).to eq(false)
+      end
+
+      it 'should not create any tag sidebar section link records when tagging is disabled' do
+        SiteSetting.tagging_enabled = false
+
+        user = Fabricate(:user)
+
+        expect(SidebarSectionLink.exists?(linkable_type: 'Category', user_id: user.id)).to eq(true)
+        expect(SidebarSectionLink.exists?(linkable_type: 'Tag', user_id: user.id)).to eq(false)
+      end
+    end
+  end
+
   describe 'Validations' do
     describe '#username' do
       it { is_expected.to validate_presence_of :username }
@@ -2984,4 +3068,21 @@ RSpec.describe User do
       expect(user.reload.seen_notification_id).to eq(last_notification.id)
     end
   end
+
+  describe '#visible_sidebar_tags' do
+    fab!(:user) { Fabricate(:user) }
+    fab!(:tag) { Fabricate(:tag) }
+    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: hidden_tag) }
+
+    it 'should only return tag sidebar section link records of tags that the user is allowed to see' do
+      expect(user.visible_sidebar_tags).to contain_exactly(tag)
+
+      user.update!(admin: true)
+
+      expect(user.visible_sidebar_tags).to contain_exactly(tag, hidden_tag)
+    end
+  end
 end

spec/requests/admin/site_settings_controller_spec.rb

@@ -116,6 +116,44 @@ RSpec.describe Admin::SiteSettingsController do
         end
       end
 
+      context 'when updating default sidebar categories and tags' do
+        it 'does not enqueue the backfilling job if update_existing_user param is not present' do
+          expect_not_enqueued_with(job: :backfill_sidebar_site_settings) do
+            put "/admin/site_settings/default_sidebar_categories.json", params: {
+              default_sidebar_categories: "1|2",
+            }
+
+            expect(response.status).to eq(200)
+          end
+        end
+
+        it 'enqueus the backfilling job if update_existing_user param is present when updating default sidebar tags' do
+          SiteSetting.default_sidebar_tags = "tag3"
+
+          expect_enqueued_with(job: :backfill_sidebar_site_settings, args: { setting_name: 'default_sidebar_tags', new_value: 'tag1|tag2', previous_value: 'tag3' }) do
+            put "/admin/site_settings/default_sidebar_tags.json", params: {
+              default_sidebar_tags: "tag1|tag2",
+              update_existing_user: true
+            }
+
+            expect(response.status).to eq(200)
+          end
+        end
+
+        it 'enqueus the backfilling job if update_existing_user param is present when updating default sidebar categories' do
+          SiteSetting.default_sidebar_categories = "3|4"
+
+          expect_enqueued_with(job: :backfill_sidebar_site_settings, args: { setting_name: 'default_sidebar_categories', new_value: '1|2', previous_value: '3|4' }) do
+            put "/admin/site_settings/default_sidebar_categories.json", params: {
+              default_sidebar_categories: "1|2",
+              update_existing_user: true
+            }
+
+            expect(response.status).to eq(200)
+          end
+        end
+      end
+
       describe 'default categories' do
         fab!(:user1) { Fabricate(:user) }
         fab!(:user2) { Fabricate(:user) }
@@ -265,6 +303,30 @@ RSpec.describe Admin::SiteSettingsController do
           SiteSetting.setting(:default_tags_watching, "")
         end
 
+        context "for sidebar defaults" do
+          it 'returns the right count for the default_sidebar_categories site setting' do
+            category = Fabricate(:category)
+
+            put "/admin/site_settings/default_sidebar_categories/user_count.json", params: {
+              default_sidebar_categories: "#{category.id}"
+            }
+
+            expect(response.status).to eq(200)
+            expect(response.parsed_body["user_count"]).to eq(User.real.not_staged.count)
+          end
+
+          it 'returns the right count for the default_sidebar_tags site setting' do
+            tag = Fabricate(:tag)
+
+            put "/admin/site_settings/default_sidebar_tags/user_count.json", params: {
+              default_sidebar_tags: "#{tag.name}"
+            }
+
+            expect(response.status).to eq(200)
+            expect(response.parsed_body["user_count"]).to eq(User.real.not_staged.count)
+          end
+        end
+
         context "with user options" do
           def expect_user_count(site_setting_name:, user_setting_name:, current_site_setting_value:, new_site_setting_value:,
                                 current_user_setting_value: nil, new_user_setting_value: nil)

spec/serializers/current_user_serializer_spec.rb

@@ -221,15 +221,15 @@ RSpec.describe CurrentUserSerializer do
   end
 
   describe '#sidebar_tags' do
-    fab!(:tag_1) { Fabricate(:tag, name: "foo") }
+    fab!(:tag) { Fabricate(:tag, name: "foo") }
-    fab!(:tag_2) { Fabricate(:tag, name: "bar") }
+    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"]) }
-    let(:tag_sidebar_section_link) { Fabricate(:tag_sidebar_section_link, user: user) }
+    fab!(:tag_sidebar_section_link) { Fabricate(:tag_sidebar_section_link, user: user, linkable: tag) }
-    let(:tag_sidebar_section_link_2) { Fabricate(:tag_sidebar_section_link, user: user) }
+    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 experimental sidebar has not been enabled" do
-      tag_sidebar_section_link
       SiteSetting.enable_experimental_sidebar_hamburger = false
       SiteSetting.tagging_enabled = true
 
@@ -239,7 +239,6 @@ RSpec.describe CurrentUserSerializer do
     end
 
     it "is not included when tagging has not been enabled" do
-      tag_sidebar_section_link
       SiteSetting.enable_experimental_sidebar_hamburger = true
       SiteSetting.tagging_enabled = false
 
@@ -248,54 +247,37 @@ RSpec.describe CurrentUserSerializer do
       expect(json[:sidebar_tags]).to eq(nil)
     end
 
-    it "is present when experimental sidebar and tagging has been enabled" do
+    it "serializes only the tags that the user can see when experimental sidebar and tagging has been enabled" do
-      tag_sidebar_section_link
       SiteSetting.enable_experimental_sidebar_hamburger = true
       SiteSetting.tagging_enabled = true
 
-      tag_sidebar_section_link_2.linkable.update!(pm_topic_count: 5, topic_count: 0)
-
       json = serializer.as_json
 
       expect(json[:sidebar_tags]).to contain_exactly(
-        { name: tag_sidebar_section_link.linkable.name, pm_only: false },
+        { name: tag.name, pm_only: false },
-        { name: tag_sidebar_section_link_2.linkable.name, pm_only: true }
+        { name: pm_tag.name, pm_only: true }
       )
-    end
 
-    it 'includes visible default sidebar tags' do
+      user.update!(admin: true)
-      SiteSetting.enable_experimental_sidebar_hamburger = true
-      SiteSetting.tagging_enabled = true
-      SiteSetting.default_sidebar_tags = "foo|bar|secret"
 
       json = serializer.as_json
 
-      expect(json[:sidebar_tags]).to eq([
+      expect(json[:sidebar_tags]).to contain_exactly(
-        { name: "foo", pm_only: false },
+        { name: tag.name, pm_only: false },
-        { name: "bar", pm_only: false }
+        { name: pm_tag.name, pm_only: true },
-      ])
+        { name: hidden_tag.name, pm_only: false }
-    end
+      )
-
-    it 'includes tags choosen by user' do
-      SiteSetting.enable_experimental_sidebar_hamburger = true
-      SiteSetting.tagging_enabled = true
-      SiteSetting.default_sidebar_tags = "foo|bar|secret"
-      tag_sidebar_section_link = Fabricate(:tag_sidebar_section_link, user: user)
-
-      json = serializer.as_json
-
-      expect(json[:sidebar_tags]).to eq([
-        { name: tag_sidebar_section_link.linkable.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: Fabricate(:group)) }
+    fab!(:private_category) { Fabricate(:private_category, group: group) }
-    let(:category_sidebar_section_link) { Fabricate(:category_sidebar_section_link, user: user) }
+    fab!(:category_sidebar_section_link) { Fabricate(:category_sidebar_section_link, user: user, linkable: category) }
-    let(:category_sidebar_section_link_2) { Fabricate(:category_sidebar_section_link, user: user) }
+    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 SiteSetting.enable_experimental_sidebar_hamburger is false" do
       category_sidebar_section_link
@@ -307,7 +289,6 @@ RSpec.describe CurrentUserSerializer do
     end
 
     it "is not included when experimental sidebar has not been enabled" do
-      category_sidebar_section_link
       SiteSetting.enable_experimental_sidebar_hamburger = false
 
       json = serializer.as_json
@@ -315,23 +296,25 @@ RSpec.describe CurrentUserSerializer do
       expect(json[:sidebar_category_ids]).to eq(nil)
     end
 
-    it 'includes visible default sidebar categories' do
+    it 'serializes only the categories that the user can see when experimental sidebar and tagging has been enabled"' do
       SiteSetting.enable_experimental_sidebar_hamburger = true
-      SiteSetting.default_sidebar_categories = "#{category.id}|#{category_2.id}|#{private_category.id}"
 
       json = serializer.as_json
-      expect(json[:sidebar_category_ids]).to eq([category.id, category_2.id])
-    end
 
-    it 'includes categories choosen by user' do
+      expect(json[:sidebar_category_ids]).to eq([
-      SiteSetting.enable_experimental_sidebar_hamburger = true
+        category.id,
-      SiteSetting.default_sidebar_categories = "#{category.id}|#{category_2.id}|#{private_category.id}"
+        category_2.id
-
+      ])
-      category_sidebar_section_link
-      category_sidebar_section_link_2
 
+      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_sidebar_section_link.linkable.id, category_sidebar_section_link_2.linkable.id])
+
+      expect(json[:sidebar_category_ids]).to eq([
+        category.id,
+        category_2.id,
+        private_category.id
+      ])
     end
   end
 

spec/serializers/site_serializer_spec.rb

@@ -141,6 +141,16 @@ RSpec.describe SiteSerializer do
 
   describe '#anonymous_default_sidebar_tags' do
     fab!(:user) { Fabricate(:user) }
+    fab!(:tag) { Fabricate(:tag, name: 'dev') }
+    fab!(:tag2) { Fabricate(:tag, name: 'random') }
+    fab!(:hidden_tag) { Fabricate(:tag, name: "secret") }
+    fab!(:staff_tag_group) { Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: [hidden_tag.name]) }
+
+    before do
+      SiteSetting.enable_experimental_sidebar_hamburger = true
+      SiteSetting.tagging_enabled = true
+      SiteSetting.default_sidebar_tags = "#{tag.name}|#{tag2.name}|#{hidden_tag.name}"
+    end
 
     it 'is not included in the serialised object when tagging is not enabled' do
       SiteSetting.tagging_enabled = false
@@ -150,33 +160,30 @@ RSpec.describe SiteSerializer do
       expect(serialized[:anonymous_default_sidebar_tags]).to eq(nil)
     end
 
-    describe 'when tagging is enabled and default sidebar tags have been configured' do
+    it 'is not included in the serialised object when experimental sidebar has not been enabled' do
-      fab!(:tag) { Fabricate(:tag, name: 'dev') }
+      SiteSetting.enable_experimental_sidebar_hamburger = false
-      fab!(:tag2) { Fabricate(:tag, name: 'random') }
 
-      before do
+      serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json
-        SiteSetting.tagging_enabled = true
+      expect(serialized[:anonymous_default_sidebar_tags]).to eq(nil)
-        SiteSetting.default_sidebar_tags = "#{tag.name}|#{tag2.name}"
+    end
-      end
 
-      it 'is not included in the serialised object when user is not anonymous' do
+    it 'is not included in the serialised object when user is not anonymous' do
-        guardian = Guardian.new(user)
+      guardian = Guardian.new(user)
 
-        serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json
+      serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json
-        expect(serialized[:anonymous_default_sidebar_tags]).to eq(nil)
+      expect(serialized[:anonymous_default_sidebar_tags]).to eq(nil)
-      end
+    end
 
-      it 'is not included in the serialisd object when default sidebar tags have not been configured' do
+    it 'is not included in the serialisd object when default sidebar tags have not been configured' do
-        SiteSetting.default_sidebar_tags = ""
+      SiteSetting.default_sidebar_tags = ""
 
-        serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json
+      serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json
-        expect(serialized[:anonymous_default_sidebar_tags]).to eq(nil)
+      expect(serialized[:anonymous_default_sidebar_tags]).to eq(nil)
-      end
+    end
 
-      it 'is included in the serialised object when user is anonymous' do
+    it 'includes only tags user can see in the serialised object when user is anonymous' do
-        serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json
+      serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json
-        expect(serialized[:anonymous_default_sidebar_tags]).to eq(["dev", "random"])
+      expect(serialized[:anonymous_default_sidebar_tags]).to eq(["dev", "random"])
-      end
     end
   end
 

spec/services/sidebar_site_settings_backfiller_spec.rb

@@ -0,0 +1,235 @@
+# frozen_string_literal: true
+
+RSpec.describe SidebarSiteSettingsBackfiller do
+  fab!(:user) { Fabricate(:user) }
+  fab!(:user2) { Fabricate(:user) }
+  fab!(:user3) { Fabricate(:user) }
+  fab!(:staged_user) { Fabricate(:user, staged: true) }
+  fab!(:category) { Fabricate(:category) }
+  fab!(:category2) { Fabricate(:category) }
+  fab!(:category3) { Fabricate(:category) }
+  fab!(:user_category_sidebar_section_link) { Fabricate(:category_sidebar_section_link, user: user, linkable: category) }
+  fab!(:user2_category_sidebar_section_link) { Fabricate(:category_sidebar_section_link, user: user2, linkable: category) }
+  fab!(:user3_category2_sidebar_section_link) { Fabricate(:category_sidebar_section_link, user: user3, linkable: category2) }
+
+  let!(:category_sidebar_section_link_ids) do
+    [
+      user_category_sidebar_section_link.id,
+      user2_category_sidebar_section_link.id,
+      user3_category2_sidebar_section_link.id
+    ]
+  end
+
+  fab!(:tag) { Fabricate(:tag) }
+  fab!(:tag2) { Fabricate(:tag) }
+  fab!(:tag3) { Fabricate(:tag) }
+  fab!(:user_tag_sidebar_section_link) { Fabricate(:tag_sidebar_section_link, user: user, linkable: tag) }
+  fab!(:user2_tag_sidebar_section_link) { Fabricate(:tag_sidebar_section_link, user: user2, linkable: tag) }
+  fab!(:user3_tag2_sidebar_section_link) { Fabricate(:tag_sidebar_section_link, user: user3, linkable: tag2) }
+
+  let!(:tag_sidebar_section_link_ids) do
+    [
+      user_tag_sidebar_section_link.id,
+      user2_tag_sidebar_section_link.id,
+      user3_tag2_sidebar_section_link.id
+    ]
+  end
+
+  before do
+    # Clean up random users created as part of fabrication to make assertions easier to understand.
+    User.real.where("id NOT IN (?)", [user.id, user2.id, user3.id, staged_user.id]).delete_all
+  end
+
+  it 'raises an error when class is initialized with invalid setting name' do
+    expect do
+      described_class.new('some_random_setting_name', previous_value: '', new_value: '')
+    end.to raise_error(RuntimeError, "Invalid setting_name")
+  end
+
+  describe '#backfill!' do
+    context 'for default_sidebar_categories setting' do
+      it 'deletes the right sidebar section link records when categories are removed' do
+        backfiller = described_class.new(
+          "default_sidebar_categories",
+          previous_value: "#{category.id}|#{category2.id}|#{category3.id}",
+          new_value: "#{category3.id}"
+        )
+
+        expect do
+          backfiller.backfill!
+        end.to change { SidebarSectionLink.count }.by(-3)
+
+        expect(SidebarSectionLink.exists?(id: category_sidebar_section_link_ids)).to eq(false)
+      end
+
+      it 'creates the right sidebar section link records when categories are added' do
+        backfiller = described_class.new(
+          "default_sidebar_categories",
+          previous_value: "#{category.id}|#{category2.id}",
+          new_value: "#{category.id}|#{category2.id}|#{category3.id}"
+        )
+
+        expect do
+          backfiller.backfill!
+        end.to change { SidebarSectionLink.count }.by(3)
+
+        expect(SidebarSectionLink.where(linkable_type: 'Category', linkable_id: category3.id).pluck(:user_id)).to contain_exactly(
+          user.id,
+          user2.id,
+          user3.id
+        )
+      end
+
+      it 'deletes and creates the right sidebar section link records when categories are added and removed' do
+        backfiller = described_class.new(
+          "default_sidebar_categories",
+          previous_value: "#{category.id}|#{category2.id}",
+          new_value: "#{category3.id}"
+        )
+
+        original_count = SidebarSectionLink.count
+
+        expect do
+          backfiller.backfill!
+        end.to change { SidebarSectionLink.where(linkable_type: 'Category', linkable_id: category.id).count }.by(-2)
+          .and change { SidebarSectionLink.where(linkable_type: 'Category', linkable_id: category2.id).count }.by(-1)
+          .and change { SidebarSectionLink.where(linkable_type: 'Category', linkable_id: category3.id).count }.by(3)
+
+        expect(SidebarSectionLink.count).to eq(original_count) # Net change of 0
+
+        expect(SidebarSectionLink.where(linkable_type: 'Category', linkable_id: category3.id).pluck(:user_id)).to contain_exactly(
+          user.id,
+          user2.id,
+          user3.id
+        )
+      end
+    end
+
+    context 'for default_sidebar_tags setting' do
+      it 'deletes the right sidebar section link records when tags are removed' do
+        backfiller = described_class.new(
+          "default_sidebar_tags",
+          previous_value: "#{tag.name}|#{tag2.name}|#{tag3.name}",
+          new_value: "#{tag3.name}"
+        )
+
+        expect do
+          backfiller.backfill!
+        end.to change { SidebarSectionLink.count }.by(-3)
+
+        expect(SidebarSectionLink.exists?(id: tag_sidebar_section_link_ids)).to eq(false)
+      end
+
+      it 'creates the right sidebar section link records when tags are added' do
+        backfiller = described_class.new(
+          "default_sidebar_tags",
+          previous_value: "#{tag.name}|#{tag2.name}",
+          new_value: "#{tag.name}|#{tag2.name}|#{tag3.name}"
+        )
+
+        expect do
+          backfiller.backfill!
+        end.to change { SidebarSectionLink.count }.by(3)
+
+        expect(SidebarSectionLink.where(linkable_type: 'Tag', linkable_id: tag3.id).pluck(:user_id)).to contain_exactly(
+          user.id,
+          user2.id,
+          user3.id
+        )
+      end
+
+      it 'deletes and creates the right sidebar section link records when tags are added and removed' do
+        backfiller = described_class.new(
+          "default_sidebar_tags",
+          previous_value: "#{tag.name}|#{tag2.name}",
+          new_value: "#{tag3.name}"
+        )
+
+        original_count = SidebarSectionLink.count
+
+        expect do
+          backfiller.backfill!
+        end.to change { SidebarSectionLink.where(linkable_type: 'Tag', linkable_id: tag.id).count }.by(-2)
+          .and change { SidebarSectionLink.where(linkable_type: 'Tag', linkable_id: tag2.id).count }.by(-1)
+          .and change { SidebarSectionLink.where(linkable_type: 'Tag', linkable_id: tag3.id).count }.by(3)
+
+        expect(SidebarSectionLink.count).to eq(original_count) # net change of 0
+
+        expect(SidebarSectionLink.where(linkable_type: 'Tag', linkable_id: tag3.id).pluck(:user_id)).to contain_exactly(
+          user.id,
+          user2.id,
+          user3.id
+        )
+      end
+    end
+  end
+
+  describe '#number_of_users_to_backfill' do
+    context 'for default_sidebar_categories setting' do
+      it "returns 3 for the user count when a new category for all users is added" do
+        backfiller = described_class.new(
+          "default_sidebar_categories",
+          previous_value: "",
+          new_value: "#{category3.id}"
+        )
+
+        expect(backfiller.number_of_users_to_backfill).to eq(3)
+      end
+
+      it "returns 2 for the user count when category which 2 users have configured in sidebar is removed" do
+        backfiller = described_class.new(
+          "default_sidebar_categories",
+          previous_value: "#{category.id}|#{category2.id}",
+          new_value: "#{category2.id}"
+        )
+
+        expect(backfiller.number_of_users_to_backfill).to eq(2)
+      end
+
+      # category, category2 => category2, category3
+      it "returns 3 for the user count when a new category is added and a category is removed" do
+        backfiller = described_class.new(
+          "default_sidebar_categories",
+          previous_value: "#{category.id}|#{category2.id}",
+          new_value: "#{category2.id}|#{category3.id}"
+        )
+
+        expect(backfiller.number_of_users_to_backfill).to eq(3)
+      end
+    end
+
+    context 'for default_sidebar_tags setting' do
+      it "returns 3 for the user count when a new tag for all users is added" do
+        backfiller = described_class.new(
+          "default_sidebar_tags",
+          previous_value: "",
+          new_value: "#{tag3.name}"
+        )
+
+        expect(backfiller.number_of_users_to_backfill).to eq(3)
+      end
+
+      # tag, tag2 => tag2
+      it "returns 2 for the user count when tag which 2 users have configured in sidebar is removed" do
+        backfiller = described_class.new(
+          "default_sidebar_tags",
+          previous_value: "#{tag.name}|#{tag2.name}",
+          new_value: "#{tag2.name}"
+        )
+
+        expect(backfiller.number_of_users_to_backfill).to eq(2)
+      end
+
+      # tag, tag2 => tag2, tag3
+      it "returns 3 for the user count when a new tag is added and a tag is removed" do
+        backfiller = described_class.new(
+          "default_sidebar_tags",
+          previous_value: "#{tag.name}|#{tag2.name}",
+          new_value: "#{tag2.name}|#{tag3.name}"
+        )
+
+        expect(backfiller.number_of_users_to_backfill).to eq(3)
+      end
+    end
+  end
+end