Merge pull request #1573 from railsaholic/use_existing_find_by_username_or_email

Utilize already existing method 'find_by_username_or_email'

app/controllers/session_controller.rb

@@ -10,14 +10,11 @@ class SessionController < ApplicationController
     params.require(:login)
     params.require(:password)
 
-    login = params[:login].strip
-    login = login[1..-1] if login[0] == "@"
+    login    = params[:login].strip
+    password = params[:password]
+    login    = login[1..-1] if login[0] == "@"
 
-    if login =~ /@/
-      @user = User.where(email: Email.downcase(login)).first
-    else
-      @user = User.where(username_lower: login.downcase).first
-    end
+    @user = User.find_by_username_or_email(login)
 
     if @user.present?
 
@@ -28,7 +25,7 @@ class SessionController < ApplicationController
       end
 
       # If their password is correct
-      if @user.confirm_password?(params[:password])
+      if @user.confirm_password?(password)
 
         if @user.is_banned?
           render json: { error: I18n.t("login.banned", {date: I18n.l(@user.banned_till, format: :date_only)}) }
@@ -57,7 +54,7 @@ class SessionController < ApplicationController
   def forgot_password
     params.require(:login)
 
-    user = User.where('username_lower = :username or email = :email', username: params[:login].downcase, email: Email.downcase(params[:login])).first
+    user = User.find_by_username_or_email(params[:login])
     if user.present?
       email_token = user.email_tokens.create(email: user.email)
       Jobs.enqueue(:user_email, type: :forgot_password, user_id: user.id, email_token: email_token.token)

app/models/user.rb

@@ -124,13 +124,11 @@ class User < ActiveRecord::Base
   end
 
   def self.find_by_username_or_email(username_or_email)
-    conditions = if username_or_email.include?('@')
-      { email: Email.downcase(username_or_email) }
-    else
-      { username_lower: username_or_email.downcase }
-    end
-
-    users = User.where(conditions).to_a
+    users = if username_or_email.include?('@')
+              find_by_email(username_or_email)
+            else
+              find_by_username(username_or_email)
+            end
 
     if users.size > 1
       raise Discourse::TooManyMatches
@@ -139,6 +137,14 @@ class User < ActiveRecord::Base
     end
   end
 
+  def self.find_by_email(email)
+    where(email: Email.downcase(email))
+  end
+
+  def self.find_by_username(username)
+    where(username_lower: username.downcase)
+  end
+
   def enqueue_welcome_message(message_type)
     return unless SiteSetting.send_welcome_message?
     Jobs.enqueue(:send_system_message, user_id: id, message_type: message_type)

spec/models/user_spec.rb

@@ -829,8 +829,8 @@ describe User do
 
     context 'when multiple users are found' do
       it 'raises an exception' do
-        user_query = stub(to_a: [stub, stub])
-        User.stubs(:where).with(username_lower: 'bob').returns(user_query)
+        user_query = [stub, stub]
+        User.stubs(:find_by_username).with('bob').returns(user_query)
 
         expect { User.find_by_username_or_email('bob') }.to raise_error(Discourse::TooManyMatches)
       end