Added ID to admin user search; removed Make user admin; removed Make …

…user admin API; Improved flaky spec

app/controllers/admin/users_controller.rb

@@ -1,7 +1,7 @@
 module Admin
   class UsersController < BaseController
 
-    before_filter :get_user, only: [:edit, :update, :destroy, :become, :make_admin]
+    before_filter :get_user, only: [:edit, :update, :destroy, :become]
 
     def index
       security_log :users_searched_by_admin, search: params[:search]
@@ -41,16 +41,7 @@ def become
       redirect_to request.referrer
     end
 
-    def make_admin
-      if @user.update_attribute(:is_administrator, true)
-        security_log :admin_created, user_id: params[:id], username: @user.username
-        redirect_to request.referrer, notice: 'User successfully updated.'
-      else
-        redirect_to request.referrer, alert: 'Unable to update user.'
-      end
-    end
-
-  protected
+    protected
 
     def get_user
       @user = User.find(params[:id])

app/routines/admin/search_users.rb

@@ -33,7 +33,7 @@ class SearchUsers
     protected
 
     def exec(query, options={})
-      options = options.merge({:return_all => true})
+      options = options.merge(return_all: true)
       run(:search_users, query, options)
 
       per_page = options[:per_page] || 20
@@ -45,4 +45,4 @@ def exec(query, options={})
     end
 
   end
-end
+end

app/routines/search_users.rb

@@ -44,39 +44,44 @@ def exec(query, options={})
       with.default_keyword :any
 
       with.keyword :username do |usernames|
-        users = users.where{lower(username).like_any my{prep_names(usernames)}}
+        users = users.where{ lower(username).like_any my{ prep_names(usernames) } }
       end
 
       with.keyword :first_name do |first_names|
-        users = users.where{lower(first_name).like_any my{prep_names(first_names)}}
+        users = users.where{ lower(first_name).like_any my{ prep_names(first_names) } }
       end
 
       with.keyword :last_name do |last_names|
-        users = users.where{lower(last_name).like_any my{prep_names(last_names)}}
+        users = users.where{ lower(last_name).like_any my{ prep_names(last_names) } }
       end
 
       with.keyword :full_name do |names|
         names = prep_names(names)
-        users = users.where{ (lower(first_name).op('||', ' ').op('||', lower(last_name)).like_any names) }
+
+        users = users.where do
+          lower(first_name).op('||', ' ').op('||', lower(last_name)).like_any names
+        end
       end
 
       with.keyword :name do |names|
         names = prep_names(names)
-        users = users.where{ (lower(first_name).op('||', ' ').op('||', lower(last_name)).like_any names) |
-                             (lower(first_name).like_any names) |
-                             (lower(last_name).like_any names) }
+
+        users = users.where do
+          lower(first_name).op('||', ' ').op('||', lower(last_name)).like_any(names) |
+                                                   lower(first_name).like_any(names) |
+                                                    lower(last_name).like_any(names)
+        end
       end
 
       with.keyword :id do |ids|
-        users = users.where{id.in ids}
+        users = users.where(id: ids)
       end
 
       with.keyword :email do |emails|
-        users = users.joins{contact_infos}
-                     .where(contact_infos: {type: 'EmailAddress',
+        users = users.joins(:contact_infos).where(contact_infos: {value: emails})
+        users = users.where(contact_infos: {type: 'EmailAddress',
                                             verified: true,
-                                            is_searchable: true})
-                     .where{contact_infos.value.in emails}
+                                            is_searchable: true}) unless options[:return_all]
       end
 
       # Rerun the queries above for 'any' terms (which are ones without a
@@ -85,18 +90,19 @@ def exec(query, options={})
       with.keyword :any do |terms|
         names = prep_names(terms)
 
-        users = users.joins{contact_infos.outer}.where{
-                  (                     username.like_any names)           | \
-                  (            lower(first_name).like_any names)           | \
-                  (             lower(last_name).like_any names)           | \
-                  (lower(first_name)
-                     .op('||', ' ')
-                     .op('||', lower(last_name)).like_any names)           | \
-                  (                           id.in       terms)           | \
-                  ((         contact_infos.value.in       terms)           & \
-                  (           contact_infos.type.eq       'EmailAddress')  & \
-                  (       contact_infos.verified.eq       true)            & \
-                  (  contact_infos.is_searchable.eq       true))}
+        users = users.joins{contact_infos.outer}.where do
+          contact_infos_query = contact_infos.value.in terms
+          contact_infos_query &= (contact_infos.type.eq('EmailAddress') &
+                                  contact_infos.verified.eq(true) &
+                                  contact_infos.is_searchable.eq(true)) unless options[:return_all]
+
+                                                            username.like_any(names) |
+                                                   lower(first_name).like_any(names) |
+                                                    lower(last_name).like_any(names) |
+          lower(first_name).op('||', ' ').op('||', lower(last_name)).like_any(names) |
+                                                                        id.in(terms) |
+                                                                 contact_infos_query
+        end
       end
 
     end
