There is a typo error in user search form that renders the same content without doing the search.

Patch provided.

CommentFileSizeAuthor
#18 search_c.patch1.68 KBtheborg
#17 search_b.patch2.36 KBtheborg
#13 search.patch1.14 KBchx
#9 user_search_c.patch859 bytestheborg
#4 user_search_b.patch819 bytestheborg
user_search_a.patch819 bytestheborg

Comments

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

trivial patch, tested, works, RTBC.

theborg’s picture

theborg commented

Related to this form also: http://drupal.org/node/208790

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Needs work

Hm, we use NULL not null. Also what does search_data() do there? There is not even a parameter at that position AFAIS:

in the patch:
drupal_get_form('search_form', url('admin/user/search'), $keys, 'user', null, search_data($keys, 'user'));

function signature:
function search_form(&$form_state, $action = '', $keys = '', $type = NULL, $prompt = NULL) {

theborg’s picture

theborg commented
StatusFileSize
new user_search_b.patch819 bytes

Corrected NULL typo.

As per form_api:
drupal_get_form can take optional additional arguments, which will be simply passed on to the $form builder function.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Yes, that doc refers to the arguments of the form builder I quoted above. How does search_data($keys, 'user') get there.

chx’s picture

chx commented

What Gabor says is that you are passing NULL for $prompt and then something else which has no matching parameter in the function signature.

theborg’s picture

theborg commented

Yes, I'm aware of that, sorry, I missunderstood documentation.

The keys end as a callback parameter to drupal_retrieve_form and search_form function

  // If $callback was returned by a hook_forms() implementation, call it.
  // Otherwise, call the function named after the form id.
  $form = call_user_func_array(isset($callback) ? $callback : $form_id, $args);

Sure there is a correct way of solving this bug.

chx’s picture

chx commented

SIGH! You are passing five (5) arguments to search_form which takes four (4) arguments aside from the form_state that's automatically added. The fifth argument is not going to be used. Got it?

theborg’s picture

theborg commented
Status: Needs work » Needs review
StatusFileSize
new user_search_c.patch859 bytes

Ok, got it, sorry for that mess.

This patch sets the action to null, so it will go to 'search/user'.

dvessel’s picture

dvessel commented
Title: User search form not working » Administrative user search form not working

Only affects the admin end.

The patch works but I don't think you need to check for keys. search_data will handle it just fine but I'm not sure if the returning nothing and appending to the string would cause a notice.

chx’s picture

chx commented
Status: Needs review » Needs work

If you pass in NULL then the action 

  if (!$action) {
    $action = url('search/'. $type);
  }

wil be search/user. This is not what you want. I agree with dvessel that passing in a NULL for search_data will work the function returns NULL very fast which PHP nicely casts to the empty string.

chx’s picture

chx commented

Why in the first place do we have this page? What does it do that search/user does not?

chx’s picture

chx commented
Status: Needs work » Needs review
StatusFileSize
new search.patch1.14 KB

catch said we nuked admin/content/search for it did nothing that search/node did not. Same for this page. Fixing the search form does not hurt, however.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Needs work

Nothing.

fwiw, admin/content/search was removed a couple of months ago for similar reasons.

chx’s picture

chx commented
Status: Needs work » Needs review

I guess catch crossposted my patch.

dvessel’s picture

dvessel commented
Status: Needs review » Needs work

Works just fine. The menu link still exists though. Thought it was cached in the menu system.. got it cleared and it's still there.

theborg’s picture

theborg commented
Status: Needs work » Needs review
StatusFileSize
new search_b.patch2.36 KB

Tested and works ok.

Removed the search link from the admin users menu.

theborg’s picture

theborg commented
Status: Needs review » Needs work
StatusFileSize
new search_c.patch1.68 KB

Edit: $form['#redirect'] = FALSE; causes /search path to not work.

theborg’s picture

theborg commented
Status: Needs work » Needs review
chx’s picture

chx commented
Status: Needs review » Reviewed & tested by the community

OK.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Fixed

Hm, all right. The argument sounds right that we removed the content search on the same grounds. Well, committed.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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