Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve AdminSearchController#find_students. #17018

Merged
merged 2 commits into from Aug 12, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 12 additions & 5 deletions dashboard/app/controllers/admin_search_controller.rb
Expand Up @@ -17,7 +17,7 @@ def find_students
users = users.where("name LIKE ?", "%#{params[:studentNameFilter]}%")
end
if params[:studentEmailFilter].present?
hashed_email = Digest::MD5.hexdigest(params[:studentEmailFilter])
hashed_email = User.hash_email params[:studentEmailFilter]
users = users.where(hashed_email: hashed_email)
end
if params[:teacherNameFilter].present? || params[:teacherEmailFilter].present?
Expand All @@ -26,17 +26,24 @@ def find_students
where("email LIKE ?", "%#{params[:teacherEmailFilter]}%").
all
if teachers.count > 1
# TODO(asher): Display a warning to the admin that multiple teachers
# matched.
flash[:alert] = 'Multiple teachers matched the name and email search criteria.'
end
if teachers.first
array_of_student_ids = teachers.first.students.pluck(:id)
users = users.where(id: array_of_student_ids)
end
end
if params[:sectionFilter].present?
array_of_student_ids = Section.find_by_code(params[:sectionFilter]).students.pluck(:id)
users = users.where(id: array_of_student_ids)
section = Section.with_deleted.find_by_code params[:sectionFilter]
if section.nil?
flash[:alert] = 'Section not found.'
elsif section.deleted?
flash[:alert] = 'Section is deleted.'
end
if section
array_of_student_ids = section.students.pluck(:id)
users = users.where(id: array_of_student_ids)
end
end

@users = users.page(params[:page]).per(MAX_PAGE_SIZE)
Expand Down
2 changes: 1 addition & 1 deletion dashboard/app/views/admin_search/find_students.html.haml
Expand Up @@ -43,7 +43,7 @@
= paginate @users
%table.users
%tr
- ['ID', 'Name', 'Email', 'Deleted At', 'Provider', 'Undelete'].each do |field|
- ['ID', 'Name', 'Email', 'Provider', 'Deleted At', 'Undelete'].each do |field|
%th
%span= field
- @users.each do |user|
Expand Down
34 changes: 34 additions & 0 deletions dashboard/test/controllers/admin_search_controller_test.rb
Expand Up @@ -17,6 +17,40 @@ class AdminSearchControllerTest < ActionController::TestCase
generate_admin_only_tests_for :find_students
generate_admin_only_tests_for :lookup_section

#
# find_student tests
#

test 'find_students flashes warning if multiple teachers match' do
create_list :teacher, 2, name: 'multiple'

get :find_students, params: {teacherNameFilter: 'multiple'}

assert_response :success
assert_select '.container .alert-danger', 'Multiple teachers matched the name and email search criteria.'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that I don't understand why the assert_equal message, flash[:alert] syntax fails.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intersting also that it's (presumably) working for the undelete_section tests but not for the lookup_section test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I noticed. Honestly, I didn't spend almost any time trying to understand, it didn't seem worth it.

end

test 'find_students flashes warning for nil section' do
get :find_students, params: {sectionFilter: 'AAAAAA'}

assert_response :success
assert_select '.container .alert-danger', 'Section not found.'
end

test 'find_students flashes warning for deleted section' do
section = create :section
section.destroy

get :find_students, params: {sectionFilter: section.code}

assert_response :success
assert_select '.container .alert-danger', 'Section is deleted.'
end

#
# undelete_section tests
#

test "undelete_section is admin only" do
sign_in(@not_admin)
post :undelete_section, params: {section_code: @teacher_section.code}
Expand Down