From 836dee1120b04b10b5ed73426f76765b9d8c8d42 Mon Sep 17 00:00:00 2001
From: Leo McArdle <leomcardle@gmail.com>
Date: Mon, 31 Jul 2017 22:27:29 +0000
Subject: [PATCH] FIX: add additional email to tests and clean up resulting
 mess

---
 app/controllers/admin/groups_controller.rb   | 1 +
 app/jobs/scheduled/pending_users_reminder.rb | 2 +-
 lib/admin_user_index_query.rb                | 5 +++--
 spec/fabricators/user_email_fabricator.rb    | 5 +++++
 spec/fabricators/user_fabricator.rb          | 6 +++++-
 spec/models/user_spec.rb                     | 2 +-
 spec/services/user_destroyer_spec.rb         | 4 ++--
 7 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb
index 1ad9367aa8e..c49edc630d2 100644
--- a/app/controllers/admin/groups_controller.rb
+++ b/app/controllers/admin/groups_controller.rb
@@ -23,6 +23,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 4f5524ec9d4..534e45179f0 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -612,7 +612,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