While search keys work fine when they are set on the path (like it's done with core search and all modules that use it), they fail, e.g., for search views, which pass the search keys in a GET parameter – in that case, the breadcrumb link leads to an empty search. The attached patch should fix this.

CommentFileSizeAuthor
#8 Screen shot 2011-07-07 at 7.42.30 AM.png46.87 KBcpliakas
facetapi--keys-breadcrumb.patch998 bytesdrunken monkey

Comments

cpliakas’s picture

cpliakas commented

I see what you are getting at. I think this is why there is an @todo to add a getBaseQuery() method in the adapter, but your solution might eliminate the need for this and may be a more elegant. I would be interested in getting Peter's feedback, because he understands the breadcrumb code more than I do :-)

Thanks for contributing,
Chris

cpliakas’s picture

cpliakas commented

Thomas,

Just for my benefit so I can write some tests, where do the search keys come from in Search API, or is the location configurable?

~Chris

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

In search pages, they are in the path, just like in "normal" core searches, but when creating search views they are handed in GET parameters just like all other values for exposed Views filters. The concrete parameter is thus configurable, as it is for exposed Views filters in general. But I don't think the actual GET parameter matters for the code, as long as it's not "q" or "page" (which would of course be insane anyways).

cpliakas’s picture

cpliakas commented

I gotcha. Thanks for the context. Short summary, Facet API should recognize search keys in any $_GET variable (minus the notable exceptions, of course).

pwolanin’s picture

pwolanin commented

That looks like an elegant patch, since we know the keys will be some place in the 'q' or query string.

cpliakas’s picture

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

Thanks for reviewing, Peter.

cpliakas’s picture

cpliakas commented

Added supporting tests in commit 1bae5a1. Without the patch the tests fail, with the patch they succeed.

cpliakas’s picture

cpliakas commented
Status: Reviewed & tested by the community » Fixed
StatusFileSize
new Screen shot 2011-07-07 at 7.42.30 AM.png46.87 KB

Committed patch at 315f22c. We have a new committer!

Status: Fixed » Closed (fixed)

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