Doesn't work with Facetapi Pretty Paths // needs improved logic to get path

Hi guys,

I have been testing the latest dev and updated, particularly the commits: 08c2c8e and 19f1b53 (August 22, 2012) and I encountered a few issues.

(1)Firstly when working on my localhost, I have a base_path which is most likely longer than on production:
http://localhost/mysite1
instead of http://mysite1.com , which I would assume most likely works fine.
So I'm thinking perhaps the base_path should be stripped off from the request_uri call (supposed to return $path).

(2)Secondly, related with ticket:
http://drupal.org/node/1764886
with language prefix there is also a problem: prefix is doubled in URL and the patch submitted on ticket probably needs more work since as noted by morningtime in note#3 there are issues caused by the call to current_path()). Besides, overriding the $path with current_path in the theme function kind of defeats the purpose of the latest commits (see August 22 2012).
One solution to that could be to strip off the language prefix from the request_uri/$path, as in the first point.

(3)Lastly, it seems that when Facetapi Pretty Paths is not enabled, the search arguments f[0], etc... are encoded in the fragment: ?f[0]=....
localhost/mysite/en/views1?f[0]=field1:182

When Facetapi Pretty Paths *is* enabled the path then becomes something like:
localhost/mysite/en/views1/field1/182
and the fragment ?f[0]=field1:182 is not needed anymore

The bug I encountered is that after enabling Facetapi Pretty Paths, the path of the links created on the sort links are a combination of both:
localhost/mysite/en/views1/field1/182?sort=created&order=desc&f[0]=field1:182

the problem is that the f[0]=... shouldn't be there, since it's already passed in the pretty path.

I have tried looking around and see if I could find any work around or solution, but so far, I haven't been able to find any good solution to that last point.

For the first 2, I found myself adding call to str_replace, which seems to work, but I didn't have enough time yet to find any potential solution for 3.

Feel free to let me know if you have any ideas how this feature could be fixed, since I think it would be great to have compatibility with Facetapi Pretty Paths.

I'm looking forward to getting your feedback and comments.
Thanks in advance.

Hi morningtime,

I'm glad to be able to follow up on this ticket and thanks again for all your help and feedback in the facetapi_pretty_paths tracker:
http://drupal.org/node/1661552
As you may have noticed, I would like to help you move forward on this, therefore, I have done a little bit of work here that I hope will simplify your task.

Pretty much, below, I'm applying the fixes to the latest dev: 7.x-1.2+4-dev.

Alright, following up with the post, you'll find attached an attempt (File named: sort_pretty_paths-11.patch) to tackle this issue for good with the method you suggested earlier (see above).
From what I saw, there are 2 cases, either facetapi_pretty_paths is enabled or it's not.

In case it's not, I thought we could align the module's $path (line 174 .module file) on the latest change from search_api in the facet adapter:
http://drupalcode.org/project/search_api.git/blobdiff/589b8a01c70534ca24...

In case it is, then, we've got the code from: facetapi_pretty_paths
Hi morningtime,

I'm glad to be able to follow up on this ticket and thanks again for all your help and feedback in the facetapi_pretty_paths tracker:
http://drupal.org/node/1661552

Pretty much, below, I'm applying the fixes to the latest dev: 7.x-1.2+4-dev.

Alright, following up with the post, you'll find attached an attempt to tackle this issue for good with the method you suggested earlier (see above).
From what I saw, there are 2 cases, either facetapi_pretty_paths is enabled or it's not.

In case it's not, I thought we could align the module's $path (line 174 .module file) on the latest change from search_api in the facet adapter:
http://drupalcode.org/project/search_api.git/blobdiff/589b8a01c70534ca24...

In case it is, then, we've got the code from: facetapi_pretty_paths to retrieve the path.

  //Override the path if facetapi_pretty_paths is enabled
  if (module_exists('facetapi_pretty_paths')) {  
	  // Get the facet api adapter by the machine readable name of searcher.
	  $adapter = facetapi_adapter_load('search_api@'.$index['id']);
	  // Get the url processor and check if it uses pretty paths.
	  $urlProcessor = $adapter->getUrlProcessor();
	  if ($urlProcessor instanceof FacetapiUrlProcessorPrettyPaths) {
		// Retrieve the full path from the url processor.
		$path = $urlProcessor->getFullPath();
	  }
  }

Could you please help me test this patch guys?
I have already tested all these cases, with/without language prefix, with/without folder on localhost or on other types of dev environments, it really seems like everything is now in order.
But I would really be glad to hear your feedbacks after testing and double checking.

Also, I have the feeling the line:
$adapter = facetapi_adapter_load('search_api@'.$index['id']);
could be even further improved, since I got the 'search_api@' part hard coded in there, but I haven't really looked at how this could potentially be obtained dynamically with a function, for example, if there is any at all.

I would really greatly appreciate your feedback on this, and ideally, a module roll out, so that we're able to move forward smoothly and safely with this issue.
Thanks in advance for your help, feedback and advice, really highly appreciated.

I always forget to update the status ....

looks good to me - i'm happy you folks are getting along with this solution!

just wanted to mention that you should stick to the coding standards for future patches, see http://drupal.org/coding-standards
there is a great tool for reviewing patches and more: http://drupal.org/project/dreditor

Once again, thank you so much for your valuable input, dasjo.
We're also glad to see this module working now with the facet pretty paths module, which features are very interesting as well.

I have installed/configured dreditor and now I have a new tab on the patch files in the tracker when browsing with Chrome:
Review
It's really cool and at the same time I refreshed a bit my memory on coding standards.

Please find attached a slightly cleaner patch according to coding standards (indentation, whitespace at end of line, etc...).
[File named: sort_pretty_paths-15.patch]

Feel free to let me know guys is there would be anything else to do with this patch/ticket, I would be glad to revise the patch again.

Otherwise, it would be great to see this in the dev version soon.

Thanks in advance!