projectblacklight/blacklight — pull request #647 by jrochkind (2013-12-04 22:06:17+00:00)
Title: do not merge options into params

The #params_for_search method takes a rails (query) params hash, and transforms it by omitting certain keys (the :omit_keys option) and returning a hash.

The params for search hash takes an `options` hash argument; one optional key of the hash is `params`, to specify the query params, otherwise it takes it from the controller @params method. Another optional key of the hash is `omit_keys`

In my own local app, I add some custom additional options (such as `omit_facet_value`) to this method, by over-riding certain methods and calling super.

The current implementation is _merging_ the _options hash_ into the _params hash_. So you might end up with, say, `omit_keys` being considered a query param. That's wrong, the code is confusing two different hashes.

In fact, this won't cause a problem in most apps, because the code _deletes_ the two options hash keys it acts upon, so by the time it merges options into params options will be empty and it will be a no-op. But won't do anything useful either.

However, in my app, since I'm adding additional options keys that aren't deleted, then they get erroneously merged into params, then they get put into hidden fields by caller, then I end up with keys like `omit_facet_value` in my query params. Doh!

So this simply stops doing that. I'm unable to run tests locally due to bundler complaining (not sure what's differnet about my setup than anyone elses), so we'll see what Travis says I guess!