diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index e7e169d9462..99e2dc8dc2a 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -37,6 +37,7 @@ class Admin::GroupsController < Admin::AdminController valid_emails[email] = valid_usernames[username_lower] = id id end + valid_users.uniq! invalid_users = users.reject! { |u| valid_emails[u] || valid_usernames[u] } group.bulk_add(valid_users) if valid_users.present? users_added = valid_users.count diff --git a/app/jobs/scheduled/pending_users_reminder.rb b/app/jobs/scheduled/pending_users_reminder.rb index 5c682bb4e35..36448e37c26 100644 --- a/app/jobs/scheduled/pending_users_reminder.rb +++ b/app/jobs/scheduled/pending_users_reminder.rb @@ -12,7 +12,7 @@ module Jobs query = query.where('users.created_at < ?', SiteSetting.pending_users_reminder_delay.hours.ago) end - newest_username = query.limit(1).pluck(:username).first + newest_username = query.limit(1).select(:username).first&.username return true if newest_username == previous_newest_username # already notified diff --git a/lib/admin_user_index_query.rb b/lib/admin_user_index_query.rb index 0f216c606bc..080b2135de2 100644 --- a/lib/admin_user_index_query.rb +++ b/lib/admin_user_index_query.rb @@ -4,7 +4,8 @@ class AdminUserIndexQuery def initialize(params = {}, klass = User, trust_levels = TrustLevel.levels) @params = params - @query = initialize_query_with_order(klass).joins(:user_emails) + @outer_query = initialize_query_with_order(klass) + @query = klass.joins(:user_emails).distinct @trust_levels = trust_levels end @@ -134,7 +135,7 @@ class AdminUserIndexQuery append filter_by_ip append filter_exclude append filter_by_search - @query + @outer_query.from(@query, 'users') end end diff --git a/spec/fabricators/user_email_fabricator.rb b/spec/fabricators/user_email_fabricator.rb index 3d0d0cc8b0c..8e7814bf219 100644 --- a/spec/fabricators/user_email_fabricator.rb +++ b/spec/fabricators/user_email_fabricator.rb @@ -2,3 +2,8 @@ Fabricator(:user_email) do email { sequence(:email) { |i| "bruce#{i}@wayne.com" } } primary true end + +Fabricator(:alternate_email, from: :user_email) do + email { sequence(:email) { |i| "bwayne#{i}@wayne.com" } } + primary false +end diff --git a/spec/fabricators/user_fabricator.rb b/spec/fabricators/user_fabricator.rb index 067f335ac81..fe1dd39944f 100644 --- a/spec/fabricators/user_fabricator.rb +++ b/spec/fabricators/user_fabricator.rb @@ -1,7 +1,7 @@ Fabricator(:user_stat) do end -Fabricator(:user) do +Fabricator(:user_single_email, class_name: :user) do name 'Bruce Wayne' username { sequence(:username) { |i| "bruce#{i}" } } email { sequence(:email) { |i| "bruce#{i}@wayne.com" } } @@ -11,6 +11,10 @@ Fabricator(:user) do active true end +Fabricator(:user, from: :user_single_email) do + after_create { |user| Fabricate(:alternate_email, user: user) } +end + Fabricator(:coding_horror, from: :user) do name 'Coding Horror' username 'CodingHorror' diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 2f2eb281d22..8bb2f6410dd 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -615,7 +615,7 @@ describe User do it 'email whitelist should be used when email is being changed' do SiteSetting.email_domains_whitelist = 'vaynermedia.com' - u = Fabricate(:user, email: 'good@vaynermedia.com') + u = Fabricate(:user_single_email, email: 'good@vaynermedia.com') u.email = 'nope@mailinator.com' expect(u).not_to be_valid end diff --git a/spec/services/user_destroyer_spec.rb b/spec/services/user_destroyer_spec.rb index d803e9f2ee4..838607a45a1 100644 --- a/spec/services/user_destroyer_spec.rb +++ b/spec/services/user_destroyer_spec.rb @@ -53,10 +53,10 @@ describe UserDestroyer do destroy end - it "adds email to block list if block_email is true" do + it "adds emails to block list if block_email is true" do expect { UserDestroyer.new(@admin).destroy(@user, destroy_opts.merge(block_email: true)) - }.to change { ScreenedEmail.count }.by(1) + }.to change { ScreenedEmail.count }.by(2) end end