Problem/Motivation

I updated my site (drupal 9) from search_api 1.28 to 1.29 and think I discovered a bug relating to the changes in this issue: https://www.drupal.org/project/search_api/issues/3260862, specifically this line: https://git.drupalcode.org/project/search_api/-/commit/d80778b51e772572d....

My commerce catalog is created using the SearchApiAllTerms contextual filter and rendering the view on a taxonomy term page to show all products that belong to it. After updating, the view just shows EVERY product regardless of the taxonomy term you are viewing. The problem appears to be that the condition is never actually added to the query.

Steps to reproduce

  • Create a search api view.
  • Add the SearchApiAllTerms conextual filter
  • Enter an ID for the contextual filter in the preview area
  • The results are not filtered

Proposed resolution

  • Ensure the query condition is added to the query object in the SearchApiAllTerms.
  • Investigate whether other views plugins have the same issue.

Comments

ericchew created an issue. See original summary.

peterjlord’s picture

Hi there,

I seem to be having this issue as well. After (drupal 9) upgrading from search_api 1.28 to 1.29. I've reverted back to 1.28 until there is a patch. I may try and investigate.

Thanks

ericchew’s picture

Status: Active » Needs review
StatusFileSize
new728 bytes

Here is a patch to fix it.

I'm not sure if there is a good way to fix the test so that it catches this issue. It is a Unit Test that mocks the SearchApiQuery object and the test is checking mock objects instead of getting the conditions from the actual query (where the conditions were not present).

https://git.drupalcode.org/project/search_api/-/blob/8.x-1.x/tests/src/U...

drunken monkey’s picture

Title: SearchApiAllTerms views argument doesn't filter results » Regression in Views argument plugins (date and all terms)
Component: General code » Views integration
StatusFileSize
new3.93 KB

Thanks for reporting this issue and providing a patch. Also thanks to Mike for pinging me on Slack, spotting the cause and also that the date argument plugin is affected, too. This is indeed quite a bad regression. Even worse that our automated tests didn’t catch it.
However, your proposed solution doesn’t take the new API introduced in #3260862: Add a $add_directly parameter to QueryInterface::createConditionGroup() into account, to which we should definitely still switch. Also, we should also fix the date plugin, while we’re at it.

Patch attached, please test/review!

arefen’s picture

Hi. Thanks, drunken monkey
Patch #4 saved my life

drunken monkey’s picture

drunken monkey’s picture

Status: Needs review » Fixed

Good to hear! I’ll take that as a confirmation that the patch worked as expected.
Committed. Thanks again, everyone!

Status: Fixed » Closed (fixed)

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

phma’s picture

When can we expect a release for this fix?

it-cru’s picture

@phma: Fix is included in 8.x-1.30 :)