Advanced Search and Range Limit plugins override same methods (and squash each other)

[cbeer]

The advanced search and range limit plugins both override these methods (in order to display the constraints and search results) with conflicting overrides. Perhaps Blacklight needs to provide a way for plugins to configure parameter names, or otherwise?

  def query_has_constraints?(localized_params = params)
    !(localized_params[:q].blank? and localized_params[:f].blank?)
  end

  def has_search_parameters?
    !params[:q].blank? or !params[:f].blank? or !params[:search_field].blank?
  end

[jrochkind]

Can they both call super, and add on their custon logic to the super call? So it doesn't matter which comes first, and they both end up in play?

Something like:

def query_has_constraints?(localized_params = params)
   super && localized_params[:f].blank?
end

or whatever. That's just an example, not sure of the exact logic involved.

I'm surprised they don't call super already, it seems like when you're overriding a method like this, you should always call super then add your own on.

[cbeer]

Not with the way they're doing overrides. We'd have to alias_method and call it appropriately (and, maybe we should...)

[jrochkind]

Why can't it call super as in an ordinary method sub-class or module override? super isn't available for some reason? I don't understand. I guess I have to take a look at the code to understand. If I figure out a way that seems like it should work, should I P.R. it? To master?

If possible, it seems preferable to use standard ruby subclass/module override with super, rather than alias method chain.

[jrochkind]

I think maybe we established in IRC that it can call super.

One method already does, although maybe not appropriately: https://github.com/projectblacklight/blacklight_advanced_search/blob/master/lib/blacklight_advanced_search/render_constraints_override.rb?#L19

I think making these methods call super appropriately can make them work together, regardless of which ends up mixed in first.

[barmintor]

This is a backwards compatibility issue if it were to go forward, but also seems a little stale. I'm going to mark it 8.x pending additional feedback here.

[barmintor]

This should be verified prior to an 8.0 release, but I think the FilterField API work addresses this in modern Blacklight.