From e0ff74ead0e70b103d6a0a30b822cff3db7ff216 Mon Sep 17 00:00:00 2001 From: Jesse House Date: Wed, 19 Jun 2013 09:11:04 -0700 Subject: [PATCH] extract Admin::UsersController#index to its own query class - move query to its own class - use postgres ILIKE case insensitive - removed duplicated list of trust levels --- app/controllers/admin/users_controller.rb | 21 +--- lib/admin_user_index_query.rb | 55 +++++++++ .../components/admin_user_index_query_spec.rb | 114 ++++++++++++++++++ 3 files changed, 172 insertions(+), 18 deletions(-) create mode 100644 lib/admin_user_index_query.rb create mode 100644 spec/components/admin_user_index_query_spec.rb diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 2064c9d3967..c579ba8bca8 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -1,28 +1,13 @@ require_dependency 'user_destroyer' +require_dependency 'admin_user_index_query' class Admin::UsersController < Admin::AdminController before_filter :fetch_user, only: [:ban, :unban, :refresh_browsers, :revoke_admin, :grant_admin, :revoke_moderation, :grant_moderation, :approve, :activate, :deactivate, :block, :unblock] def index - # Sort order - if params[:query] == "active" - @users = User.order("COALESCE(last_seen_at, to_date('1970-01-01', 'YYYY-MM-DD')) DESC, username") - else - @users = User.order("created_at DESC, username") - end - - if ['newuser', 'basic', 'regular', 'leader', 'elder'].include?(params[:query]) - @users = @users.where('trust_level = ?', TrustLevel.levels[params[:query].to_sym]) - end - - @users = @users.where('admin = ?', true) if params[:query] == 'admins' - @users = @users.where('moderator = ?', true) if params[:query] == 'moderators' - @users = @users.blocked if params[:query] == 'blocked' - @users = @users.where('approved = false') if params[:query] == 'pending' - @users = @users.where('username_lower like :filter or email like :filter', filter: "%#{params[:filter].downcase}%") if params[:filter].present? - @users = @users.take(100) - render_serialized(@users, AdminUserSerializer) + query = ::AdminUserIndexQuery.new(params) + render_serialized(query.find_users, AdminUserSerializer) end def show diff --git a/lib/admin_user_index_query.rb b/lib/admin_user_index_query.rb new file mode 100644 index 00000000000..c52f190f946 --- /dev/null +++ b/lib/admin_user_index_query.rb @@ -0,0 +1,55 @@ +class AdminUserIndexQuery + def initialize(params = {}, klass = User, trust_levels = TrustLevel.levels) + @params = params + @query = initialize_query_with_order(klass) + @trust_levels = trust_levels + end + + attr_reader :params, :trust_levels + + def initialize_query_with_order(klass) + if params[:query] == "active" + klass.order("COALESCE(last_seen_at, to_date('1970-01-01', 'YYYY-MM-DD')) DESC, username") + else + klass.order("created_at DESC, username") + end + end + + def filter_by_trust + levels = trust_levels.map { |key, value| key.to_s } + if levels.include?(params[:query]) + @query.where('trust_level = ?', trust_levels[params[:query].to_sym]) + end + end + + def filter_by_query_classification + case params[:query] + when 'admins' then @query.where('admin = ?', true) + when 'moderators' then @query.where('moderator = ?', true) + when 'blocked' then @query.blocked + when 'pending' then @query.where('approved = false') + end + end + + def filter_by_search + if params[:filter].present? + @query.where('username_lower ILIKE :filter or email ILIKE :filter', filter: "%#{params[:filter]}%") + end + end + + # this might not be needed in rails 4 ? + def append(active_relation) + @query = active_relation if active_relation + end + + def find_users_query + append filter_by_trust + append filter_by_query_classification + append filter_by_search + @query + end + + def find_users + find_users_query.take(100) + end +end \ No newline at end of file diff --git a/spec/components/admin_user_index_query_spec.rb b/spec/components/admin_user_index_query_spec.rb new file mode 100644 index 00000000000..01985156f76 --- /dev/null +++ b/spec/components/admin_user_index_query_spec.rb @@ -0,0 +1,114 @@ +require 'spec_helper' +require_dependency 'admin_user_index_query' + +describe AdminUserIndexQuery do + describe "sql order" do + it "has default" do + query = ::AdminUserIndexQuery.new({}) + expect(query.find_users_query.to_sql).to match("created_at DESC") + end + + it "has active order" do + query = ::AdminUserIndexQuery.new({ query: "active" }) + expect(query.find_users_query.to_sql).to match("last_seen_at") + end + end + + describe "no users with trust level" do + + TrustLevel.levels.each do |key, value| + it "#{key} returns no records" do + query = ::AdminUserIndexQuery.new({ query: key.to_s }) + expect(query.find_users.count).to eq(0) + end + end + + end + + describe "users with trust level" do + + TrustLevel.levels.each do |key, value| + it "finds user with trust #{key}" do + Fabricate(:user, trust_level: TrustLevel.levels[key]) + query = ::AdminUserIndexQuery.new({ query: key.to_s }) + expect(query.find_users.count).to eq(1) + end + end + + end + + describe "with a pending user" do + + let!(:user) { Fabricate(:user, approved: false) } + + it "finds the unapproved user" do + query = ::AdminUserIndexQuery.new({ query: 'pending' }) + expect(query.find_users.count).to eq(1) + end + + end + + describe "with an admin user" do + + let!(:user) { Fabricate(:user, admin: true) } + + it "finds the admin" do + query = ::AdminUserIndexQuery.new({ query: 'admins' }) + expect(query.find_users.count).to eq(1) + end + + end + + describe "with a moderator" do + + let!(:user) { Fabricate(:user, moderator: true) } + + it "finds the moderator" do + query = ::AdminUserIndexQuery.new({ query: 'moderators' }) + expect(query.find_users.count).to eq(1) + end + + end + + describe "with a blocked user" do + + let!(:user) { Fabricate(:user, blocked: true) } + + it "finds the blocked user" do + query = ::AdminUserIndexQuery.new({ query: 'blocked' }) + expect(query.find_users.count).to eq(1) + end + + end + + describe "filtering" do + context "by email fragment" do + before(:each) { Fabricate(:user, email: "test1@example.com") } + + it "matches the email" do + query = ::AdminUserIndexQuery.new({ filter: "est1" }) + expect(query.find_users.count).to eq(1) + end + + it "matches the email using any case" do + query = ::AdminUserIndexQuery.new({ filter: "Test1" }) + expect(query.find_users.count).to eq(1) + end + end + + context "by username fragment" do + before(:each) { Fabricate(:user, username: "test_user_1") } + + it "matches the username" do + query = ::AdminUserIndexQuery.new({ filter: "user" }) + expect(query.find_users.count).to eq(1) + end + + it "matches the username using any case" do + query = ::AdminUserIndexQuery.new({ filter: "User" }) + expect(query.find_users.count).to eq(1) + end + end + + end +end \ No newline at end of file