Add a method that URL Processors can implement to alter the path of facet links

the corresponding issue in the Pretty Facets Paths for Search API module is #1213358: Move to Facet API

+1

Hello! I think I'm lost in big amount of related issues. Can anyone tell me, what is the status of pretty paths for Facet API?

Hi Chris, thanks for response.
I'm not sure I can help either if you can't provide good guidance now..
Do you expect to have some progress on task in upcoming 2 weeks? Thanks a lot for your work!

Hey, I'm interested in helping out with this and I can write code. I'm also pretty good at figuring out existing code. I will be able to dedicate significant time to this relatively soon (next week or the week after that). Would anyone who has been working on this give me an idea of where they left off and what the next step needs to be? What is the first question that needs to be answered in order to make this feature happen?

@vrc3: good to hear that. Where I got stuck was here #1226592: Make URL creation & resolving pluggable. I'm not sure it's clear from the issue there but I ran into a problem that the core of facetapi didn't allow for changing the path when I traced it back through the code (if I remember correctly). You may end up having to "get" how facetapi works and patching it in order to allow for URL modifications. That's what I remember. There's a very minimal implementation of the URL processor (pretty paths) under that issue that you could use as a starting point.

here is my first take on this.
note: it is still a very rough draft, so don't expect the following code to satisfy your pretty-facet-paths-expectations, yet :)

i have attached two patches:

facetapi-prettypaths-integration-draft.patch adds a postProcess function to FacetapiFacetProcessor which calls the new postProcessFacetBuild of FacetapiUrlProcessor. postProcessFacetBuild allows the url-processor to post-process the facet build object. In example, modify it's path or query parameters.

facetapi_pretty_paths-draft.patch is based on miiimooo's draft (http://drupal.org/node/1226592#comment-5126886). it will replace the default url processor with the custom FacetapiUrlProcessorPrettyPaths. within getQueryString i store pathSegments that are used in postProcessFacetBuild to modify the $item['#path'] of the facet build object in order to override the path.

i will continue to work on this next monday. of course i still need to implement the "inbound" mapping of the "vocabulary/term-name-tid" url parts that the prettypaths url processor currently produces.

generally i find it difficult to map vocabulary names / term names to ids as the "search_api_facets_paths" currently does, especially when you have special characters. so we are thinking of using the term id as an addition within the "pretty path". anyways the facetapi-prettypaths-integration-draft approach would allow for different url-processor plugins with optimized approaches.

let me know what you think

Great to hear there's some progress on this. Back in the Search API Pretty Paths module I used a pluggable path rewriting method. So a module could easily implement it's own re-writing method. Also my colleague came up with this two functions for path escaping (not sure if this of any use for you):

function search_api_facets_paths_escape($text) {
  return drupal_strtolower(str_replace(
    array("-", " ", "_"),
    array("--", "-", "---"), $text));
}
function search_api_facets_paths_unescape($text) {
  $text = preg_replace("/\b-\b/", " ", $text);
  $text = str_replace("---", "_", $text);
  return drupal_strtolower(str_replace("--", "-", $text));
}

Glad to see you guys are working on this. Thanks!

Back in the Search API Pretty Paths module I used a pluggable path rewriting method. So a module could easily implement it's own re-writing method.

Hi miiimooo,
Is it a code that you have already working? Can you share it?

This is awesome! I'm testing it out now on my sandbox site now. I'll let you know what I figure out over the weekend.

So, I've tested out the patches in comment #10 and got a few more things working. For my tests, I worked with a custom search page. I haven't tested out taxonomy pages yet, or the main search. And I suspect there may be other issues there (specifically with getting taxonomy pages with pretty paths to play nice with url aliases).

My changes were all in the url_processor_pretty_paths.inc file. First, I replaced $facet['field alias'] with $facet['field api name'] when calling getPathSegment so that the vocabulary and term names get set correctly. Then, I filled in fetchParams so that the facet params are filled in from the args instead of from $_GET. Finally, I unset $item['#query'] at the end of postProcessFacetBuild to remove the un-pretty query string.

Patch is attached. It should be applied after applying the patches in comment #10. (This is the first time I've posted a patch here, so if I did something wrong or there's another convention as far as creating the patch with or without previous patches applied, please let me know.)

One fairly big issue is that, with these changes, the search keys will include the whole pretty path. For instance, if I do a search for "chicken" and one of the facets is "garlic", the pretty path on that facet may look like "http://www.example.com/recipe-search/chicken/Ingredients/garlic-7158". But when you click that, the search key changes from "chicken" to "chicken/Ingredients/garlic-7158". As far as I can tell, this is because the arguments in the page callback for apachesolr_search_custom_page (and other similar callbacks) are greedy. I'm not quite sure whether this can be altered from a menu_alter hook, or whether we need to propose some changes to the apachesolr module as well, in order to make pretty paths work. Thoughts?

thanks for the feedback everyone!

i have been working on this today and will continue tomorrow. you can find the current state in my sandbox:
http://drupal.org/sandbox/dasjo/1430182

