Add an option to allow search pages to support facet only searching

Just tested this and it works for me - thanks!

Leaving at needs review until the "[all items]" question is resolved.

Hello -- this patch works quite nicely, but the Apache Solr Search Advanced Options have 4 options for the Behavior on empty search

1. Show search box
2. Show enabled facets' blocks under the search box
3. Show enabled facets' blocks in their configured regions
4. Show enabled facets' blocks in their configured regions and first page of all available results

Can the option to show enabled facets' blocks under the search box (and not show those facets in their configured region) be added to this patch? That would match the functionality and provide a very important option.

Hey I really want to test out this functionality as this is what I'm looking to do. However im new to patching files... any chance this has been committed to the dev branch yet?

Ok I finally figured out how to patch a module... After quickly testing this patch, so far so good. However I get a message at the top of the search page saying The following search keys are too short or too common and were therefore ignored: "".

I assume this is cause I am only using facets and don't have anything typed in the search box. Is there any way to remove the message?

A quick question concerning patch in #2: how can i change the sort order of the empty search result?

That does the job perfectly. Thx.

What is the status on this functionality?
I think a lot of sites would need this. We do. Currently I have just hacked the module and moved it to modules/custom/ to avoid losing it when upgrading.

its not possible to apply it to the latest version. it may be not commited because this feature belongs into facet api? i dont know.

The module maintainer probably just hasn't had time to review it.

Yes, that's exactly it. I'm very sorry, but I was first injured and am now still pretty busy looking at all the issues I missed.

In any case, this looks very good, well done! The hack with using $_GET['f'] for determining whether facets are set is a bit dirty (the parameter could, e.g., be called something else) – could you maybe try to find out whether there's a cleaner way?
Also, it seems this needs a re-roll – I know, it's my own fault, but could you please take care of that?

Other than that, I'd happily commit this patch. Thanks a lot for your efforts!

straight reroll of #2

#19 works, thanks for the reroll, scor. However, I get the same message mentioned in #7...

The following search keys are too short or too common and were therefore ignored: "".

Updated patch to only show this message when $page->options['empty_behavior'] == 'none'

Updated the above patch to squash form_validate errors when submitting a search with no keywords.

IE if a given search page allows "facet only" behavior, you should be able to submit the search form with an empty keywords field.

