sort users not logged in last when sorting by last login

test plan:
* on the account user search page, when searching by
 login descending, it should put users without any
 login time last

closes #CORE-1688

Change-Id: I2c13c52d6ada64c31865944f4a1bb32ebc2625ad
Reviewed-on: https://gerrit.instructure.com/159807
Tested-by: Jenkins
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>

app/controllers/courses_controller.rb

@@ -990,7 +990,7 @@ class CoursesController < ApplicationController
     get_context
     if authorized_action(@context, @current_user, :read_reports)
       scope = User.for_course_with_last_login(@context, @context.root_account_id, 'StudentEnrollment')
-      scope = scope.order('login_info_exists, last_login DESC')
+      scope = scope.order('last_login DESC NULLS LAST')
       users = Api.paginate(scope, self, api_v1_course_recent_students_url)
       user_json_preloads(users)
       render :json => users.map { |u| user_json(u, @current_user, session, ['last_login']) }

app/models/user.rb

@@ -305,7 +305,7 @@ class User < ActiveRecord::Base
 
   scope :for_course_with_last_login, lambda { |course, root_account_id, enrollment_type|
     # add a field to each user that is the aggregated max from current_login_at and last_login_at from their pseudonyms
-    scope = select("users.*, MAX(current_login_at) as last_login, MAX(current_login_at) IS NULL as login_info_exists").
+    scope = select("users.*, MAX(current_login_at) as last_login").
       # left outer join ensures we get the user even if they don't have a pseudonym
       joins(sanitize_sql([<<-SQL, root_account_id])).where(:enrollments => { :course_id => course })
         LEFT OUTER JOIN #{Pseudonym.quoted_table_name} ON pseudonyms.user_id = users.id AND pseudonyms.account_id = ?

lib/user_search.rb

@@ -80,9 +80,9 @@ module UserSearch
 
     users = if options[:sort] == "last_login"
               if options[:order] == 'desc'
-                users.order("MAX(current_login_at) desc, id desc")
+                users.order(Arel.sql("MAX(current_login_at) DESC NULLS LAST, id DESC"))
               else
-                users.order("MAX(current_login_at), id")
+                users.order(Arel.sql("MAX(current_login_at), id"))
               end
             elsif options[:sort] == "username"
               if options[:order] == 'desc'

spec/apis/v1/users_api_spec.rb

@@ -647,6 +647,12 @@ describe "Users API", type: :request do
 
       expect(json.count).to eq 1
       expect(json.first['last_login']).to eq p.current_login_at.iso8601
+
+      # it should sort too
+      json = api_call(:get, "/api/v1/accounts/#{@account.id}/users",
+        { :controller => 'users', :action => "index", :format => 'json', :account_id => @account.id.to_param },
+        { include: ['last_login'], sort: "last_login", order: 'desc'})
+      expect(json.first['last_login']).to eq p.current_login_at.iso8601
     end
   end