the facet global configuration dialog now has an option to set a pretty_path_alias
Pretty paths will be generated in the following way: "/". For taxonomy terms it outputs the id as well: "/-".
http://drupalcode.org/sandbox/dasjo/1430182.git/blob/62338e05d022e152ba0...

this works, also i incorporated vrc3's work above to make fetchParams interpret the values correctly
http://drupalcode.org/sandbox/dasjo/1430182.git/blob/62338e05d022e152ba0...

sounds good, but my problem is that while fetchParams does the same as for FacetapiUrlProcessorStandard, somehow the searches aren't filtered, yet.

cpliakas, do you know if i'm missing something here?

thanks for reviewing, i'm happy to hear your suggestions.

sry doublepost

One comment at this early stage, would it be possible to move the $this->processors[$facet['name']]->postProcess(); statement into the FacetapiFacetProcessor::process() method right after the $this->processQueryStrings($this->build); statement on line 1133? This would prevent the postProcessFacetBuild() hook from firing when there are no items, and more importantly when the facet failed its dependencies. Let me know your thoughts on this.

cpliakas, i'm with your suggestion in #11. actually i started with the process method right after processQueryStrings :) but during testing i think i switched it because i was struggling with the getQueryString logic. the attached patch should be the way to go now.

also thanks for the input miiimooo in #12. i decided to include term-ids as we have lots of terms which include special characters, escaping them in a reversable way would be quite challenging / not produce nice looking urls anyways :)

some notes by vrc3 from #15 are still valid, so we have to check this out, but now i'm trying to get the actual filtering working. it might also be due to implications with using search_api + facetapi ...

i was able to fix the problem described in #16 and committed it to the sandbox.

As far as getting the search path, if you added a condition to the loop in fetchParams that exits after the last valid pair of vocab name (or alias) and term names are found, you could take the next argument popped off the array as the search key. But this doesn't do you any good because by the time fetchParams is run, the callback in apachesolr has already grabbed the search keys.

vrc3, thanks for reviewing!

dasjo, at first glance at your fetchParams code, you have three array_pops in your loop. From what I understand of the new code you have in postProcessFacetBuild, there are still only two slashes added.

fetchParams contains the same amount (3) of array_pops as your patch does. only 2 affect the $args array, while the 3rd is used to get the term-id array_pop(explode('-', $value).

Also, the new clean path includes the vocab id instead of the term id. The params that get returned by fetchParams needs the term id not the vocab id.

thanks i had mixed vid with tid, fixed :)

As far as getting the search path, if you added a condition to the loop in fetchParams that exits after the last valid pair of vocab name (or alias) and term names are found, you could take the next argument popped off the array as the search key. But this doesn't do you any good because by the time fetchParams is run, the callback in apachesolr has already grabbed the search keys.

yeah, not sure here, yet. actually i was thinking that we might need to load the view to get it's search path in order to make sure we don't try to interpret parts of the view-path as pretty facet params...

Yeah, I noticed that I got mixed up on the array pops. I'm debugging it now.

dasjo,

To get filtering to work, change

$params['f'][] = $facet['field api name'] . ':' . $value;

to

$params['f'][] = $facet_alias . ':' . $value;

in fetchParams

Also, even though FacetapiUrlProcessorStandard::getQueryString() and the previous version of FacetapiUrlProcessorPrettyPaths::getQueryString() only differ by one line, that line is necessary, because otherwise the filters for the pretty paths are not combined. Perhaps that line shouldn't belong in that function, but this change needs to be somewhere.

Here's one patch that combines what has been done so far and gets the filtering working again.

If you add this line:

$keys = array_shift(explode("/", $keys));

to the beginning of apachesolr_search_custom_page in apachesolr_search.pages.inc (in the apachesolr module) you should see the filtering working for custom search pages. Add the same line to the beginning of apachesolr_search_search_execute to see it working on core search pages. I don't necessarily advocate this as the best permanent solution for the search keys problem (it will break search queries that include slashes, but that may be unavoidable), but it makes the patch I posted testable. And that's a good thing.

thanks again for your input, vrc3

i have rewritten postProcessFacetBuild and added the lost getQueryString (thanks for mentioning) so selecting / deselecting facets works for me so far.

upcoming:
- fix current search block links
- don't loose query params, for example with fulltext search combined

maybe:
- make some parts pluggable?

Hey people,

it is very hard to follow your workflow here. Are you working with the sandbox or do you want to move the facetapi_pretty_paths in the facetapi module? If so, please provide valid patches for the facetapi project. If you are working in a sandbox, you should consider moving this issue to the sandbox and make issues in the main projects for follow ups if necessary?

I wanted to test this but I could not figure out how to use the latest patch

hi nick_vh, that's a good point. i'm working on the sandbox right now, but i think this could ship with facetapi once its stable.
cpliakas would have to decide anyways, currently its work in progress, so if you prefer we can defer discussion to an issue at my sandbox.

I do like the activeness of this topic and would be sad to see it go lost in space. Would a separate branch in facetapi be a solution?

i have enhanced the Facet API Pretty Paths further.

this currently is a separate module which depends on facetapi with the integration patch posted in #18

