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

Conversation

ashercodeorg
Copy link
Contributor

@ashercodeorg ashercodeorg commented Aug 11, 2017

This makes several improvements and fixes:

  1. Fixes the ordering of the column headers in the view to match the data.
  2. Resolves a TODO to show a warning if multiple teachers match.
  3. Displays an alert to the admin if the section does not exist or is deleted.
    In doing so, resolves this HB error (namely NoMethodError: undefined method 'students' for nil:NilClass) happening when the searched section does not exist.

EXAMPLE WARNING (section not found):

image

@ashercodeorg
Copy link
Contributor Author

FYI @Hamms.

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.

@ashercodeorg
Copy link
Contributor Author

@Hamms: This LG?

@ashercodeorg ashercodeorg merged commit caef51b into staging Aug 12, 2017
@ashercodeorg ashercodeorg deleted the fixFindStudents branch August 12, 2017 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants