Apache Solr Advanced Search module provides users with advanced search options using apache solr.
This module is for Drupal 6.When we set apache solr as default search, the advanced searching options from core search are lost.So this module makes advanced searching available with apache solr.
There is no other module which provides advanced search for apache solr.
It creates a new page at url: /advancedsearch

Admin Settings:

Admin can configure advanced search settings under Site Configurations > Apache Solr > Advanced Search.

Admin can configure where to show "Advanced Search" link under each search form.
Admin can also enable or disable content specific search.

Advanced Search Page:

User can perform a more specific search by using the options such as search containing:

  • All of these words
  • Exact phrase
  • Any of these words
  • None of these words

Each of the above can be searched either in the Title of the page or Any part of the page.
User can also choose how many results should be shown per page during every search.This module also provides option to allow user to search in specific content type.

Project Page: http://drupal.org/sandbox/Avaneesh/1717362

Git Repository:
git clone http://git.drupal.org/sandbox/Avaneesh/1717362.git apachesolr_advanced_search

Code Reviewed:

http://drupal.org/node/1720336#comment-6331102
http://drupal.org/node/1721292#comment-6331168

Comments

Avaneesh’s picture

Issue summary: View changes

Modified description

Avaneesh’s picture

Issue summary: View changes

modified description

Avaneesh’s picture

Issue summary: View changes

added code reviewed information.

Milena’s picture

Status: Needs review » Needs work

Hi,

I'm not able to install your module due to lack of a server that has SOLR installed. But I've worked with SOLR (mainly for Drupal 7 though) so I thought that I might be able to give you some general advices based on code. I hope they will be useful.

.module file

apachesolr-advanced-search-content-type-option - use underscore instead of hyphen.

apachesolr_advanced_search_menu()

Do not use TRUE as access callback because everyone will have access to specific menu items.
With access to search page it is not so harmful (but generally should not be done). But to admin pages there must be some restrictions.

apachesolr_advanced_search_advancedsearch_settings()

As you provide variables you should uninstall them in hook_uinstall in .install file.

'#title' => t(''),
This is not a good example. I do not know how can I translate empty string ;)
If you don't want to have title consider hiding it with .css file.

 $form['submit_message'] = array(
    '#type' => 'value',
    '#value' => t('The Apache Solr Advanced search settings has been saved.'),
  ); 

Why not using drupal_set_message() instead?

You can also check system_settings_form() as it is designed for administration forms.

apachesolr_advanced_search_form_alter()

Variables are user input. Check them with one of proper functions. See http://drupal.org/writing-secure-code

Do not add links through a tag. Use l() function instead.

apachesolr_advanced_search_advancedsearch()

 $list = array(
    'any' => 'any part in the page',
    'title' => 'in the title of the page',
  );

Use t() function for any output.

Also empty title issue and not using t() in attributes.

apachesolr_advanced_search_advancedsearch_submit()

Do not use check_plain while there is no user input. It's unneccessary.
check_plain('Please enter some keywords.')

Also "$base_url" is unneccessary parsing. $base_url is good enough.

Other

Sometimes you use single quotes, sometimes double quotes.
Please read http://drupal.org/coding-standards#quotes

klausi’s picture

