The FacetapiDependencyFacet::getActiveFacets() method parses the query string to get the active facet items. The problem with this approach is that it depends on the $_GET['f'] query string variable, which may or may not be where facet data is stored depending on which URL processor plugin is being used by the searcher. In addition, the FacetapiDependency parent class actually provides this information already via the $this->activeItems property. Therefore we can reduce code and eliminate hard coding $_GET['f'] in the application logic.

CommentFileSizeAuthor
#1 facetapi-1388666-1.patch2.48 KBcpliakas
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cpliakas’s picture

Status: Active » Needs review
FileSize
2.48 KB

The attached patch makes the suggested change.

danielnolde’s picture

Status: Needs review » Fixed

FIXED.
Ahh, great hint! This was exactly what i had hoped for: I saw the same problem in retrieving the active facets via the URL this way, and did it so just provisionally. I just didn't know about the correct way to do it, and hoped that someone would point me to it, once the module is public - so thanks again, Chris!

Is there any similar information available to simplify FacetapiDependencyFacet::getEnabledFacets(), too?

cpliakas’s picture

Thanks for applying. Regarding FacetapiDependencyFacet::getEnabledFacets(), I think you are using the correct API function in facetapi_get_enabled_facets(), which does most of the hard work. Well done! I posted an issue at #1389510: Simply the FacetapiDependencyFacet::getEnabledFacets() method discussing some other minor cleanups in this method, but overall I think it is fine.

Great work,
Chris

Status: Fixed » Closed (fixed)

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