This was useful for me because I use search_api_location (https://drupal.org/sandbox/madmatter23/1799850), which modifies the search form rather than adding a facet. I'm sure there are other similar use cases.

Here is an updated patch for the latest dev version
I also fixed one small bug where the not operator was missing in the validate function

You'll need to take this bit out of the patch:

-- a/search_api_page.info
+++ b/search_api_page.info
@@ -5,3 +5,10 @@ core = 7.x
 package = Search
 
 configure = admin/config/search/search_api/page
+
+; Information added by drupal.org packaging script on 2013-07-10
+version = "7.x-1.0-rc1+1-dev"
+core = "7.x"
+project = "search_api_page"
+datestamp = "1373485506"
+

Also, this patch still has the $_GET['f'] in it that was complained about earlier by the maintainer (see comment #17). So it probably needs to figure out a better way to verify that facets have been set.

Weird how that info part came into the patch, anyways attached is the one without it.
And I totally read over that comment (#17), will try to break my brain on that point.

I don't think you even need to, or should – just execute the search in any case, and maybe check afterwards whether filters were set and a result should be displayed. As ohthehugemanatee points out, things like location search could also add some filter that could lead to the search being valuable even though no keys were given.

On the other hand, things like node access could also set filters, so maybe that's not so ideal after all … Maybe just check for any additional GET parameters?
In any case, I don't think search_api_page_search_execute_empty_search() is needed – just set $limit to 0 for the search.

I have removed the search_api_page_search_execute_empty_search() and instead using search_api_page_search_execute() with limit set to 0 to create the facet only search page
Find attached the patch. its basically identical to search_api_page-show_empty_search_results-22-1235026.patch exept for the change to use the search_api_page_search_execute() function instead

Thanks for reworking your patch!
However, there's still a lot of issues I found with it:

1. +++ b/search_api_page.admin.inc
   @@ -112,7 +112,21 @@ function search_api_page_admin_add(array $form, array &$form_state) {
   +        'none' => t("Show search box"),
   

   Please use single quotes where possible.

2. +++ b/search_api_page.install
   @@ -238,3 +238,15 @@ function search_api_page_update_7103() {
    }
   +/* Add the default option for the empty search option to all existing search
   + * pages.
   + */
   

   Please use the proper format for documentation comments and leave an empty line above it.

3. +++ b/search_api_page.install
   @@ -238,3 +238,15 @@ function search_api_page_update_7103() {
   +  $pages = search_api_page_load_multiple();
   +  if (!empty($pages)) {
   +    foreach ($pages as $page) {
   +      $page->options['empty_behavior'] = 'none';
   +      $page->save();
   +    }
   +  }
   

   According to the API, you shouldn't use module (especially CRUD) functions in update hooks. Please rewrite to use just database functions.

   Or, just leave the update hook out entirely, and check for the existence of the option everywhere you use it. To make it easier, you can also just use an empty string instead of 'none' as the first option, so can just use empty() for checking for that.

4. +++ b/search_api_page.module
   @@ -493,7 +493,9 @@ function search_api_page_search_form(array $form, array &$form_state, Entity $pa
   +  if (!trim($form_state['values']['keys_' . $form_state['values']['id']]) && !($page->options['empty_behavior'] == 'results' || $page->options['empty_behavior'] == 'blocks'))  {
   

   Why not just check for 'none' here, instead of both of the others?

5. +++ b/search_api_page.pages.inc
   @@ -32,7 +32,8 @@ function search_api_page_view($id, $keys = NULL) {
   +  if ($keys || $page->options['empty_behavior'] == 'results' || ($page->options['empty_behavior'] == 'blocks' && !empty($_GET['f']))) {
   

   I wouldn't explicitly check for $_GET['f'] here, as that's pluggable. Just execute the search in any case and afterwards check whether there were filters set. (#1390598: Add a way to easily identify facet filters inside the query would of course help with that.)

6. +++ b/search_api_page.pages.inc
   @@ -81,7 +82,7 @@ function search_api_page_view($id, $keys = NULL) {
   -    if (!empty($results['ignored'])) {
   +    if (!empty($results['ignored']) && $page->options['empty_behavior'] == 'none') {
   

   Why wouldn't we want to display ignored keywords for other settings of the empty behavior?

7. +++ b/search_api_page.pages.inc
   @@ -89,6 +90,9 @@ function search_api_page_view($id, $keys = NULL) {
   +  } elseif ($page->options['empty_behavior'] == 'blocks') {
   +    // Run an empty search so we can have facets.
   +    search_api_page_search_execute($page,'',0);
   

   Please keep to the coding standards.
   Also, use NULL here instead of an empty string (also in search_api_page_search_execute() below).

Will there be new work on this?
I would really like to have this funcitonality in production env.

@'drunken monkey' Hey there I am working on re-rolling the patch with the changes you've suggested in #27 above. I agree that it makes sense in 5. that $GET['f'] not be included here, but I am having trouble figuring out how to determine if any other add-ons (such as the facet api) have added additional filters on the query. I checkout out the Search API issue link you posted (#1390598: Add a way to easily identify facet filters inside the query) and scoured through the commits around that time, but don't see how this was implemented.

Can you point me in the right direction of how I can determine this so that I add a conditions in the following if...

-  if ($keys) {
  // Do a search if we have keys, or our empty behavior and facets dictate.
  if ($keys || $page->options['empty_behavior'] == 'results' || ($page->options['empty_behavior'] == 'blocks' && !empty($_GET['f']))) {
     // Override per_page setting with GET parameter.
     $limit = $page->options['per_page'];
     if (!empty($page->options['get_per_page'])
@@ -81,7 +82,7 @@ function search_api_page_view($id, $keys = NULL) {
       $ret['results']['#pager']['#theme'] = 'pager';
     } 

to something more like:

  // Do a search if we have keys, or our empty behavior and facets dictate.
  if ($keys || (!empty($page->options['empty_behavior']) && ($page->options['empty_behavior'] == 'results' || !(some_function_to_determine_if there_is_something_more_then_an_empty_search())))) {

Here is an update of the patch, but with the $GET['f'] still in there. Otherwise it cannot work as is. However, if you can point me to the right place then I can update that if with something not involving a facetapi specific query param.

Thanks a lot for re-rolling and working on this!
Attached is a version which fixes this last problem (as well as a few coding standards problems) and would be ready to be committed in my opinion.
Please test/review!

@drunken monkey ...Sorry...no attachment
greetings, Martijn

@drunken monkey I'd be happy to test the latest patch once you submit it.

Oops, sorry! Here it is.

It works for me just great. I've marked it RTBC, but you may want additional feedback from others.

Thanks for testing!
However, upon re-testing finally before committing I saw that empty searches actually produce a "No valid keywords present." warning when using the database backend. That of course shouldn't happen – it seems we were sending an empty string instead of NULL as the keywords for empty searches, causing this problem.
The attached patch should fix this problem, too. Please test/review!

SearchApiQueryFilterInterface::getFilters() will actually return an array with BOTH SearchApiFilter objects AND arrays (consisting of a field, a value and an operator)... the logic to determine if a search had facet filters set (in search_api_page_query_has_facets()) wouldn't recognize the array(field, value, operator) case.

I've improved the logic to ask the FacetapiAdapter object for it's active items.

Whoops, looks like I used some functions from the facetapi module and forgot to check that it's installed and enabled first.

Fixed patch attached.

While I can't review my own code, I can review the code in the patch in #36 to help move this issue forward...

+++ b/search_api_page.admin.inc
@@ -112,6 +112,17 @@ function search_api_page_admin_add(array $form, array &$form_state) {
+      '' => t('Just show the search box'),

@@ -445,6 +456,17 @@ function search_api_page_admin_edit(array $form, array &$form_state, Entity $pag
+  $form['options']['empty_behavior'] = array(
...
+      '' => t('Just show the search box'),

An empty value for this option could have problems in some browsers.

According to the HTML4.01 specification, "If a control doesn't have a current value when the form is submitted, user agents are not required to treat it as a successful control." and "A form data set is a sequence of control-name/current-value pairs constructed from successful controls".

In other words, if a user selects "Just show the search box", a browser might not POST the empty_behavior control at all, meaning form validation could fail if a user selects "Just show the search box".

As far as I can tell, this currently wouldn't cause a problem because empty_behavior is not a required field; however, it's conceivable that someone might hook_form_alter() this form and make empty_behavior required.

+++ b/search_api_page.admin.inc
@@ -445,6 +456,17 @@ function search_api_page_admin_edit(array $form, array &$form_state, Entity $pag
+      '#default_value' => (empty($page->options['empty_behavior'])) ? '' : $page->options['empty_behavior'],

According to the Drupal coding standards, "Use an indent of 2 spaces, with no tabs.": this line has 6 spaces when it should have 4.

Other than those two things, the code in #36 looks good.

It works on my site; but I haven't yet tested all aspects of the patch.

Thanks a lot for your work! You're right, I was mistaken in assuming that all facet filters will be set as filter objects, I seem to have been confused myself there.
Your approach looks very clean and also seems to work fine, thanks! I just adjusted it to use the known "searcher" ID for the currently used index, since the previous code could have been a little buggy in situations with multiple searches on a single page.

Thanks also for spotting the indent error! This and a few other style issues should now also be fixed.
I'm pretty sure the empty key won't cause any problems, though.

Anyways, patch attached, please make sure it still works for you, everyone!
(Also, it's remarkable how popular this is, I don't think I've ever had a patch with so many contributors … We should really get this in now!)

i didn't do a thorough code review, but i can say that #41 worked for me -- thanks!

Would have been good to get a few more reviews, but it'll probably be alright.
So, (finally) committed.
Thanks again everyone for all your help! Never had such a large commit message. ;)

One thing to notice, and is quite important in my case, is that if you start your browsing with filtering some facets, and after that search for a term, the filtered facets are reset. This because the query parameters are ignored.

In

/**
 * Form submission handler for search_api_page_search_form().
 *
 * @see user_login_form_validate()
 */
function search_api_page_search_form_submit(array $form, array &$form_state) {
  $keys = trim($form_state['values']['keys_' . $form_state['values']['id']]);

  $keys = strtr($keys, array("\\" => "\\\\", '/' => "\\"));

  $form_state['redirect'] = $form_state['values']['base_' . $form_state['values']['id']] . '/' . $keys;

  // To completely controll the redirect destination, we need to remove the
  // 'destination' GET parameter, which would override our destination, if
  // present.
  unset($_GET['destination']);
}

Attached a patch.

That's a completely different issue, please create a new one instead of resurrecting fixed ones.

Well, don't want to nitpick, but its this fix that created that issue in the first place.