Simply the FacetapiDependencyFacet::getEnabledFacets() method [#1389510]

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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

cpliakas CreditAttribution: cpliakas commented 30 December 2011 at 15:26

Priority:	Normal	» Minor
Status:	Active	» Needs review

File	Size
facetapi_bonus-1389510-1.patch	1.43 KB

Overall 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 CreditAttribution: cpliakas commented 30 December 2011 at 15:30

Another item I forgot to mention regarding the patch above... the dependency already gives you the facet in the $this->facet property, 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 CreditAttribution: danielnolde commented 2 January 2012 at 11:09

Status:

Needs review

» Fixed

Ah, 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 CreditAttribution: cpliakas commented 2 January 2012 at 15:39

Version:

» 7.x-1.0

Thanks for committing.

you have commit rights, too - do you want to do commits of reviewed patches directly for future issues?

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