[Regression] View term filter 'is/not empty' and 'not one of' operators do not work

Drupalorg is the best place to triage this. The SearchAPI-powered Views have not made it into Project Issue yet. Likely, this could be a problem in SearchAPI.

This is definitely a pain for prepping for things like issue triage, sprints, etc.

I also found #1529016: "Is not one of" filter operator broken but from #1 it sounds like we might not be using Views for this after all?

Closed #2244721: NOT IN filter on issue search does not work as a duplicate of this issue. That issue had the "d.o hitlist" tag which I've added here for parity.

#477984: CCK field is empty/not empty filter doesn't work when exposed in the Views issue queue sounds like it's describing a similar issue. There's a recent patch against the 7.x-dev branch.

The root of this appears to be a long-standing bug in Views that ignores empty/not empty exposed filters if a value isn't passed as well. (See #477984: CCK field is empty/not empty filter doesn't work when exposed, referenced above.)

I set this up on a clean install and discovered the following:

When the filter is set to display all items where the tags are empty (TID is NULL), the query produced by views_plugin_query_default() looks like this:

SELECT node.title AS node_title, node.nid AS nid, node.created AS node_created, 'node' AS field_data_field_test_tags_node_entity_type FROM node node WHERE (( (node.status = :db_condition_placeholder_0) AND (node.type IN (:db_condition_placeholder_1)) )) ORDER BY node_created DESC LIMIT 10 OFFSET 0

Note that it is completely ignoring the exposed filter settings.

If you also somehow include a term in your query (for example, TID is NULL and TID = beta blocker), the query instead looks like this:

SELECT node.title AS node_title, node.nid AS nid, node.created AS node_created, 'node' AS field_data_field_test_tags_node_entity_type FROM node node LEFT JOIN taxonomy_index taxonomy_index ON node.nid = taxonomy_index.nid WHERE (( (node.status = :db_condition_placeholder_0) AND (node.type IN (:db_condition_placeholder_1)) AND (taxonomy_index.tid IS NULL ) )) ORDER BY node_created DESC LIMIT 10 OFFSET 0

So, somewhere in Views, there is logic checking that an exposed filter has a value before including that filter in the query. But, since an empty/non-empty filter doesn't have a value, it doesn't pass the logic check, so doesn't get included when the query is built.

Additionally, within the filter handler there is $this->operator, which is supposed to contain a string like 'or', 'and', or 'empty'. In all cases, it is just 'or'.

The patch posted to #477984 doesn't appear to have any affect on the issue.

I sat with tim.plunkett for a bit @ the DrupalCon LA sprint to look into this. Attached patch isn't beautifully elegant in all ways, but it fixes the problem. I'll try to summarize:

- Generally, Views makes all sorts of assumptions that if a filter has no input, it should be ignored.
- There's a mechanism to over-ride this, in the form of the accept_exposed_input() method. If that returns TRUE, Views assumes that the handler has all the input it needs to be included in the query.
- Only all the way up in views_handler_filter::accept_exposed_input() (nearly the base of the class hierarchy) do filters that include an operator argument set $this->operator.
- views_handler_filter_in_operator is the class that adds the EMPTY vs. NOT EMPTY choices to the possible operators.
- However, views_handler_filter_term_node_tid (the child class for filtering by taxonomy term), has a clause inside accept_exposed_input() that is bailing out early if there's no value, forgetting about the possibility of the EMPTY and NOT EMPTY operator choices.

So, it's a bit ugly, but this patch is adding the code to properly set $this->operator inside views_handler_filter_term_node_tid::accept_exposed_input() early enough for us to use that knowledge to return TRUE if we're dealing with one of the (NOT)? EMPTY operators, since we don't need any further input beyond the operator.

It seems evil and wrong that properly initializing $this->operator is a side effect of this function that's also being used to determine if we have enough input yet. :/ But a complete re-design of this system is out of scope for this issue. Since views_handler_filter_term_node_tid::accept_exposed_input() is currently where the bug lives (returning FALSE when it shouldn't), that seems like an acceptable level to fix this.

Thoughts/objections/alternatives?

Thanks!
-Derek

Note: fixing this on d.o will actually require fixing SearchAPI, since that's what powers the production issue queue views. I just uploaded an equivalent patch to that queue:

#2489882: Taxonomy views filter doesn't handle is/not empty operator

@jesss: If you wanted to test that patch to search api on the dev site you were using, that'd be fantastic. Please let us know if that's possible.

Thanks!
-Derek

Please ignore testbot failures for this patch. See #2273481: Contrib PSR-4 PHPUnit tests are not picked up by PIFR for more.

hooray!

Hooray indeed! However, this doesn't actually affect d.o. :) The one we really need to deploy is actually #2489882: Taxonomy views filter doesn't handle is/not empty operator.

Did some form of life review earlier already.

Great improvements!

Those tests were not fun to write. Use CSS selector instead of xpath then. :P

I think the actual tricky part is to setup all that stuff.

weird, that was rtbc, and was *still showing green*. not sure why that failed message was added in #27. retested.

green. back to rtbc then.

Oh, yes, please let's not hit this bug again in 4 years.

Committed and pushed to 8.0.x. Thanks!