Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Based on the discussion at http://drupal.org/node/1388666#comment-5414652, we can discuss ways to simplify the FacetapiDependencyFacet::getEnabledFacets() in this thread.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | facetapi_bonus-1389510-1.patch | 1.43 KB | cpliakas |
Comments
Comment #1
cpliakas commentedOverall the FacetapiDependencyFacet::getEnabledFacets() uses the correct API functions to gather the enabled facets. There are a few very minor items that could be cleaned up, and I would recommend getting the searcher via the adapter as opposed to the form (although it is a perfectly sane approach). See the attached patch for the idea.
Comment #2
cpliakas commentedAnother item I forgot to mention regarding the patch above... the dependency already gives you the facet in the
$this->facetproperty, so we can use that instead of getting the name from the form. Again a minor item because the form approach is sane, just adds some consistency and code clarity.Comment #3
danielnolde commentedAh, niccce! That's exactly what i had in mind - using all the indirect data from the form was a clumsy workaround for not knowing the proper way to access or get the data directly - thanks, Chris!
reviewed, tested and committed (ahh - you have commit rights, too - do you want to do commits of reviewed patches directly for future issues?)
Comment #4
cpliakas commentedThanks for committing.
That's OK. I want you to drive this module as much as possible. Now that your application got accepted, I will definitely be less active in the queue and look forward to see where you take this module. Of course I am always available for advice on the inner workings of Facet API if you want to get a second opinion on something.
Thanks,
Chris