From 0e6cb752dab3f1fd204f93e192b5626db066dd50 Mon Sep 17 00:00:00 2001
From: Blake Erickson <o.blakeerickson@gmail.com>
Date: Sat, 25 Feb 2017 11:51:40 -0700
Subject: [PATCH] Clean up valid order names

Add a sortable mappings list to match other endpoints and so that you
don't have to use database column names.

Example: 'created' => 'created_at'

Also cleaned up some of the logic since a lot of it got moved into the
SORTABLE_MAPPING hash.
---
 lib/admin_user_index_query.rb                 | 41 ++++++++-----------
 .../components/admin_user_index_query_spec.rb |  8 ++--
 2 files changed, 22 insertions(+), 27 deletions(-)

diff --git a/lib/admin_user_index_query.rb b/lib/admin_user_index_query.rb
index 35a281015f7..8279ad20170 100644
--- a/lib/admin_user_index_query.rb
+++ b/lib/admin_user_index_query.rb
@@ -10,6 +10,20 @@ class AdminUserIndexQuery
 
   attr_reader :params, :trust_levels
 
+  SORTABLE_MAPPING = {
+    'created' => 'created_at',
+    'last_emailed' => "COALESCE(last_emailed_at, to_date('1970-01-01', 'YYYY-MM-DD'))",
+    'seen' => "COALESCE(last_seen_at, to_date('1970-01-01', 'YYYY-MM-DD'))",
+    'username' => 'username',
+    'email' => 'email',
+    'trust_level' => 'trust_level',
+    'days_visited' => 'user_stats.days_visited',
+    'posts_read' => 'user_stats.posts_read_count',
+    'topics_viewed' => 'user_stats.topics_entered',
+    'posts' => 'user_stats.post_count',
+    'read_time' => 'user_stats.time_read'
+  }
+
   def find_users(limit=100)
     find_users_query.limit(limit)
   end
@@ -18,42 +32,23 @@ class AdminUserIndexQuery
     find_users_query.count
   end
 
-  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[:ascending]
     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)
     order = []
 
     custom_order = params[:order]
     if custom_order.present? &&
-      without_dir = custom_order.downcase.sub(/ (asc|desc)$/, '')
-      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
+      without_dir = SORTABLE_MAPPING[custom_order.downcase.sub(/ (asc|desc)$/, '')]
+      order << "#{without_dir} #{custom_direction}"
     end
-    
+
     if !custom_order.present?
       if params[:query] == "active"
-        order << "#{pg_coalesce("last_seen_at")} DESC"
+        order << "COALESCE(last_seen_at, to_date('1970-01-01', 'YYYY-MM-DD')) DESC"
       else
         order << "users.created_at DESC"
       end
diff --git a/spec/components/admin_user_index_query_spec.rb b/spec/components/admin_user_index_query_spec.rb
index f3e3b814c89..e5c71957084 100644
--- a/spec/components/admin_user_index_query_spec.rb
+++ b/spec/components/admin_user_index_query_spec.rb
@@ -33,12 +33,12 @@ describe AdminUserIndexQuery do
     end
 
     it "allows custom ordering for stats wtih default direction" do
-      query = ::AdminUserIndexQuery.new({ order: "topics_entered" })
+      query = ::AdminUserIndexQuery.new({ order: "topics_viewed" })
       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", ascending: true })
+      query = ::AdminUserIndexQuery.new({ order: "topics_viewed", ascending: true })
       expect(query.find_users_query.to_sql).to match("topics_entered ASC")
     end
   end
@@ -91,7 +91,7 @@ describe AdminUserIndexQuery do
     end
 
     it "shows nil values first with asc" do
-      users = ::AdminUserIndexQuery.new({ order: "last_emailed_at", ascending: true }).find_users
+      users = ::AdminUserIndexQuery.new({ order: "last_emailed", ascending: true }).find_users
 
       expect(users.count).to eq(2)
       expect(users.first.username).to eq("system")
@@ -99,7 +99,7 @@ describe AdminUserIndexQuery do
     end
 
     it "shows nil values last with desc" do
-      users = ::AdminUserIndexQuery.new({ order: "last_emailed_at"}).find_users
+      users = ::AdminUserIndexQuery.new({ order: "last_emailed"}).find_users
 
       expect(users.count).to eq(2)
       expect(users.first.last_emailed_at).to_not eq(nil)