In #2859139: Automatically disable caching of views facet sources (regression) we noticed that there's code duplication in the module, we do something like this a couple of times. And we'll probably do this more if we want to handle views-specific stuff.
Because of this, it's may be a good idea to add a getViewsDisplay method to the \Drupal\facets\FacetSource\SearchApiFacetSourceInterface
class.
Because we use this class also for integration with the search_api_pages modules we have to find a way to do this that allows us to return FALSE or something similar from that method.
$facet_source_display_id = $facet_source->getPluginDefinition()['display_id'];
$search_api_display = \Drupal::service('plugin.manager.search_api.display')
->createInstance($facet_source_display_id);
$search_api_display_definition = $search_api_display->getPluginDefinition();
if (!empty($search_api_display_definition['view_id'])) {
$view_id = $search_api_display_definition['view_id'];
$view_display = $search_api_display_definition['view_display'];
$view = Views::getView($view_id);
$view->setDisplay($view_display);
}
Todo:
- Discuss if we want this.
- Implement in the interface
- Implement the code
- Overwrite the current usages of this code to the new code
- Write an additional test.
Comment | File | Size | Author |
---|---|---|---|
#11 | api_break_add-2860393-11.patch | 8.69 KB | borisson_ |
Comments
Comment #2
borisson_Doing this will break the API, so we should do this before we tag the first beta.
Comment #3
borisson_Comment #5
borisson_This is probably the 200th time I had this problem.
Comment #6
acbramley CreditAttribution: acbramley commentedNit:
I believe you could use ViewsDisplayBase::getView here instead so
$this->getDisplay()->getView()->getExecutable()
Other than that it looks good, I can't help but think this may be better suited on the ViewsDisplayBase class though.
Comment #7
borisson_You're right, on both points. Back to needs work, I'll fix those tonight.
Comment #8
borisson_Actually, moving this to the ViewsDisplayBase class is not possible, as that would be a bc break for search api, that's why I'd prefer to keep this here.
I fixed the other remark.
Comment #10
borisson_Reverted 8, can't use a protected method, making it public is an api break for search api.
Comment #11
borisson_Patch introduced coding standards issues. Thanks testbot!
Comment #12
acbramley CreditAttribution: acbramley commentedMakes sense.
Damn, that's a shame! Sorry I should've noticed that!
This is all good from my point of view then.
Comment #13
borisson_Committed and pushed, thanks for the review!