Facet API Bonus
Drupal 7 module
http://drupal.org/sandbox/daniel.nolde/1387636
git clone http://git.drupal.org/sandbox/daniel.nolde/1387636.git facet_api_bonus
---------------
Facet API Bonus for Drupal 7 is a collection of additional Facet API plugins and functionality, foremost filter and dependency plugins – And a place to collect more additional Facet API extensions.
Currently Facet API Bonus includes:
- Facet Dependency: Dependency plugin to make one facet (say "product category") to show up depending on other facets or specific facet items being active (say "content type" is "product" or "service"). Very flexible, supports multiple facets to be dependencies, as well as regexp for specifying facet item dependencies, as well as option how to behave if a dependency is being lost.
- Exclude Items: Filter plugin to exclude certain facet items by their markup/title or internal value (say excluding "page" from "content types"). Regexp are also possible.
- Rewrite Items: Filter plugin to rewrite labels or internals of the facet items via a admin-configurable callback function (in a structured array, before rendering). Very handy to rewrite list field values or totally custom encoded facet values for user friendly output.
Further additions are very welcome!
Facet API Bonus is written for Drupal 7, and is stable, tested, and ready to be used in production environments.
Requirements:
- Facet API is obviously required, as well as a compatible search module (e.g. apachesolr, search_api).
Similar projects:
- http://drupal.org/project/facetapi_extra, created at the same time, seems to aim for the same goal, containing but one smaller dependency plugin at the moment. Facet API Bonus offers more different plugins for more use cases, the plugins in Facet API Bonus are richer in function and number at the moment. Maintainers will later discuss further distinction or merge.
---------------
My earlier application for full project access had been stuck in the cvs>git transition, hopefully this one will work ;)
Comments
Comment #1
jinlong CreditAttribution: jinlong commentedIt appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.
Review of the master branch:
This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Go and review some other project applications, so we can get back to yours sooner.
Source: http://ventral.org/pareview - PAReview.sh online service
Comment #2
cpliakas CreditAttribution: cpliakas commentedA point of clarification against the review in #1...
All of these files DO contain classes that need to be autoloaded, so the API is implemented correctly in this instance and the module would be broken if these directives were removed.
I installed the module and played around with the internals for a while. I did find a couple of minor bugs that are posted against the issue queue and listed below:
#1388644: Do not explicitly return TRUE in the FacetapiDependencyFacet::execute() method
#1388666: Remove the FacetapiDependencyFacet::getActiveFacets() method in favor of the FacetapiDependency::activeItems property
#1388702: Facet labels are double-translated in FacetapiDependencyFacet::getEnabledFacets()
#1388750: Notice errors when "Exclude specified items" filter is active but the list is empty
Going down the How to review Full Project applications list for new modules, my comments are as follows:
Module description and uniqueness
The module's goals and description are clearly defined on the project page. This could be copied to the README.txt file as-is. In addition, it explains how it is different from the Facet API Extra module. My only comment here is that there is a piece of overlapping functionality in the "Another Facet" dependency that could be posted to Facet API Extra as a feature request against the "Parent" dependency where the two pieces could then be merged together.
Licensing
There are no violations of the GPL.
Security
The module correctly uses API functions such as
check_plain()
to sanitize user generated data. The only area of concern is the "Rewrite facet items via callback function" filter where you can specify a function though a textfield to alter the facet items. Ideally a list of available callbacks would be provided by a hook to whitelist which ones could be used to avoid giving site builders the ability to invoke any available function. See #1388804: Pull callbacks from a hook as opposed to allowing users to specify a function name through a textfield for more info.Best practices and coding standards
The module adheres to programming best practices and also adheres to Drupal coding standards minus a few minor issues. Again, it is better than most of the modules on Drupal.org and shows an understanding of the guidelines. Reviewing the Module documentation guidelines page, there is no README.txt file as mentioned in #1 and there is also no hook_help() implementation. In this instance I would say this is OK and
hook_help()
would actually be overkill since most of the usage and UI is inherited from Facet API. There are good labels and descriptions in the UI that explain how to use the various filters and dependencies.Conclusion
Overall I feel there is no major reason to deny this application. There are a few issues to resolve, mainly the security issue, but it shouldn't prevent this process from moving forward. I do think that the "Another Facet" dependency should be submitted to Facet API extra and removed from this project. Then to be more descriptive of what this module accomplished, I would propose renaming it to "Facet API Advanced Filters" to narrow the scope and not conflict with the purpose of Facet API Extra. Let's discuss, then I would support this issue being marked as RTBC.
Comment #3
danielnolde CreditAttribution: danielnolde commentedThanks for the prompt and rich feedback, guys!!
Fixed:
No fix needed:
Needs discussion:
Pareview clean.
Issue queue processed and clean.
Thanks again for the great feedback, as usual with Drupal community interaction, i learned a lot!
Looking forward to have this RTBC!
Comment #4
cpliakas CreditAttribution: cpliakas commentedBased on the actions in #3, I am marking this application as RTBC. Judging by the attention to the issue queue and willingness to work with other developers, danielnolde will be a great maintainer and a valuable contributor. I look forward to this module being promoted to a full project so people can start downloading an official release. The functionality is very useful and a great add-on to Facet API.
Comment #5
klausiAnyway, there is no real blocker for this application, so ...
Thanks for your contribution, danielnolde! Welcome to the community of project contributors on drupal.org.
I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.
Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.
As you continue to work on your module, keep in mind: Commit messages - providing history and credit and Release naming conventions.
Comment #7
jgladstone CreditAttribution: jgladstone commentedWould like to see an example of a simple facet name ( #markup ) change via facet_items_alter function
Comment #8
klausiThis project application is closed. Please use the issue queue of the project.