From 7e31a369f55764a6bccab340b8a3827710623793 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Wed, 17 Jun 2015 16:07:09 -0400
Subject: [PATCH] Spec and refactor User.find_for_commit

Now it executes a single query instead of a possible three at the cost
of some scary-looking ARel calls.
---
 app/models/user.rb       | 24 ++++++++++++++++++++----
 spec/models/user_spec.rb | 25 +++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 982c05212ce..4ef8259049d 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -220,10 +220,26 @@ class User < ActiveRecord::Base
     end
 
     def find_for_commit(email, name)
-      # Prefer email match over name match
-      User.where(email: email).first ||
-        User.joins(:emails).where(emails: { email: email }).first ||
-        User.where(name: name).first
+      user_table = arel_table
+      email_table = Email.arel_table
+
+      # Use ARel to build a query:
+      query = user_table.
+        # SELECT "users".* FROM "users"
+        project(user_table[Arel.star]).
+        # LEFT OUTER JOIN "emails"
+        join(email_table, Arel::Nodes::OuterJoin).
+        # ON "users"."id" = "emails"."user_id"
+        on(user_table[:id].eq(email_table[:user_id])).
+        # WHERE ("user"."email" = '<email>' OR "user"."name" = '<name>')
+        # OR "emails"."email" = '<email>'
+        where(
+          user_table[:email].eq(email).
+          or(user_table[:name].eq(name)).
+          or(email_table[:email].eq(email))
+        )
+
+      find_by_sql(query.to_sql).first
     end
 
     def filter(filter_name)
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index f3e278e5c5f..fcd65467285 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -340,6 +340,31 @@ describe User do
     end
   end
 
+  describe '.find_for_commit' do
+    it 'finds by primary email' do
+      user = create(:user, email: 'foo@example.com')
+
+      expect(User.find_for_commit(user.email, '')).to eq user
+    end
+
+    it 'finds by secondary email' do
+      email = create(:email, email: 'foo@example.com')
+      user  = email.user
+
+      expect(User.find_for_commit(email.email, '')).to eq user
+    end
+
+    it 'finds by name' do
+      user = create(:user, name: 'Joey JoJo')
+
+      expect(User.find_for_commit('', 'Joey JoJo')).to eq user
+    end
+
+    it 'returns nil when nothing found' do
+      expect(User.find_for_commit('', '')).to be_nil
+    end
+  end
+
   describe 'search' do
     let(:user1) { create(:user, username: 'James', email: 'james@testing.com') }
     let(:user2) { create(:user, username: 'jameson', email: 'jameson@example.com') }