diff --git a/app/jobs/scheduled/pending_users_reminder.rb b/app/jobs/scheduled/pending_users_reminder.rb index 8069a573367..c2e392d65a4 100644 --- a/app/jobs/scheduled/pending_users_reminder.rb +++ b/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 diff --git a/lib/admin_user_index_query.rb b/lib/admin_user_index_query.rb index 3c2eeec9d31..8279ad20170 100644 --- a/lib/admin_user_index_query.rb +++ b/lib/admin_user_index_query.rb @@ -10,16 +10,31 @@ 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.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 custom_direction + asc = params[:ascending] + asc.present? && asc ? "ASC" : "DESC" end def initialize_query_with_order(klass) @@ -27,21 +42,25 @@ 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 + 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 << "COALESCE(last_seen_at, to_date('1970-01-01', 'YYYY-MM-DD')) DESC" + else + order << "users.created_at DESC" end + + order << "users.username" end - if params[:query] == "active" - order << "COALESCE(last_seen_at, to_date('1970-01-01', 'YYYY-MM-DD')) DESC" + if params[:stats].present? && params[:stats] == false + klass.order(order.reject(&:blank?).join(",")) else - order << "users.created_at DESC" + klass.includes(:user_stat).order(order.reject(&:blank?).join(",")) end - - order << "users.username" - - klass.order(order.reject(&:blank?).join(",")) end def filter_by_trust diff --git a/spec/components/admin_user_index_query_spec.rb b/spec/components/admin_user_index_query_spec.rb index a4a7e48abf2..e5c71957084 100644 --- a/spec/components/admin_user_index_query_spec.rb +++ b/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", ascending: 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_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_viewed", ascending: 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", ascending: 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"}).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) }