@@ -108,10 +114,10 @@ def exec(query, options={})
     # Ordering
 
     # Parse the input
-    order_bys = (options[:order_by] || 'username').split(',').collect{|ob| ob.strip.split(' ')}
+    order_bys = (options[:order_by] || 'username').split(',').map{ |ob| ob.strip.split(' ') }
 
     # Toss out bad input, provide default direction
-    order_bys = order_bys.collect do |order_by|
+    order_bys = order_bys.map do |order_by|
       field, direction = order_by
       next if !SORTABLE_FIELDS.include?(field)
       direction ||= SORT_ASCENDING
@@ -125,7 +131,7 @@ def exec(query, options={})
     order_bys = ['username', SORT_ASCENDING] if order_bys.empty?
 
     # Convert to query style
-    order_bys = order_bys.collect{|order_by| "#{order_by[0]} #{order_by[1]}"}
+    order_bys = order_bys.map{|order_by| "#{order_by[0]} #{order_by[1]}"}
 
     order_bys.each do |order_by|
       # postgres requires the "users." bit to make it table_name.column_name
@@ -145,8 +151,7 @@ def exec(query, options={})
 
     # Return no results if maximum number of results is exceeded
 
-    outputs[:items] = (outputs[:total_count] > MAX_MATCHING_USERS) ?
-                        User.none : users
+    outputs[:items] = (outputs[:total_count] > MAX_MATCHING_USERS) ? User.none : users
 
   end
 

app/views/admin/users/search.js.erb

@@ -7,27 +7,18 @@
   contents = osu.action_list(
     records: users,
     list: {
-      headings: ['Username', 'First Name', 'Last Name', 'Is Admin?', 'Actions'],
-      widths: ['20%', '20%', '20%', '20%', '20%'],
+      headings: ['ID', 'Username', 'First Name', 'Last Name', 'Is Admin?', 'Actions'],
+      widths: ['10%', '20%', '20%', '20%', '10%', '20%'],
       data_procs:
         [
+          Proc.new { |user| user.id.to_s },
           Proc.new do |user|
             security_log_params = { query: "user:\"#{user.username}\"" }
             link_to user.username, admin_security_log_path(search: security_log_params)
           end,
           Proc.new { |user| user.first_name || '---' },
           Proc.new { |user| user.last_name || '---' },
-          Proc.new { |user|
-            if user.is_administrator
-              'Yes'
-            else
-              link_to 'Make them admin',
-                      make_admin_admin_user_path(user),
-                      method: :post,
-                      remote: true,
-                      class: 'make-admin'
-            end
-          },
+          Proc.new { |user| user.is_administrator ? 'Yes' : 'No' },
           Proc.new { |user|
             sign_in_link = link_to 'Sign in as',
                                    become_admin_user_path(user),

config/routes.rb

@@ -142,7 +142,6 @@
 
     resources :users, only: [:index, :update, :edit] do
       post 'become', on: :member
-      post 'make_admin', on: :member
     end
 
     resource :security_log, only: [:show]

spec/routines/admin/search_security_log_spec.rb

@@ -3,14 +3,14 @@
 describe Admin::SearchSecurityLog, type: :routine do
 
   let!(:user)            { FactoryGirl.create :user, first_name: 'Test', last_name: 'User' }
-  let!(:app)             { FactoryGirl.create :doorkeeper_application }
+  let!(:app)             { FactoryGirl.create :doorkeeper_application, name: 'Some Test App' }
 
   let!(:anon_sl)         { FactoryGirl.create :security_log, user: nil }
   let!(:user_sl)         { FactoryGirl.create :security_log, user: user }
   let!(:app_sl)          { FactoryGirl.create :security_log, user: nil, application: app }
   let!(:app_and_user_sl) { FactoryGirl.create :security_log, user: user, application: app }
 
-  let!(:another_app)     { FactoryGirl.create :doorkeeper_application }
+  let!(:another_app)     { FactoryGirl.create :doorkeeper_application, name: 'Another Test App' }
   let!(:ip_sl)           { FactoryGirl.create :security_log, application: another_app,
                                                              remote_ip: '192.168.0.1' }
   let!(:type_sl)         { FactoryGirl.create :security_log, application: another_app,