backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 40 forks source link

Views: Search Filter: On Empty Input "Show None" option Shows All #5729

Open quicksketch opened 2 years ago

quicksketch commented 2 years ago

Description of the bug

When using a view to build a search page, the option for "On Empty Input" controls whether an empty dataset will be returned or if all results will be shown. This option seems to only work if the field is also made required. This should not be a requirement, since landing on a search page URL directly should not immediately throw a validation error.

image

Steps To Reproduce

To reproduce the behavior:

  1. Enable search module.
  2. Add a new view page of type content at any path, e.g. /test-search.
  3. Add the filter for "Search: Search terms"
  4. Change the "On empty input" option to "Show None". Make sure that the checkbox for "Required" is unchecked.
  5. Save the view and visit the page.
  6. With no search terms entered, no search results should be shown. Instead, all content is shown within the limit of the pager.

Additional information

Add any other information that could help, such as:

quicksketch commented 2 years ago

PR filed at https://github.com/backdrop/backdrop/pull/4174 that should fix this problem.

This filter seems made to always execute regardless of whether a value is specified, so I set the $always_required property to TRUE, but then we don't want to put a required asterisk on the field, since otherwise emptying out the search terms would not be allowed.

This can be tested manually following the steps to reproduce, except now the "Required" checkbox is hidden entirely when configuring the filter.

indigoxela commented 2 years ago

This filter seems made to always execute regardless of whether a value is specified, so I set the $always_required property to TRUE

Yet another undocumented property in views, yay :grinning: Edit: oh, it is documented.

I did a quick check in the sandbox, but this change seems to have side effects:

As now it's "always required", it's not possible to set it to required by intention anymore (no such admin form item):

require-not-possible

On the views admin UI, when trying to run the preview, I get:

SQLSTATE[42S22]: Column not found: 1054 Unknown column 'FALSE' in 'where clause' 

I created the simplest view possible, and the query is:

SELECT node.`title` AS `node_title`, node.`nid` AS `nid`
FROM 
{node} node
WHERE (( (`node`.`status` = '1') AND (`FALSE` = '') ))
LIMIT 10 OFFSET 0

For the "regular" page that error only shows up in dblog.

indigoxela commented 2 years ago

This is all very, very odd. I sat for quite a while and stared at this code block: https://github.com/backdrop/backdrop/blob/1.x/core/modules/search/views/views_handler_filter_search.inc#L126-L140 Does this make sense, anyway? Most of all: add_where($this->options['group'], 'FALSE'); (huh?)

However, I found a "trick" to get the "Show All"/"Show None" setting working by using the validator and if we do not want to show content, do something similar like views_handler_argument::default_empty() does.

Not sure if that's correct. Just opened the PR for easier discussion.