Add Fallback to default drupal search option

In other words, if the return code of the request is not 200, do a drupal_goto("search/node/$keys");

An interesting feature. I wouldn't oppose it, I don't think.

Hi Robert -

Jacob's dad Raj here (aka pahariwalla) on assignment to make this patch. We decided to give the admin 3 options for what happens if Apache Solr is not available.

1) redirect to Drupal search results
2) show "no results" as it currently does
3) same as 2) but with error message

apachesolr.module: I've added a select field "apachesolr_failure" to the settings form with these 3 options
apachesolr_search.module: call to a new function "apachesolr_failure" in the try/catch in apachesolr_search_search

p.s. thanks for the good advice re: my previous patch #279698: Allow for restricted use of solr search to be for Indexing Only, Searching Only, or Both. We're bailing on that on the first go-around. Sorry I didn't get back to that... looks like you addressed it on day 1. doh! I swear I was checking to see the status, and then was expecting to get an email when someone commented... yikes triple faux pas.

Hope this one goes a little smoother.

Raj

Your patch is not very clean, the absolute path to your module is written and it is reversed. Attached is a clean, otherwise unaltered version.
Couldn't test it, though, because under D5 the scriptat my server just shows a white screen instead of the "No results" page, strangely.

Many thanks and apologies. Not sure about the white screen as I can't replicate the behavior here.. Anyways, I think I ran the patch properly this time, also changed up the options to be associative array instead of constants.

The withe screen isn't the fault of your patch, as it appears also without applying it. This just means I can't help testing this.
The patch looks good this time, thanks.

You have the dubious honor of enabling my humble introduction to the drupal community. It's been a long haul from COBOL, through the dark and hidden environs of VB and MS, but it appears I may have found a hook into the light.

Thanks for your help and patience.

critical = solution needed for 1.0 release.

+ drupal_set_message('The Apache Solr search engine is not available. Please contact your site administrator', 'error');

Punctuation. Needs . after administrator: "Please contact your site administrator."

+ break;
+ case 'show_drupal_results':
+ drupal_goto("search/node/". $querystring);

XSS. Unfiltered $querystring.

Added period ("."). Also did the same in setting form in-line doc.

1) Just be be sure my process is correct... After backing up and blitzing my apachesolr directory, I used "cvs -up dP " to get the latest, made the mods, tested locally, and ran "cvs diff -up" to create the patch file. please advise, thanks.

2) In testing, I found what may be a lousy user experience for a specific combination of this setting and the new/proposed search_config "default search" setting. #295136: Choose which search implementation should be default (including search box)

Assume the following settings:
apachesolr_failure = "No Results"
search_config_default_search = "apachesolr_search"

The problem: If apachesolr is not available you will always see "No Results....", even if the Drupal search returns results.

Should we remove "No results" option? I can't remember the reason we put it there to start with. Might it be to prevent the execution of the Drupal search - thus avoiding a potential hit on the server?

We could change apachesolr_failure logic to check first to see if there are Drupal search results and redirect, but that of course would require executing the Drupal search...

Ideas?

Have to read your questions and answer them later, but the drupal_goto still has an XSS attack vector. Please put the variable in drupal_urlencode: http://api.drupal.org/api/function/drupal_urlencode/6

No worries on previous questions, but you might want to consider the usability issue at some point.

This patch replaces this:

drupal_goto("search/node/". $querystring);

to this:

drupal_goto(drupal_urlencode("search/node/". $querystring));

Still lots of issues with the patch, but I'm cleaning it up:

I moved the apachesolr_fail function to the apachesolr.module because this is a central setting and a central API, whereas the apachesolr_search.module is an optional implementation of the search.

Added t()

drupal_set_message(t('The Apache Solr search engine is not available. Please contact your site administrator.'), 'error');

Removed superfluous break;

    case 'show_drupal_results':
      drupal_goto('search/node/' . drupal_urlencode($querystring));
      break;
    case 'show_no_results':
      return;
  }
}               

Only the variable needed to be query encoded:

    case 'show_drupal_results':
      drupal_goto('search/node/' . drupal_urlencode($querystring));    

Added code comments to the new function.

Changed the order of events in the exception catch block because watchdog should record the error in all cases (wouldn't happen if the redirect comes first).

        catch (Exception $e) {
          watchdog('Apache Solr', $e->getMessage(), WATCHDOG_ERROR);
          apachesolr_failure($query->get_query());
        }

Changed whitespace. Note spaces before and after =>

+    '#options' => array('show_error' => t('Show Error'), 
+      'show_drupal_results' => t('Show Drupal Results'), 
+      'show_no_results' => t('Show No Results')
+    ),

Added a message for usability (because the redirect is invisible otherwise):

      drupal_set_message(t("%search_name is not available. Your search is being redirected.", array('%search_name' => $search_name)));   

Here's what the patch looked like in the end.

Committed to DRUPAL-5 and DRUPAL-6--1. Thanks!

Robert - I sure do appreciate these lessons in Drupal standards, php, and coding in general ... After so many years of one-man-projects, my standards have laxed and I tend to do things the way I'm used to ... y'know, with my eyes closed.

I will be posting a log somewhere with all of the examples with "before and after" images of the code I've been writing. I'm looking forward to continuing the effort and reducing the editorial requirements.

!! Thanks thanks thanks !!

Thanks for your help.

Moving the status to "needs review" was unintentional, right?

Most welcome. Help is fun.

I changed the status ? Sorry - not sure how I did that. Probably mis-using the cursor or page keys... d'oh