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..1dfeeff2d1d 100644 --- a/lib/admin_user_index_query.rb +++ b/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 diff --git a/spec/components/admin_user_index_query_spec.rb b/spec/components/admin_user_index_query_spec.rb index a4a7e48abf2..6f935a8e3c5 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", 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) }