Status: Needs work » Closed (won't fix)

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

Avaneesh’s picture

Status: Closed (won't fix) » Needs review

Hi Milena,
Thanks for your helpful comments and suggestions.

Sorry for the delay, I was busy with some other project so coudn't work on this.
I have made changes to the code as per your comments and committed the changes.

Reopening for review.

Thanks & regards,
Avaneesh

srutheesh’s picture

Hello Avaneesh,

In the .module file i can see one code like

  $rows = $form_state['values']['results_per_page'];
  $_SESSION['rows'] = $rows;

Is there is another way to set the $rows instead of session variable.

Thank you,
Srutheesh

alexmoreno’s picture

Hi Avaneesh,

your git link to clone the project is wrong, it should be: git clone http://git.drupal.org/sandbox/Avaneesh/1717362.git apachesolr_advanced_search

I've found this problem running coder:

apachesolr_advanced_search.module

Line 94: Potential problem: drupal_set_message() only accepts filtered text, be sure all !placeholders for $variables in t() are fully sanitized using check_plain(), filter_xss() or similar. (Drupal Docs) [security_3]
  drupal_set_message(t('The Apache Solr Advanced search settings has been saved.'), $type = 'status');
Line 322: Potential problem: drupal_set_message() only accepts filtered text, be sure all !placeholders for $variables in t() are fully sanitized using check_plain(), filter_xss() or similar. (Drupal Docs) [security_3]
    drupal_set_message(t('Please enter some keywords.'), $type = 'error');

Personally, I would also comment the code it a bit more, trying to following the "Doxygen and comment formatting conventions": http://drupal.org/node/1354

Thanks a lot for your effort and contribution to the community.

alexmoreno’s picture

Issue summary: View changes

added code reviewed information.

Avaneesh’s picture

Hi srutheesh,
Thanks for your suggestion.
Results Per Page needs to be on saved per user basis and since it needs to be persisted across pages the use of session variable is justified in this case.
I don't have any other logic for saving 'results_per_page' , if you have anything to suggest please feel free.

Regards,
Avaneesh

Avaneesh’s picture

Hi urwen,
Thanks for your helpful review.

1. changed the git link as per your comment.
2. I think the error for drupal_set_message() was because of $type = 'status' as the message I am passing is simple plain text, It should have been just 'status'. Fixed the issue.
3. Added some inline comments for the code.

Thanks & Regards.
Avaneesh

heddn’s picture

Looks like a good addition to the solr space. Hopefully you can port it to Drupal 7 soon.

apachesolr_advanced_search.module

  • Line 43/70/105/228/312: You don't need to specify NULL as that is the default for variable_get()
  • Line 126: This is a bit of a nit but it would make more sense grammatically to rephrase "any part in the page" to "in any part in the page".
  • Line 153: Pass through t()?
  • Line 169: Pass through t()?
  • Line 184: Pass through t()?
  • Line 240/310: Maybe don't use $_SESSION super global directly? See http://api.drupal.org/api/drupal/includes%21session.inc/6
  • #4 & #6: Why don't you add the parameter to the url query string? That's how I typically see this done.
klausi’s picture

"Session data must always be accessed via the $_SESSION superglobal." from http://api.drupal.org/api/drupal/includes!session.inc/function/sess_write/6

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

muhleder’s picture

Hi Avaneesh,

I've reviewed the code and I can see a few issues that you should address.

1. apachesolr version
You don't say which version of apachesolr is required, there is v1 and v3. You should specify this in the README and also in the .info file (if this is possible in D6)

2. Permissions
The permissions you're testing for in apachesolr_advanced_search_menu() need to be defined in apachesolr_advanced_search_perm()
http://api.drupal.org/api/drupal/developer%21hooks%21core.php/function/h...

3. PHP notices
$type_filter .= '"' . $key . '"';
$type_filter needs to be defined (as an empty string) before it is used or it will trigger PHP notices for developers who have their PHP set up that way.

4. Undefined variable
drupal_goto($base_url . 'search/apachesolr_search/' . $keys, $filter);
$base_url is not defined at this point, you need to add global $base_url; to this function as well.

muhleder’s picture

Status: Needs review » Needs work

Setting to needs work for these 4 minor issues. Code looks good to me apart from that.

PA robot’s picture

Status: Needs work » Closed (won't fix)

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

I'm a robot and this is an automated message from Project Applications Scraper.

PA robot’s picture

Issue summary: View changes

changed git repository link

scresante’s picture

FYI:
Using Drupal 7, I had a tough time finding out how to integrate the advanced search dropdown into an apache solr search form. I figured porting this sandbox module would be overkill, and indeed there is an easier way.
What I did was I hooked
mymodule_form_apachesolr_search_custom_page_search_form_alter and added the advanced search from node.module but without the same logic checks. If you search node.module for 'use advanced search' you will find it.

Hopefully this is helpful to anyone trying to get advanced search on their apachesolr search form.

ccx105’s picture

Tried #13 but didn't make the advanced search from node work with apache solr.