CVS edit link for ConnorK

I want to contribute a new module that I call "Adjusti-Search". It creates a configurable block that can be used to integrate all searches together. The module doesn't provide any actual search abilities by itself. Instead, it takes the URLs for all searches you want integrated (within Drupal and external to your site) into the search block and allows users to select which search they want to use. The search string that a user enters is encoded within the URL that corresponds to the search they selected and the page is redirected to that search.

Details about formatting URLs to be used with this module can be found in the help section of the module.

CommentFileSizeAuthor
#1 adjustisearch.zip5.76 KBConnorK
#5 adjustisearch.zip5.58 KBConnorK

Comments

ConnorK’s picture

Issue tags: -Module review
StatusFileSize
new5.76 KB

Here is the actual module.

The actual code for the module can be found in Comment #5.

Please review.

avpaderno’s picture

Status: Postponed (maintainer needs more info) » Needs review
Issue tags: +Module review
dawehner’s picture

    $block['subject'] = variable_get('adjustisearch_block_title', 'Search:');

Verry minor, but you should do 'Search:' translateable.

adjustisearch_form_builder

Thats just a comment, you don't use any lines together with the two different formIds. Why not just use two form functions for the two forms.
Then you could save hook_forms.

avpaderno’s picture

Status: Needs review » Needs work
  1. Use the Drupal string functions when available.
  2.   $form_state['values']['adjustisearch_block_title'] = check_plain($form_state['values']['adjustisearch_block_title']);
      $form_state['values']['adjustisearch_url_list'] = check_plain($form_state['values']['adjustisearch_url_list']);
      $form_state['values']['adjustisearch_box_size'] = check_plain($form_state['values']['adjustisearch_box_size']);
    

    A validate function should only validate the data, not modify it. Then, the data are filtered in output, not in input; differently, users would see & altered in &, when they change the settings for the second time.

ConnorK’s picture

Status: Needs work » Needs review
StatusFileSize
new5.58 KB

Attached is the module with the above comments taken into account.

Changes
--------

1. Separated form construction, thus eliminating the hook_form() (Thanks for pointing that out dereine).
2. Made instance of string "Search:" translatable (Thanks again dereine).
3. Replaced PHP string functions with Drupal string functions when possible.
4. Dropped the portions of the validate function that were unnecessary (Thanks for pointing that out KiamLaLuno).

avpaderno’s picture

Status: Needs review » Fixed
Issue tags: +Module review
    '#description'   => t("Enter the URL search list, with one URL per line. These URLs must be entered in <i>Adjusti-Search URL Format (ASUF)</i>, which can be found on the <a href='../help/adjustisearch' target='_blank'>Adjusti-Search help page</a>. Any URL not in <i>ASUF</i> will be ignored."),

I am not sure the relative URL would work; what should it point to (../help/adjustisearch)?

The rest is fine.

ConnorK’s picture

Fixed. Re-encoded the link using url(). Will be in the final version of this release.

Status: Fixed » Closed (fixed)
Issue tags: -Module review

Automatically closed -- issue fixed for 2 weeks with no activity.

avpaderno’s picture

Component: Miscellaneous » new project application
Assigned: Unassigned » avpaderno
Issue summary: View changes
Status: Closed (fixed) » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.