FEATURE: Add order logic to admin users controller

Added order and direction parameters for sorting admin user pages. This
commit only includes backend api changes.

https://meta.discourse.org/t/make-admin-users-list-sortable-suggestion/47649

Now you can pass in `order` and `asc` parameters to the
`/admin/users/list/<query>.json` endpoint.

Example:

`/admin/users/list/active.json?&order=post_count` which defaults to desc

and

`/admin/users/list/active.json?order=post_count&asc=true`

app/jobs/scheduled/pending_users_reminder.rb

@@ -7,7 +7,7 @@ module Jobs
 
     def execute(args)
       if SiteSetting.must_approve_users && SiteSetting.pending_users_reminder_delay >= 0
-        query = AdminUserIndexQuery.new({query: 'pending'}).find_users_query # default order is: users.created_at DESC
+        query = AdminUserIndexQuery.new({query: 'pending', stats: false}).find_users_query # default order is: users.created_at DESC
         if SiteSetting.pending_users_reminder_delay > 0
           query = query.where('users.created_at < ?', SiteSetting.pending_users_reminder_delay.hours.ago)
         end

lib/admin_user_index_query.rb

@@ -11,15 +11,28 @@ class AdminUserIndexQuery
   attr_reader :params, :trust_levels
 
   def find_users(limit=100)
-    find_users_query.includes(:user_stat).limit(limit)
+    find_users_query.limit(limit)
   end
 
   def count_users
     find_users_query.count
   end
 
-  def self.orderable_columns
-    %w(created_at days_visited posts_read_count topics_entered post_count trust_level)
+  def self.orderable_user_columns
+    %w(created_at trust_level last_emailed_at last_seen_at username email)
+  end
+  
+  def self.orderable_stat_columns
+    %w(days_visited posts_read_count topics_entered post_count time_read)
+  end
+
+  def custom_direction
+    asc = params[:asc]
+    asc.present? && asc ? "ASC" : "DESC"
+  end
+
+  def pg_coalesce(column)
+    "COALESCE(#{column}, to_date('1970-01-01', 'YYYY-MM-DD'))"
   end
 
   def initialize_query_with_order(klass)
@@ -28,20 +41,31 @@ class AdminUserIndexQuery
     custom_order = params[:order]
     if custom_order.present? &&
       without_dir = custom_order.downcase.sub(/ (asc|desc)$/, '')
-      if AdminUserIndexQuery.orderable_columns.include?(without_dir)
-        order << custom_order
+      if AdminUserIndexQuery.orderable_user_columns.include?(without_dir)
+        without_dir = (without_dir == "last_seen_at") ? pg_coalesce("last_seen_at") : without_dir
+        without_dir = (without_dir == "last_emailed_at") ? pg_coalesce("last_emailed_at") : without_dir
+        order << "#{without_dir} #{custom_direction}"
+      end
+      if AdminUserIndexQuery.orderable_stat_columns.include?(without_dir)
+        order << "user_stats.#{without_dir} #{custom_direction}"
       end
     end
+    
+    if !custom_order.present?
+      if params[:query] == "active"
+        order << "#{pg_coalesce("last_seen_at")} DESC"
+      else
+        order << "users.created_at DESC"
+      end
 
-    if params[:query] == "active"
-      order << "COALESCE(last_seen_at, to_date('1970-01-01', 'YYYY-MM-DD')) DESC"
-    else
-      order << "users.created_at DESC"
+      order << "users.username"
     end
 
-    order << "users.username"
-
-    klass.order(order.reject(&:blank?).join(","))
+    if params[:stats].present? && params[:stats] == false
+      klass.order(order.reject(&:blank?).join(","))
+    else
+      klass.includes(:user_stat).order(order.reject(&:blank?).join(","))
+    end
   end
 
   def filter_by_trust

spec/components/admin_user_index_query_spec.rb

@@ -23,9 +23,24 @@ describe AdminUserIndexQuery do
     end
 
     it "allows custom ordering" do
-      query = ::AdminUserIndexQuery.new({ order: "trust_level DESC" })
+      query = ::AdminUserIndexQuery.new({ order: "trust_level" })
       expect(query.find_users_query.to_sql).to match("trust_level DESC")
     end
+    
+    it "allows custom ordering asc" do
+      query = ::AdminUserIndexQuery.new({ order: "trust_level", asc: true })
+      expect(query.find_users_query.to_sql).to match("trust_level ASC" )
+    end
+
+    it "allows custom ordering for stats wtih default direction" do
+      query = ::AdminUserIndexQuery.new({ order: "topics_entered" })
+      expect(query.find_users_query.to_sql).to match("topics_entered DESC")
+    end
+
+    it "allows custom ordering and direction for stats" do
+      query = ::AdminUserIndexQuery.new({ order: "topics_entered", asc: true })
+      expect(query.find_users_query.to_sql).to match("topics_entered ASC")
+    end
   end
 
   describe "no users with trust level" do
@@ -70,6 +85,28 @@ describe AdminUserIndexQuery do
 
   end
 
+  describe "correct order with nil values" do
+    before(:each) do
+      Fabricate(:user, email: "test2@example.com", last_emailed_at: 1.hour.ago)
+    end
+
+    it "shows nil values first with asc" do
+      users = ::AdminUserIndexQuery.new({ order: "last_emailed_at", asc: true }).find_users
+
+      expect(users.count).to eq(2)
+      expect(users.first.username).to eq("system")
+      expect(users.first.last_emailed_at).to eq(nil)
+    end
+
+    it "shows nil values last with desc" do
+      users = ::AdminUserIndexQuery.new({ order: "last_emailed_at"}).find_users
+
+      expect(users.count).to eq(2)
+      expect(users.first.last_emailed_at).to_not eq(nil)
+    end
+
+  end
+
   describe "with an admin user" do
 
     let!(:user) { Fabricate(:user, admin: true) }