The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Over at #865510: Add project_solr_browse_projects_form as a block on the main search page when a project type facet is selected it became very clear that the project_solr_browse_projects_form() makes a bunch of assumptions that cause lots of grief in drupalorg_search.module when trying to reuse this form in a block on the main solr search page. We're currently doing lots of ugly, fragile hacks in form_alter() and a 2nd submit handler. We should clean this mess up and make project_solr_browse_projects_form() more sane in the first place, so we don't have to jump through so many hoops in drupalorg_search to be able to reuse it.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 875438-3.project_solr.patch | 1.07 KB | dww |
| #3 | 875438-3.project_solr.patch | 1.07 KB | dww |
| #3 | 875438-3.drupalorg_search.patch | 2.89 KB | dww |
Comments
Comment #1
dwwtagging for sprint #2
Comment #2
dwwThanks to #428292: Download & Extend Landing Page: Filter by version and #880598: Search sorts don't function as expected in project browse navigation (search pages only) this code is a lot better already. Really the main thing that still sucks is the manner by which
$form_state['redirect']is being tweaked. csevb10 and I discussed this at some length, and there's not a ton we can do. The main problems are:A) This line from drupalorg_search_project_solr_browse_project_form_submit() is really cryptic:
because it's not obvious wtf
$form_state['redirect'][1]is about. The main submit handler sets form_state['redirect'] to an array for drupal_goto(), the 2nd element is the query array for the redirect. It seems like it'd be nice to use named array keys for this (both in the main submit and in the other submit callbacks) so that it's more self-documenting.B) That same line is a bit cryptic because the use of explode()/implode() isn't particularly self-evident. We're doing it to handle the case where filters is currently empty. This could use a comment.
C) The same line is also rather fragile since it depends on the main submit handler happening first. I'm not sure what we should do about that. csevb10 suggested that we specifically unset the main submit handler when we alter this form in drupalorg_search, and then call it directly ourselves at the right moment, so we know the other handler is happening and $form_state has the state we expect. I'm not sure this is a good idea or not. ;)
Anyway, I'm going to roll a patch for at least A and B...
Comment #3
dwwdrumm suggested we just punt on (C). Effectively, that means if someone breaks this chain of submit handlers, we'll probably end up with a fatal PHP error as we walk off the end of an index in
$form_state['redirect']that doesn't exist. So, here are patches for (A) and (B). Untested.Comment #5
dwwheh, whoops. silly bot. wrong project. i guess there's no way it could know that. here's just he patch for project to fool it.
Comment #6
csevb10 commentedEverything works as expected. Tested locally and all posts worked as intended.
Comment #7
dwwCommitted to HEAD of project and drupalorg. Merged the project changes into BZR. The drupalorg changes are happening automatically. Should be live on redesign.d.o within a minute or 2.