All taxonomy terms fields contextual filter not working with multiple tids

Could you please elaborate on how the current code “doesn't appear to work at all”? Maybe with some examples? The Views admin UI live preview should already give you a nice representation of the query, so maybe just show a comparison of what conditions are placed before/after your patch for some example arguments.
The code looks correct to me, that’s why I ask. (Though it does in any case seem like a good idea to use IN instead of multiple conditions, for performance reasons.)

Anyways, seems we desperately need some test coverage for this. Would you be able to provide one that shows how this currently doesn’t work, and that your patch fixes this?

Also, seems you forgot to rename $keep in one instance.

Updating this patch to support both AND (tid,tid,tid) and OR (tid+tid+tid) queries.

This still needs tests (at least functional ones) to ensure that this keeps working but I think that this change makes sense, adding an IN operator for performance sounds like a good idea.

I'm not sure if we should do anything to Search API solr for this. @mkalkbrenner should know.

Ran into this issue today, and Patch #6 cleared it up for me.

Since a scenario was requested, you can perform the following steps to reproduce.

Scenario:

1. Create content type with two fields, each with a different vocabulary (A and B) of terms (one single-value only, one allowed multiple values). Add the fields in your search index and re-index all the content.
2. Create two nodes, one with a term from vocab A and one with a term from vocab B.
3. Create a search view for the content. Add the contextual filter for "Search: All taxonomy term fields" and set the following information:

When filter value is not available:

1. Provide default value: selected
2. Type: Taxonomy term ID from URL
3. Load default filter from node page: checked
4. Limit terms by vocabulary: (select your vocabs)
5. Multiple-value handling: Filter to items that share any term

When filter value is available or default provided:

1. Specify validation criteria: checked
2. Validator: Taxonomy term ID
3. Vocabulary: (select your vocabs)
4. Multiple arguments: One or more IDs separated by , or +
5. Action to take if not validate: Display contents of "No results found"

More:

1. Allow multiple values: checked

Testing:

1. Enable the Preview query to be visible.
2. In your Preview with contextual filters input box, enter the two taxonomy IDs with a + between them (example: 13+17), and click Update preview.

Expected behaviour:

1. You should see both nodes appear (since each one is tagged by one of the terms in the contextual filter argument).
2. You should see the Query condition using an OR statement for taxonomy IDs (since the + symbol is for OR, allowing you to say term X or term Y).

Actual behaviour:

1. You see no nodes appear.
2. You see the Query condition using an AND statement.
3. You can confirm this bug by switching your contextual argument to use commas instead of a + (example: 13,17). Query will still use AND conditions.

Thanks a lot for the detailed description! I was now easily able to reproduce this, and confirm that this is a problem. (Though it’s a long way away from “doesn't appear to work at all”, of course – but yes, we want to support multi-value arguments with both conjunctions.)

The attached is a cleaned-up version of the patch in #6 (also thanks a lot for that!), which works fine for me, too, and should also cover a few special cases. Please test/review to make sure this still works as intended.

However, with the code getting more complex we should also definitely have test coverage, so this is still needed before we can commit.

Forgot to attach the patch.

Hm, OK, thinking about it a bit more, the “no field found for term” logic didn’t quite make sense. Should be better this way. (But this is exactly why we need tests! With examples, this is much easier to understand.)

Rerolled patch #11 for 8.x-1.16. Minimized a couple lines of code to use inline-IFs since they were simple checks and values.

Any chance we can get this committed for next release? I see we're just missing the tests. Can anyone take the use-case I provided in #8 to write some tests?

I'm not set up for test automation atm, and likely won't have time to do so for quite a while due to scheduling.

I have verified this issue exists and patches #11 & #12 fix the issue consistently for their respective module versions. If someone could submit tests so we could get this patched that'd be stellar!

Thanks for the re-roll, and the suggested small changes! However, please provide an interdiff when making actual changes (apart from the re-roll) so it’s easier to see what you changed.
While the inline ?: condition makes sense in the first case, the second one with two variables being set is probably clearer when kept in an actual if. No big difference either way, in any case, and mostly just taste.
The @var line of course definitely makes more sense immediately before the foreach, thanks for that!

The other two changes seem more accidental – specifically, the one seems to indicate you don’t have your editor/IDE set to UTF-8, which could cause a lot of hard-to-detect problems.

Anyways, as you say, tests are still needed.

@drunken monkey, your patch worked for me! Thank you so much!

Thanks for the feedback, niles38!

Well, it took more or less a whole day, but I now got tests working for this so we can finally commit it.
Please test/review!

Committed.
Thanks again, everyone!

I have the same error with 8.x-1.29 version
I can't apply the patch.
When i run the query with taxonomy term ID = 26, i get this:

Query	
Index: default_product_index
Keys: NULL
Searched languages: it
Conditions:
  status = 1
Sorting: title ASC
Options: array (
    'search_api_view' => 'object (Drupal\\views\\ViewExecutable)',
    'search_api_included_languages' => 
    array (
      0 => 'it',
    ),
  )

No term ID inside the query.

While the title of views changes correctly based on the ID.

Thank you

I have the same error with search-api version 8.x-1.29, Drupal 9.5.9, any ideas please?

Ended up here for the same reason. Looks like a regression which was fixed here, but hasn't been released yet #3354906: Regression in Views argument plugins (date and all terms): https://git.drupalcode.org/project/search_api/-/commit/9faec4590e48d8add...