* Add an index with taxonomy references
* Add a view with "is empty" filter

The view has no results even when it should,
the query is broken: "IS NULL NULL".

Index: mcp_files
Keys: NULL
Conditions:
  [
    field_technologies IS NOT NULL NULL
  AND
    field_topics IS NULL NULL
  AND
    type <> NULL
  ]
Options: array (
    'search_api_view' => 'object (Drupal\\views\\ViewExecutable)',
    'search_api_base_path' => 'file-todo',
  )
CommentFileSizeAuthor
#5 2858303-5--views_taxonomy_filter_empty.patch1.72 KBdrunken monkey
#5 2858303-5--views_taxonomy_filter_empty--interdiff.txt2.32 KBdrunken monkey
#2 search_api_views_fix_opempty_filters.patch717 bytestavib47
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

axel.rutz created an issue. See original summary.

tavib47’s picture

tavib47 CreditAttribution: tavib47 as a volunteer commented
Assigned: Unassigned » tavib47
FileSize
search_api_views_fix_opempty_filters.patch717 bytes

Hi,

I implemented the opEmpty() method of SearchApiFilterTrait to change the conditions from IS NULL NULL and IS NOT NULL NULL to = NULL and <> NULL

geek-merlin’s picture

geek-merlin
they/omg
Primary language German
Location Freiburg, Germany
CreditAttribution: geek-merlin at Clever Systems, MachbarMacher, DruConsult commented
Status: Active » Needs review

Thanks for coding this, code makes sense. It will take some weeks though before i will work on this again and can test it.

Status: Needs review » Needs work

The last submitted patch, 2: search_api_views_fix_opempty_filters.patch, failed testing.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
CreditAttribution: drunken monkey as a volunteer commented
Status: Needs work » Needs review
FileSize
2858303-5--views_taxonomy_filter_empty--interdiff.txt2.32 KB
2858303-5--views_taxonomy_filter_empty.patch1.72 KB

Great catch, thanks a lot for reporting and fixing!
Your patch looks pretty good, but I think we can solve this even simpler, and more safely (in case code from anywhere else does this) – patch attached.
Also, however we fix this, it means we can remove the existing opEmpty() overrides for the numeric and date filters.

Finally, it would be great to add a test for that to the ViewsTest. On the other hand, by removing the custom overrides for the numeric and date filters, we'll already have some testing there, so it's not too bad if we don't additionally test this for taxonomy term references, too.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer and at Dazzle commented
Status: Needs review » Reviewed & tested by the community

Looks good, I like removing code!

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
CreditAttribution: drunken monkey as a volunteer commented

Thanks for reviewing!
Would be great, though, to also get a verification from beluoctavian or axel.rutz (or someone else experiencing this issue) that this indeed resolves the problem properly.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
CreditAttribution: drunken monkey as a volunteer commented
Status: Reviewed & tested by the community » Fixed

Well, then I guess we'll just have to hope for the best.
Committed.
Thanks again, everyone!

Status: Fixed » Closed (fixed)

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