search

codeforamerica / ohana-web-search

A mobile-friendly website for finding human and social services in your community
http://ohana-web-search-demo.herokuapp.com
BSD 3-Clause "New" or "Revised" License
69 stars 74 forks source link

Search filter form is submitting unnecessary parameters and causing specs to fail #423

Closed monfresh closed 10 years ago

monfresh commented 10 years ago

Once in a while, Travis and/or local tests will fail because the app makes a request that was not captured in a cassette. See the latest Travis failure for example. It says that this request was not recognized:

http://ohana-api-test.herokuapp.com/api/search?action=index&controller=organizations&keyword=&location=&location-option=&location-option-input=&org-name-option=&org-name-option-input=&org_name=&service-area-option=&service_area=&utf8=%E2%9C%93

Note that the request includes unexpected parameters such as location-option, location-option-input, org-name-option, org-name-option-input, and service-area-option. The culprit seems to be search_filter_manager.js.

Why is JS involved in submitting the form, and why is it submitting these additional parameters?

monfresh commented 10 years ago

The bug is actually in the filter partial. The name attribute of the input fields should correspond to the search parameter that the API expects. All of the input fields have a name set to "#{css_field}-option", when they should just be set to the field name that was passed in to the partial from _options.html.haml.

The various filter-related methods in session_helpers, and the specs that use them should be updated as well.

It looks like this bug was originally introduced in an attempt to "DRY" up the filter-related tests, so that you could use the same variable for the field name and the fieldset, which won't work if you want the fieldset name to only contain dashes, because some field names have to have an underscore, like org_name and service_area.

monfresh commented 10 years ago

I think I finally figured it out. This issue was caused by various search-related bugs, which I fixed in this commit in SMC-Connect. The build passed on the first try on Travis, which is a good sign.

I'll fix it here as well and send a PR tomorrow.

anselmbradford commented 10 years ago

You may want to reference https://github.com/smcgov/SMC-Connect/compare/master...sync-ohana-web-search as the second form was involved in the gelocation button, which has been re-worked to be included in the one form. I can submit a PR for https://github.com/smcgov/SMC-Connect/tree/sync-ohana-web-search if that would be helpful? I thought I would run through your instructions in https://github.com/codeforamerica/ohana-web-search/issues/367, but https://github.com/smcgov/SMC-Connect/tree/sync-ohana-web-search is good to be reviewed, so I could go through the instructions on the next syncing. Also, to answer your earlier question the JS is used to submit the form because it cleans up and consolidates the form input names, since there are radio buttons and an input field, the names for both these inputs get submitted, leading to the extra URL parameters.