fix role filtering in new account user search

use the code that already exists for enrollment role filtering

test plan:
* shouldn't include deleted enrollments when filtering
 by role in the new account user serach

closes #ADMIN-869

Change-Id: I8d32f1a54ad6791ec91540e4e8725abfade791ce
Reviewed-on: https://gerrit.instructure.com/144019
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Tested-by: Jenkins
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: James Williams  <jamesw@instructure.com>
This commit is contained in:
James Williams 2018-03-19 09:47:59 -06:00
parent a5825e3a59
commit b14891827d
2 changed files with 5 additions and 23 deletions

View File

@ -425,11 +425,11 @@ class UsersController < ApplicationController
page_opts = {} page_opts = {}
if search_term if search_term
users = UserSearch.for_user_in_context(search_term, @context, @current_user, session, users = UserSearch.for_user_in_context(search_term, @context, @current_user, session,
{order: params[:order], sort: params[:sort], role_filter_id: params[:role_filter_id]}) {order: params[:order], sort: params[:sort], enrollment_role_id: params[:role_filter_id]})
page_opts[:total_entries] = nil # doesn't calculate a total count page_opts[:total_entries] = nil # doesn't calculate a total count
else else
users = UserSearch.scope_for(@context, @current_user, users = UserSearch.scope_for(@context, @current_user,
{order: params[:order], sort: params[:sort], role_filter_id: params[:role_filter_id]}) {order: params[:order], sort: params[:sort], enrollment_role_id: params[:role_filter_id]})
end end
includes = (params[:include] || []) & %w{avatar_url email last_login time_zone} includes = (params[:include] || []) & %w{avatar_url email last_login time_zone}

View File

@ -44,13 +44,7 @@ module UserSearch
end end
context.shard.activate do context.shard.activate do
# TODO: Need to optimize this as it's not using the base scope filter for the conditions statement query # TODO: Need to optimize this as it's not using the base scope filter for the conditions statement query
base_scope = base_scope.where(conditions_statement(search_term, context.root_account, {restrict_search: restrict_search})) base_scope.where(conditions_statement(search_term, context.root_account, {restrict_search: restrict_search}))
if options[:role_filter_id] && options[:role_filter_id] != ""
base_scope = base_scope.where("#{options[:role_filter_id]} IN
(SELECT role_id FROM #{Enrollment.quoted_table_name}
WHERE #{Enrollment.quoted_table_name}.user_id = #{User.quoted_table_name}.id)")
end
base_scope
end end
end end
@ -131,27 +125,15 @@ module UserSearch
users.order_by_sortable_name users.order_by_sortable_name
end end
if options[:role_filter_id] && options[:role_filter_id] != ""
users = users.where("#{options[:role_filter_id]} IN
(SELECT role_id FROM #{Enrollment.quoted_table_name}
WHERE #{Enrollment.quoted_table_name}.user_id = #{User.quoted_table_name}.id)")
end
if enrollment_role_ids || enrollment_roles if enrollment_role_ids || enrollment_roles
users = users.joins(:not_removed_enrollments).distinct if context.is_a?(Account)
roles = if enrollment_role_ids roles = if enrollment_role_ids
enrollment_role_ids.map{|id| Role.get_role_by_id(id)}.compact enrollment_role_ids.map{|id| Role.get_role_by_id(id)}.compact
else else
enrollment_roles.map{|name| context.is_a?(Account) ? context.get_course_role_by_name(name) : enrollment_roles.map{|name| context.is_a?(Account) ? context.get_course_role_by_name(name) :
context.account.get_course_role_by_name(name)}.compact context.account.get_course_role_by_name(name)}.compact
end end
conditions_sql = "role_id IN (?)" users = users.where("role_id IN (?)", roles.map(&:id))
# TODO: this can go away after we take out the enrollment role shim (after role_id has been populated)
roles.each do |role|
if role.built_in?
conditions_sql += " OR (role_id IS NULL AND type = #{User.connection.quote(role.name)})"
end
end
users = users.where(conditions_sql, roles.map(&:id))
elsif enrollment_types elsif enrollment_types
enrollment_types = enrollment_types.map { |e| "#{e.camelize}Enrollment" } enrollment_types = enrollment_types.map { |e| "#{e.camelize}Enrollment" }
if enrollment_types.any?{ |et| !Enrollment.readable_types.keys.include?(et) } if enrollment_types.any?{ |et| !Enrollment.readable_types.keys.include?(et) }