Merge pull request #1046 from house9/admin-user-index-2

extract Admin::UsersController#index to its own query class

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

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

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