From a1df68d4c417cd3d4f5de0ad8c9a76c4f2b15a86 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Thu, 18 Jun 2020 21:09:54 +0300 Subject: [PATCH] FIX: Do not change tracked categories for staged users (#10076) --- .../admin/site_settings_controller.rb | 8 ++++-- .../admin/site_settings_controller_spec.rb | 25 +++++++++++-------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/app/controllers/admin/site_settings_controller.rb b/app/controllers/admin/site_settings_controller.rb index 4d662f7dd50..c143e04dca7 100644 --- a/app/controllers/admin/site_settings_controller.rb +++ b/app/controllers/admin/site_settings_controller.rb @@ -61,7 +61,7 @@ class Admin::SiteSettingsController < Admin::AdminController (new_category_ids - previous_category_ids).each do |category_id| skip_user_ids = CategoryUser.where(category_id: category_id).pluck(:user_id) - User.where.not(id: skip_user_ids).select(:id).find_in_batches do |users| + User.real.where(staged: false).where.not(id: skip_user_ids).select(:id).find_in_batches do |users| category_users = [] users.each { |user| category_users << { category_id: category_id, user_id: user.id, notification_level: notification_level } } CategoryUser.insert_all!(category_users) @@ -88,7 +88,7 @@ class Admin::SiteSettingsController < Admin::AdminController (new_tag_ids - previous_tag_ids).each do |tag_id| skip_user_ids = TagUser.where(tag_id: tag_id).pluck(:user_id) - User.where.not(id: skip_user_ids).select(:id).find_in_batches do |users| + User.real.where(staged: false).where.not(id: skip_user_ids).select(:id).find_in_batches do |users| tag_users = [] users.each { |user| tag_users << { tag_id: tag_id, user_id: user.id, notification_level: notification_level, created_at: now, updated_at: now } } TagUser.insert_all!(tag_users) @@ -135,8 +135,10 @@ class Admin::SiteSettingsController < Admin::AdminController user_ids = CategoryUser.where(category_id: previous_category_ids - new_category_ids, notification_level: notification_level).distinct.pluck(:user_id) user_ids += User + .real .joins("CROSS JOIN categories c") .joins("LEFT JOIN category_users cu ON users.id = cu.user_id AND c.id = cu.category_id") + .where(staged: false) .where("c.id IN (?) AND cu.notification_level IS NULL", new_category_ids - previous_category_ids) .distinct .pluck("users.id") @@ -159,8 +161,10 @@ class Admin::SiteSettingsController < Admin::AdminController user_ids = TagUser.where(tag_id: previous_tag_ids - new_tag_ids, notification_level: notification_level).distinct.pluck(:user_id) user_ids += User + .real .joins("CROSS JOIN tags t") .joins("LEFT JOIN tag_users tu ON users.id = tu.user_id AND t.id = tu.tag_id") + .where(staged: false) .where("t.id IN (?) AND tu.notification_level IS NULL", new_tag_ids - previous_tag_ids) .distinct .pluck("users.id") diff --git a/spec/requests/admin/site_settings_controller_spec.rb b/spec/requests/admin/site_settings_controller_spec.rb index 7962ab295df..1b74fb9f645 100644 --- a/spec/requests/admin/site_settings_controller_spec.rb +++ b/spec/requests/admin/site_settings_controller_spec.rb @@ -97,8 +97,9 @@ describe Admin::SiteSettingsController do end describe 'default categories' do - let(:user1) { Fabricate(:user) } - let(:user2) { Fabricate(:user) } + fab!(:user1) { Fabricate(:user) } + fab!(:user2) { Fabricate(:user) } + fab!(:staged_user) { Fabricate(:staged) } let(:watching) { NotificationLevels.all[:watching] } let(:tracking) { NotificationLevels.all[:tracking] } @@ -120,7 +121,7 @@ describe Admin::SiteSettingsController do } expect(CategoryUser.where(category_id: category_ids.first, notification_level: watching).count).to eq(0) - expect(CategoryUser.where(category_id: category_ids.last, notification_level: watching).count).to eq(User.count - 1) + expect(CategoryUser.where(category_id: category_ids.last, notification_level: watching).count).to eq(User.real.where(staged: false).count - 1) end it 'should not update existing users user preference' do @@ -135,8 +136,9 @@ describe Admin::SiteSettingsController do end describe 'default tags' do - let(:user1) { Fabricate(:user) } - let(:user2) { Fabricate(:user) } + fab!(:user1) { Fabricate(:user) } + fab!(:user2) { Fabricate(:user) } + fab!(:staged_user) { Fabricate(:staged) } let(:watching) { NotificationLevels.all[:watching] } let(:tracking) { NotificationLevels.all[:tracking] } @@ -158,7 +160,7 @@ describe Admin::SiteSettingsController do } expect(TagUser.where(tag_id: tags.first.id, notification_level: watching).count).to eq(0) - expect(TagUser.where(tag_id: tags.last.id, notification_level: watching).count).to eq(User.count - 1) + expect(TagUser.where(tag_id: tags.last.id, notification_level: watching).count).to eq(User.real.where(staged: false).count - 1) end it 'should not update existing users user preference' do @@ -173,7 +175,8 @@ describe Admin::SiteSettingsController do end describe '#user_count' do - let(:user) { Fabricate(:user) } + fab!(:user) { Fabricate(:user) } + fab!(:staged_user) { Fabricate(:staged) } let(:tracking) { NotificationLevels.all[:tracking] } it 'should return correct user count for default categories change' do @@ -183,7 +186,7 @@ describe Admin::SiteSettingsController do default_categories_watching: category_id } - expect(response.parsed_body["user_count"]).to eq(User.count) + expect(response.parsed_body["user_count"]).to eq(User.real.where(staged: false).count) CategoryUser.create!(category_id: category_id, notification_level: tracking, user: user) @@ -191,7 +194,7 @@ describe Admin::SiteSettingsController do default_categories_watching: category_id } - expect(response.parsed_body["user_count"]).to eq(User.count - 1) + expect(response.parsed_body["user_count"]).to eq(User.real.where(staged: false).count - 1) SiteSetting.setting(:default_categories_watching, "") end @@ -203,7 +206,7 @@ describe Admin::SiteSettingsController do default_tags_watching: tag.name } - expect(response.parsed_body["user_count"]).to eq(User.count) + expect(response.parsed_body["user_count"]).to eq(User.real.where(staged: false).count) TagUser.create!(tag_id: tag.id, notification_level: tracking, user: user) @@ -211,7 +214,7 @@ describe Admin::SiteSettingsController do default_tags_watching: tag.name } - expect(response.parsed_body["user_count"]).to eq(User.count - 1) + expect(response.parsed_body["user_count"]).to eq(User.real.where(staged: false).count - 1) SiteSetting.setting(:default_tags_watching, "") end