Provide information about "languages (with fallback)" to backends

Thanks a lot for pointing out this problem and providing a patch! You’re right, we totally forgot that the Solr backend needs language information for reasons other than filtering on the language code, so this should also be included in some way when using a language filter with fallback.

However, I’m also not quite convinced that such a large change, with a new QueryInterface method, is actually needed here. (If it is, it would at least need better documentation, so people have a chance of understanding what this property means.) For one thing, there are already query options, which are usually used when some specific plugin or component provides or needs some information that doesn’t concern the large majority of plugins. As such, I think this new data could just be added as a query option instead of a whole new query property with associated methods.

Also, it seems like this information should always be included for required language filters ($opInRequired in my revised patch) that do not use setLanguages() (or even for those, I guess, as it shouldn’t do any harm either?), not just when the data comes from this specific processor. (Moreover, I think it usually will.) So we can probably drop the check on the data definition. Otherwise, I think it should use a separate flag in the data definition’s settings (like currently 'views_type' => 'language',) instead of checking for the processor ID.

Finally, and completely unrelatedly, I think the boolean condition for $direct_language_condition was missing an === 'AND'. Might as well fix that, while we’re at it.

Patch with my suggested solution attached. Please review, test, and tell me if that would be enough to let you resolve the problem in the Solr backend. (It seems to me like it should, but I could of course be mistaken.)

Test fail is unrelated, see #3313067: Fix behavior of required Views input fields in search forms.

I think this is best seen in the code: As long as it’s a required (AND), positive (in) language condition, I add all selected values to the option in \Drupal\search_api\Plugin\views\filter\SearchApiLanguage::query(). But this could easily be changed, if it would make your job easier.

Good to hear, thanks a lot for your feedback!
Committed.
Thanks again!

Hi guys.
This patch broke all our search pages.
The getGroupType() returns an array instead of the expected string.

$query->getGroupType($this->options['group']) === 'AND'

So we get the error 'search_api_language Undefined field ! Incorrect filter criteria is using for searching!'

ps. we don't use solr

@drunken_monkey, edykst is correct this patch introduces a regression. We don't filter on languages anymore causing lot's of broken searches when you have big mulilingual websites.

Proper patch in the make.

My patch isn't sufficient either because the function can return a string OR an array

public function getGroupType($group) {
return !empty($this->where[$group]) ? $this->where[$group] : 'AND';
}

Thanks a lot for pinging me, Bram! Really bad that we caused a regression with this, should fix this ASAP.

However, there seems to be a separate bug regarding the backend plugins you’re using: In theory, the filter should fall back to placing a condition on the special search_api_language field, which is documented to have the same effect as the language option (basically). So, if the backend plugin doesn’t honor that, that’s a bug, too. You might want to report that there. This is also why the tests didn’t pick up the regression: As the DB backend correctly handles the search_api_language fallback, the test views worked fine. I now added explicit checks for the generated search query to make sure this doesn’t happen again.

Regarding the fix, as the getGroupType() docs clearly state that will return a string, 'AND' or 'OR', the bug is obviously in that method, not in the caller. The fix should therefore be applied there. (Seems there is only a single caller anyways, so probably a negligible risk that anyone relies on the broken old behavior.)

Please test/review the attached patch and we can hopefully fix this quickly.

You’re right, I could reproduce this behavior locally as well. Furthermore, for me, the patch in #20 did nothing to solve it. Really pretty bad. Not sure what, yet, but Core seems to have messed something up there.

Created #3318187: Fix regressions in Views filters as the follow-up issue to fix all those regressions, so people have an easier time finding it. Marking this one as “Fixed” again.
Still, thanks a lot for your work in here!