Term Search

Manual review:

- Enabled module, went to admin/config/search/settings and enabled Term Search under "Active search modules", got these warnings:

• Warning: Invalid argument supplied for foreach() in term_search_search_status() (line 151 of /Users/Rade/workspace/d7/sites/all/modules/custom/term_search/term_search.module).
• Warning: implode() [function.implode]: Invalid arguments passed in term_search_search_status() (line 156 of /Users/Rade/workspace/d7/sites/all/modules/custom/term_search/term_search.module).

-> You need to provide a default value for $vocabularies, e.g. array().

- Ran cron to re-index site, got these warnings:

• Invalid argument supplied for foreach() term_search.module:62
• implode(): Invalid arguments passed term_search.module:67

-> Same thing here, you need to provide a default value for $vocabularies if the variable is not set.

- You use db_query and db_query_range in many places. In Drupal 7 you should use db_select instead. Please see http://api.drupal.org/api/drupal/includes--database--database.inc/functi...
- Lines 108-109 are commented. If they are not needed they could probably be removed?
- PAReview returned 0 issues, very good!

Otherwise this seems like a nifty module! Please fix these issues and I will get back to you.

Cheers,
Rade

Great idea for a module.

Here are a few things in my experience of downloading and using it:

• I' not sure if this is my error or the modules, but I couldn't actually get it working. I managed to get it to index, but after I switched the default search module over to yours, after making a search I got an error "Error 310 (net::ERR_TOO_MANY_REDIRECTS): There were too many redirects.". I found no drupal_gotos or anything in your code though, so it might be unrelated, not sure.
• Also, I think believe the default drupal search uses H3 tags for its search results it looks like you are using H1's.
• This may just be me, but there seems to be a slight lack of inline comments that explain what is going on.
• When I search the site with the default node search module, I get a tab that allows me to switch to users. There isn't a tab that allows me to search for terms. To make a term search while not using the node search form I have to manually navigate to /search/term/whatever.

Keep up the hard work, I think once it's polished, this module will be a great addition.

Sam

You should post your review links to the issue summary, as stated in #1410826: [META] Review bonus.

Review of the 7.x-1.x branch:

• Drupal Code Sniffer has found some issues with your code (please check the Drupal coding standards).
  

  
  FILE: ...al-7/sites/all/modules/pareview_temp/test_candidate/term_search.install
  --------------------------------------------------------------------------------
  FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
  --------------------------------------------------------------------------------
   5 | ERROR | Additional blank line found at the end of doc comment
  --------------------------------------------------------------------------------
  
  
  FILE: ...pal-7/sites/all/modules/pareview_temp/test_candidate/term_search.module
  --------------------------------------------------------------------------------
  FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
  --------------------------------------------------------------------------------
   5 | ERROR | Additional blank line found at the end of doc comment
  --------------------------------------------------------------------------------
  

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Get a review bonus and we will come back to your application sooner.

manual review:

• project page is a bit short, see http://drupal.org/node/997024
• "$text = '<h1>' . check_plain($term->name) . '</h3>' . $term->rendered;": Opening h1 but closing h3?
• "$extra = module_invoke_all('term_update_index', $term);": if your module invokes hooks you should document them in a .api.php file.
• "'type' => check_plain($term->vocabulary_machine_name),": the vocabulary machine name cannot contain malicious characters, so check_plain() is not needed here.
• "'title' => $term->name,": a term name can contain evil characters, is check_plain() called anywhere else in the search module to catch this? I guess so, as the node_search_execute() implementation also does not sanitize this.
• "'link' => url('taxonomy/term/' . $item->sid, array('absolute' => TRUE)),": you should use entity_uri() as done for nodes in node_search_execute().
• term_search_search_status(): why do you always have to iterate over $vocabularies and create $vids? I think you could just use array_filter() to remove elements that are equivalent to FALSE.
• term_search_search_admin(): #options is not sanitized on checkboxes form elements. A vocabulary name is user provided input and can therefore cause XSS exploits. See http://drupal.org/node/28984

Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Sorry for the delay. Make sure to review more project applications and get a new review bonus and this will get finished faster.

Regarding the doc data types: the coding standards recommend using them for @param and @return. Example: "@param string name", "@return array". See http://drupal.org/node/1354#functions

manual review:
term_search_update_index(): no need to use check_plain() in this function as you are not printing anything to the user here.

But otherwise looks RTBC to me.

No objections for more that a week, so ...

Thanks for your contribution, scotthorn! Welcome to the community of project contributors on drupal.org.

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

As you continue to work on your module, keep in mind: Commit messages - providing history and credit and Release naming conventions.

Thanks to the dedicated reviewer(s) as well.