what works:
- enable the Facet API Pretty Paths will override all facetapi paths (it might be good to have options for this - globally on/off, per facet?)
- Pretty paths will be generated as "search/url/segment1/segment2/".
- By default, a segment will look like: "<alias>/<value>".
- For taxonomy terms it outputs the id as well: "<alias>/<term-name>-<term-id>". (there are some @todo notes in the code where i believe we could make this pluggable)
- selecting / deselecting facets will produce pretty paths and works also with hierarchical taxonomy facets for me so far
- other parameters than "f" remain in the url, so a combined fulltext search will still work

what doesn't:
- current search block links aren't covered, as the current_search isn't aware of the url_processor, yet
- more? we might want to have tests?

here is a new version for the integration patch without the confusing legacy code within a comments block.

so this patch + the Facet API Pretty Paths sandbox module together provide full functionality available so far and described in my above comment.

sounds like a plan, so let's focus on the integration part here.

i have promoted the Facet API Pretty Paths module to a full project!
there we can have a #1431700: General discussion & make this work :)

chris, what do you think about #1431676: Integrate with facetapi current_search module?

I agree, that the workflow is getting confusing for me too. I haven't really contributed much code before, so I don't know what the conventions are for collaborating on something like this. But it seems I don't have permissions to commit changes to someone else's git sandbox. So posting patches seemed like the best way for me to contribute code.

Did the patch not work for you? I made it to combine everything that has been done so far into one patch that creates a partially working pretty paths. It is meant to be applied to the latest dev release of facetapi.

EDIT: Okay, now I understand that this will now be a separate module....

hi vrc3, I'm sorry about the confusion. I didn't apply your patches directly as you had a different directory structure than I did with the custom module from the beginning. but anyways I think I integrated most functionality you proposed by porting your code manually. personally I find it easier working in the Git repository directly, especially when code is developing quickly. if you want I'd be happy to collaborate more, maybe I can
get you Git access but let's better discuss at the module issue queue

hi all,

i have continued to work on the Facet API Pretty Paths module.

when rewriting the logic to calculate a real basePath and use it, i noticed that our assumptions in #11 and #18 where wrong.
we need to process all facets before we can postProcess them. note: i'm currently calculating all $pathSegments in getQueryString which runs within the process function for the facets.

this means, my last patch from #30 doesn't work with the recent pretty paths solution and i'm using the original patch from #10 again.

if you know, how to better handle this, please let me know :)

upcoming: #1431676: Integrate with facetapi current_search module

more updates here: i have updated the integration patch.

my attempt is to allow different "types" of postProcessing (facet, current_search, ...)
http://drupalcode.org/project/facetapi_pretty_paths.git/blob/d10935d:/pl...
so i have created #1441302: Current search integration for FacetapiUrlProcessor postProcessing to alter paths

it works for me with search_api. you may test it with the recent codebase of Facet API Pretty Paths. let me know, what you think.

hi chris,

at the time, getFacetPath is called - do i have access to some object structure where the facet items are stored hierarchically? - like you have with $item['#item_children'] in the later generated $build array that i'm using right now.

holy cow,

i figured out #39 and just rewrote the facetapi pretty paths module using the getFacetPath approach.

seems like my approach so far was just too complicated, so i kicked out all the ugly post processing logic and imitate the getQueryString behavior for getFacetPath :)

thanks for pointing this out, chis!

the attached patch is #38 + some doc fixes

One more curiosity about the pretty paths integration is how to correctly handle the "base path".

I have a hackish implementation in fetchParams as you can see from the following, abbreviated code snippet:

  public function fetchParams() {
...
    // Pretty paths are expected to be in form "search/url/segment1/segment2/".
    $args = explode('/', $_GET['q']);
    // Skip pretty paths logic for admin pages, as
    // for pretty paths, we have to manipulate $_GET['q].
    // @todo: Fix this workaround?
    if ($args[0] == 'admin') {
      return $_GET;
    }
...
    // Traverse all segments "<alias>/<value>" from right to left (array_pop).
    while ($segment['value'] = array_pop($args)) {
      $segment['alias'] = array_pop($args);
...
    }
...
    // Mock default parameters for facetapi & related search modules.
    if (!empty($params)) {
      // Set search base path.
      $_GET['q'] = implode('/', $args);
...
    }
    return $_GET;
  }

Chris or maybe somebody else with some insights into facetapi & the related search modules,
can you think of a clean way to determine the correct base path for an arbitrary pretty search path like "search/url/alias1/val1/alias2/val2"?

(we have discussed it already a bit here #1434462: ApacheSolr module integration)

yeah would be great if we would RTBC-mark #40 + #1441302: Current search integration for FacetapiUrlProcessor postProcessing to alter paths

you mentioned, you are planning to do a new facetapi alpha release soon?
would be great to have these integration points in for releasing a first alpha of facetapi pretty paths

the patch from #45 works for me.

regarding the search path discussion in #41 #42 #43:
thanks for pointing that out chris, i have added some comments regarding this at http://drupal.org/node/1434462#comment-5619338

my bad, wasn't aware that it was already backported :)