refactor constraints rendering for easier hooks

[MrDys]

CODEBASE-235: Plugins (or occaionally local code) may wish to modify constraints rendering on the results page for added functionality. Examples are the advanced search plugin, and the range_limit plugin.

The catalog/_constraints partial is used to display constraints. The initial (before this patch) partial has kind of a monolithic in-line rendering of:

1. The header information (magnifying glass, "you searched for")
2. Actual constraints themselves
3. For results page, the "showing X of N" message. (although oddly not the actual pagination, but that's another issue)

In this patch, 2 is replaced in _constraints with a call to new helper method: render_constraints(params)

render constraints calls two other methods:

def render_constraints(my_params = params) render_constraints_query(my_params) + render_constraints_filters(my_params) end

The point of this is that all three methods are now available for plugins to over-ride to add additional content before and/or after that block (call super in your over-ride), or replace that block entirely with custom code (do not call super). This way the advanced search plugin could (if desired) completely replace the render_constraints_query section; the range_filter section can add another constraint on after super in an over-ridden render_constraints_filters, etc.

Additionally, the actual rendering of a single constraint (ie, "Title: Foo") is DRY'd out into the #render_constraint_element method. This both DRYs up the code, and provides a method that can be called by plugins to add on a new constraint in an application-specified rendering.

render_constraint_element calls the new partial template catalog/constraints_element . I took the opportunity to clean up the HTML a bit, there were a lot of extraneous spans in there that served no semantic purpose, were not targetted by CSS, and didn't need to be there.

render_constraints_query and #render_constraints_filter started out with their own partials, but after DRYing out constraints_element, the code was so simple that they didn't need a template, everything can be done in just a couple lines of helper method alone.

All of these helper methods are located in a new file/module, app/helpers/render_constraints_helper.rb. ApplicationHelper was getting awfully crowded, seemed good to start splitting things out. I thought that Rails would automatically include any helper in app/helpers, but apparently not when it's a plugin/engine, no problem just added a line to CatalogHelper "include RenderConstraintsHelper." I believe these functions are only needed in Catalog, but it could be included in ApplicationHelper instead if it's needed in multiple actions.

NOTE ON TESTS:

I would have really liked to spec the actual helper methods in RenderConstraintsHelper. These methods are the api 'hooks' that are called by Blacklight itself and by any plugins -- the important thing to spec is that these methods exist and return appropriate HTML, it doesn't matter if the implementation detail of exactly what partial they call (if any) changes.

However, rspec didn't let me do that. I tried for a while. But I couldn't create a spec that called helper methods but also actually rendered partials if the helper methods called render on partials.

So I spec'd out the _constraints_element view instead. Oh well.

All existing features still passed with this refactor, the actual HTML rendered has not changed (with the exception of some simplification which did not disturb the features).

[MrDys]

Original reporter: jrochkind

[MrDys]

jrochkind: committed