Merge pull request #17018 from code-dot-org/fixFindStudents

Improve AdminSearchController#find_students.

dashboard/app/controllers/admin_search_controller.rb

@@ -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?
@@ -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)

dashboard/app/views/admin_search/find_students.html.haml

@@ -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|

dashboard/test/controllers/admin_search_controller_test.rb

@@ -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.'
+  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}