During the progress of making apachesolr work with facetapi_pretty_paths I've ran into some deeply nested core issues.
It seems that drupal_url_encode does not encode the slashes because of aestetic reasons, which is imho bullshit.

So, this patch allows apachesolr to work together with facetapi_pretty_paths. I do have to clean it up and make sure that an empty search page also works (not the case yet)

Comments

nick_vh’s picture

StatusFileSize
new4.26 KB
nick_vh’s picture

More info and use can be found here #1434462-19: ApacheSolr module integration

nick_vh’s picture

Status: Active » Needs work
+++ b/apachesolr_search.moduleundefined
@@ -1486,11 +1491,20 @@ function theme_apachesolr_search_suggestions($variables) {
+     $form_state['redirect'] = str_replace($fv['search_block_form'], $raw_keys, $form_state['redirect']);

Some spacing issue that should be fixed. Another problem that needs to be fixed is whenever a search page without a query suddenly gets the facet information in the url

search/site
search/site/facet/value

it will take facet as the query string value. We should actively prevent this from happening somehow.

nick_vh’s picture

Status: Needs work » Needs review
StatusFileSize
new6.49 KB

This patch should work natively with latest dev of facetapi_pretty_paths. It changes the getPath parameter to add an extra slash, so we always know if there is a query or not. This is due to the fact that we do not want to handle special cases.

nick_vh’s picture

Issue tags: +RC blocker

Adding tag

swentel’s picture

Status: Needs review » Needs work

Did a quick test, here's what I found:

  • Triggering the first facet did the job (eg routes/overzicht//route-type/16), however, when I triggered a second facet (eg routes/overzicht//is_length/2/route-type/16), the is_length is not used in the query afaics, and it's also not toggled in the facet. Only the last one is checked (using checkboxes).
  • If you trigger more than one value within one facet (so facet configure as OR - eg route-type), the URL becomes something like routes/overzicht//route-type/16/route-type/19 and no filtering is happening. Not sure whether we can get something like 'route-type/16/19' and which module this problem is.
  • You need to watch out for the alias. I had a solr field called im_field_route_type which I aliased to 'type' - that's probably clashing with an actual solr field called type. Not sure whether this is a pretty paths problem or solr, and if it's even possible to 'fix' it. My guess is not, so make sure we document it. When I aliased it to 'route-type' it worked out fine.

Can't give more pointers or debugging at this point, will try more tomorrow.

nick_vh’s picture

Make sure you also use #1648582: Multiple values or hierarchies doesn't work

How the facets are ordered, that is up to the facetapi pretty paths module. The alias problem should be fixed with patch from above.
https://skitch.com/nick_vh/end3y/search-smartphone-compare shows that multiple paths are actually working?

swentel’s picture

Status: Needs work » Needs review

Hmm ok, that makes it all work, setting back to needs review to get hopefully more eyes on it :)

nick_vh’s picture

Status: Needs review » Reviewed & tested by the community

I clicked through all possible filters and widgets I could find and could not catch any error.

The only disadvantage is the extra slash that is being added sometimes, but this generally does not harm the search pages nor results. Let's commit this as last one and roll an RC

nick_vh’s picture

Version: 7.x-1.x-dev » 6.x-3.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

committed to 7.x-1.x

killua99’s picture

Status: Patch (to be ported) » Needs review
StatusFileSize
new6.81 KB
+++ b/apachesolr_search.module
@@ -180,13 +181,13 @@ function apachesolr_search_menu_alter(&$items) {
     if ($core_search) {
-      $items[$search_page['search_path'] . '/%menu_tail']['tab_root'] = 'search/' . $default_info['path'] . '/%';
-      $items[$search_page['search_path'] . '/%menu_tail']['tab_parent'] = 'search/' . $default_info['path'];
+      $items[$search_page['search_path'] . '/%']['tab_root'] = 'search/' . $default_info['path'] . '/%';
+      $items[$search_page['search_path'] . '/%']['tab_parent'] = 'search/' . $default_info['path'];
     }

This lines in 6.x-3.x are commented, So I keep it commented. Correct me if I did wrong.

Status: Needs review » Needs work

The last submitted patch, 1642140-11.patch, failed testing.

killua99’s picture

Status: Needs work » Needs review
StatusFileSize
new6.84 KB

I miss the details about variable_get() second argument.

Try it again.

Status: Needs review » Needs work

The last submitted patch, 1642140-13.patch, failed testing.

jamesharv’s picture

Version: 6.x-3.x-dev » 7.x-1.0-rc2

I just upgraded to 7.x-1.0-rc2 and I think this change has caused a problem with normal searches from search_block_form.

http://drupalcode.org/project/apachesolr.git/commitdiff/8e3c40a1b977e21c...

When I do a search the keywords get urlencoded twice. So a search for 'test search' ends up redirecting to '/search/site/test%2520search' and a search is performed for 'test%20search'.

I think the problem comes from the code below. It seems that the comment 'Replace keys with their rawurldecoded value' is not what is actually happening. The keys are actually being rawurlencoded.

// Replace keys with their rawurldecoded value
if (isset($fv['search_block_form'])) {
  $raw_keys = rawurlencode($fv['search_block_form']);
  $form_state['redirect'] = str_replace($fv['search_block_form'], $raw_keys, $form_state['redirect']);
}
nick_vh’s picture

I was expecting some problems like this to appear. In my test environment this does not happen, therefor it is important that you list the steps I need to take so I can replicate the problem.

A good test is to search for test/test and it encodes the slash and decodes it in the search. Naturally, your test with "search test" should work also!

It could also be that you have modified your search block or that your search block has a different form id, can you check that?

nick_vh’s picture