Merge pull request #495 from coopdevs/members_by_login

sort members by last_sign_in_at by default

app/controllers/users_controller.rb

@@ -2,11 +2,11 @@ class UsersController < ApplicationController
   before_filter :authenticate_user!
 
   def index
-    search_and_load_members current_organization.members.active, {s: 'member_uid asc'}
+    search_and_load_members current_organization.members.active, { s: 'user_last_sign_in_at DESC' }
   end
 
   def manage
-    search_and_load_members current_organization.members, {s: 'member_uid asc'}
+    search_and_load_members current_organization.members, { s: 'member_uid ASC' }
   end
 
   def show
@@ -60,8 +60,11 @@ def update
   def search_and_load_members(members_scope, default_search_params)
     @search = members_scope.ransack(default_search_params.merge(params.fetch(:q, {})))
 
-    @members =
-      @search.result.eager_load(:account, :user).page(params[:page]).per(20)
+    result = @search.result
+    orders = result.orders.map { |order| order.direction == :asc ? "#{order.to_sql} NULLS FIRST" : "#{order.to_sql} NULLS LAST" }
+    result = result.except(:order).order(orders.join(", ")) if orders.count > 0
+
+    @members = result.eager_load(:account, :user).page(params[:page]).per(20)
 
     @member_view_models =
       @members.map { |m| MemberDecorator.new(m, self.class.helpers) }

spec/controllers/users_controller_spec.rb

@@ -40,12 +40,14 @@
   describe "GET #index" do
     before { login(user) }
 
-    it 'sorts the users by their member_uid asc by default' do
-      member.increment!(:member_uid, Member.maximum(:member_uid) + 1)
+    it 'sorts the users by their last_sign_in_at desc by default' do
+      member.user.update_column(:last_sign_in_at, DateTime.now)
+      another_member.user.update_column(:last_sign_in_at, nil)
 
       get :index
 
-      expect(assigns(:members).last).to eq(member)
+      expect(assigns(:members).first).to eq(member)
+      expect(assigns(:members).last).to eq(another_member)
     end
 
     it 'allows to sort by member_uid' do
@@ -81,8 +83,6 @@
 
     context "with an normal logged user" do
       it "populates and array of users" do
-        login(user)
-
         get "index"
 
         expect(assigns(:members).map(&:user))
@@ -101,17 +101,39 @@
       end
     end
 
+    context 'when searching' do
+      it 'allows to search by member_uid' do
+        user = Fabricate(:user, username: 'foo', email: '[email protected]')
+        member = Fabricate(:member, user: user, organization: test_organization, member_uid: 1000)
+
+        get :index, q: { user_username_or_user_email_or_member_uid_search_contains: 1000 }
+
+        expect(assigns(:members)).to include(member)
+      end
+    end
+  end
+
+  describe "GET #manage" do
+    before { login(user) }
+
+    it 'sorts the users by their member_uid asc by default' do
+      member.increment!(:member_uid, Member.maximum(:member_uid) + 1)
+
+      get :manage
+
+      expect(assigns(:members).last).to eq(member)
+    end
+
     context 'when sorting by balance' do
       before do
-        login(user)
         member_admin.account.update_attribute(:balance, 3600)
       end
 
       context 'desc' do
         let(:direction) { 'desc' }
 
         it 'orders the rows by their balance' do
-          get :index, q: { s: "account_balance #{direction}" }
+          get :manage, q: { s: "account_balance #{direction}" }
 
           expect(assigns(:members).pluck(:user_id).first).to eq(admin_user.id)
         end
@@ -121,23 +143,12 @@
         let(:direction) { 'asc' }
 
         it 'orders the rows by their balance' do
-          get :index, q: { s: "account_balance #{direction}" }
+          get :manage, q: { s: "account_balance #{direction}" }
 
           expect(assigns(:members).pluck(:user_id).last).to eq(admin_user.id)
         end
       end
     end
-
-    context 'when searching' do
-      it 'allows to search by member_uid' do
-        user = Fabricate(:user, username: 'foo', email: '[email protected]')
-        member = Fabricate(:member, user: user, organization: test_organization, member_uid: 1000)
-
-        get :index, q: { user_username_or_user_email_or_member_uid_search_contains: 1000 }
-
-        expect(assigns(:members)).to include(member)
-      end
-    end
   end
 
   describe "GET #show" do