Construct searches using the search builder directly from the controller.

[cbeer]

I'm not there's advantage to having Blacklight::SearchService construct the search builder internally (other than it probably made sense at the time, before we encapsulated the params with SearchState).

If we hoist the search builder up to the controller (via the Searchable concern), maybe there's less magic?

Bypassing SearchService#facet_field_response and #facet_suggest_response are potentially backwards-incompatible, but I couldn't find any indication that it's a method people are actually overriding downstream.

Includes #3856, #3843, and #3854

[barmintor]

@jrochkind observes in Slack:
"SearchBuilder has had more or less the same API for a long time, and a lot of things in local apps and dependent *-lights depend on it, so beware there are downsides to changing it in backwards incompatible ways"

ArcLight uses a mixin that does not override the initializer.

Spotlight uses two: an access control mixin that does not override the initializer, and a category browse mixin that also does not.

GeoBlacklight likewise uses a mixin without overriding the initializer.

All of these expect SearchBuilder.default_processor_chain to be an appendable list of symbols identifying methods to be added to the processing chain for params. None of them directly instantiate a SearchBuilder outside their specs, but all of them instantiate a SearchBuilder without the blacklight_config kwarg in the specs for their mixins.

[barmintor]

ArcLight doesn't appear to interact with SearchService at all.
GeoBlacklight references SearchService in component spec.
Spotlight has a helper method that reproduces the logic from Blacklight::Searchable.search_service, but I only see references to it in Spotlight controller mixins.

[cbeer]

@barmintor : I think you're observing that this change is backwards-compatible (and/or having done some due diligence in some complicated apps to assure that, even if it was in some way, it doesn't impact real-world use)?

[barmintor]

@cbeer that's right - per Jonathan's concern, I think things are mostly ok, but there is a spec in GeoBlacklight that will fail and apps that use the Spotlight helper might have an issue (though I think it may be a vestigial method).

[jcoyne]

Should we change this kwarg to builder since we're always passing a SearchBuilder here?

[cbeer]

I'm just matching the argument name to the repository for consistency.

[jcoyne]

This conflates search results (documents) from facet results. Can we keep these two concerns separated?

[cbeer]

Is there any value in doing that? I find the current state incredibly confusing and it's not clear to me what behavior the seam is intending to provide that isn't better suited for the search builder.

[jcoyne]

I think that it is confusing, and having one method handle the one case makes it less so.