Created a search view with a contextual filter and added some sort fields for the view. Placed the sort block on the view, but the sort dropdown wouldnt display. When i debugged, the following condition in the build function in Drupal\search_api_sorts\Plugin\Block\SearchApiSortsBlock is failing.

if (!$search_api_display->isRenderedInCurrentRequest()) {
//Display is not rendered in current request, hide block.
return [];
}

The isRenderedInCurrentRequest function compares the current path which is for example /myview/1 to the plugin definition for the view which is /myview/%param and the condition fails there. Commenting out the return causes the sort dropdown to display properly.

The condition needs tweaking to correctly accommodate contextual filters.

Thanks
Sukanya

Comments

sukanya.ramakrishnan created an issue. See original summary.

strykaizer’s picture

Project: Search API sorts » Search API
Component: Code » General code

This looks like a search api issue, assigning accordingly

borisson_’s picture

strykaizer’s picture

Assigned: Unassigned » strykaizer
Issue tags: +dcnlights
strykaizer’s picture

Status: Active » Needs review
StatusFileSize
new808 bytes

Check on routename instead of path

strykaizer’s picture

Assigned: strykaizer » Unassigned
borisson_’s picture

Looks great, same reservations as in #2855758: Search api display for blocks always returns false in isRenderedInCurrentRequest. Not sure if this needs a test, if it doesn't let's get this in.

strykaizer’s picture

Edit: comment removed

borisson_’s picture

Status: Needs review » Reviewed & tested by the community

I had the same question a couple of times in facets as well already. Let's fix this in search api.

boobaa’s picture

Assigned: Unassigned » boobaa
Status: Reviewed & tested by the community » Needs work

As this could be done in an object-oriented fashion, we shouldn't introduce procedural code, I guess. IOW: Let's get it done the same way as the path.

boobaa’s picture

Assigned: boobaa » Unassigned
Status: Needs work » Needs review
StatusFileSize
new2.75 KB
strykaizer’s picture

Status: Needs review » Needs work

Thanks Boobaa,

One thing we should change. This is now implemented in the base class (displaypluginbase), but it should move to the views plugin version instead (ViewsDisplayBase), since we hardcode a views plugin id to match the route here.

This way, search api pages and possible other implementations can provide their own checks.

borisson_’s picture

In addition to that, I think we should remove the implementation of that method in DisplayPluginBase, so that everyone that introduces such a plugin is 100% sure that their implementation works for their use case.

PS: Next time, please provide an interdiff as well as a patch to make reviewing easier.

drunken monkey’s picture

Component: General code » Plugins
Status: Needs work » Needs review
StatusFileSize
new4.77 KB
new2.21 KB

That all makes sense to me, thanks for reporting, and for the work here so far!
So would everyone be fine with the attached patch?

The base implementation should be fine for most use cases, so I wouldn't remove that. People should make sure the base implementations work for their plugins in all cases anyways, nothing special here, as far as I can see.

borisson_’s picture

Status: Needs review » Reviewed & tested by the community

Sure, your explanation here makes a ton of sense. Thanks!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 14: 2842557-14--views_displays_route_matching.patch, failed testing.

drunken monkey’s picture

Status: Needs work » Fixed

Good to hear, thanks for your input!
Committed.
Thanks again to everyone here for your work on this!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.