Project Review Wednesday: Chosen Select

There are currently 88 new Drupal contributors awaiting review of their first project. This is a great place to contribute to the community and learn about interesting upcoming projects, for example...

Module: Chosen Select

What does it do?

Chosen Select adds a new field type to Drupal's Form API. It uses jQuery Select2 to add new widgets to traditional select inputs, for example, a searchable list of options. This is something I've done before with custom code, so having a module that covers at least the base functionality seems useful.

Look Useful? Review it! I Reviewed It!

Normally this is where we suggest you review the project, but this time I'm going to explain to you how I reviewed it. Hopefully you'll see how easy it is to do reviews, and start doing more reviews yourself.

The first thing I do when reviewing this module is note that it's using a 3rd party jQuery plugin. One of the more common problems with new modules is including 3rd party code directly in the module's repository on Drupal.org, which is not allowed. So after clicking through to look at the code, the first thing I check for is some 3rd party code. Fortunately, the code uses the Libraries API to add a dependency on the 3rd party code without directly including it. Great! This is a check pretty much anyone who can read can do. In short, if you see someone else's (other than the module contributor's) name given attribution in the code, that's a problem.

Next, let's check the README.txt. Does it make sense? It could maybe use more info, but that looks good enough. Next stop, chosenselect.info. I don't see any problems here. It has the libraries dependency, and otherwise pretty simple.

Now the hardest part: let's look at the module file. The first thing I do here is just a quick skim of the code. Does it look like other Drupal code I've seen? If not, that's probably a coding standards problem. At first glance, this looks good. Next thing I do is look more closely for any text being output, to make sure it's sanitized (for security) and, where appropriate, translatable. In short, I want to see t() or check_plain() or filter_xss() around text being output. I see a lot of t(), so that looks good.

Before I move on, I notice a constant, FIELD_BEHAVIOR_CUSTOM, that I don't recognize. If that's something the module is defining, it should be namespaced with the module name. I don't see a definition in the module file, so I checked api.drupal.org. There it is. I just learned something new. Nice!

Next, let's look at chosenselect.forms.inc. This file has more code than the main module file, so will take more time to look over, but it's basically the same process. Code looks generally in line with what I expect Drupal code to look like, and seems to have appropriate sanitization on output. Database queries are using Drupal's database API, which removes most security risk there.

One thing that catches my attention is $_GET['term']. Drupal code doesn't normally access $_GET directly, since parameters are usually passed in as path segments. So I think chosenselect_autocomplete() should probably look more like taxonomy_autocomplete(). I'm not entirely sure why that's the Drupal standard, but the project contributor can always tell me if she has a good reason to deviate from that norm. So that's enough to move the application to "needs work" status, but I want to find as much as possible before posting a review, to limit the back-and-forth between "needs review" and "needs work."

Next thing I notice is a print and exit in a function that otherwise returns JSON. Let's jump over the JavaScript file to see how that's going to handle this non-JSON response. Hmm, this has two TODO statements at the top of the code. Another thing to note. TODOs should be done before reviews start.

I don't see any sign that the autocomplete AJAX has handling for non-JSON responses, though I think some of that logic must be in the jQuery plugin. I'll just post that as a question. The only other file to review is CSS, which looks fine. It's hard to have problems with CSS.

The last thing I do is run the project through the automated review script on ventral.org. This would be the first thing I'd do if I suspect a lot of errors, but this project looked pretty good, so the script just confirmed what I missed, which looks to be just minor code formatting issues.

I posted my review as a comment and changed the status to "needs work." So that's what a review looks like. Try it yourself, you might like it.

Pro Tip: If you've never reviewed a project application before, you can find instructions for reviewers on Drupal.org and the Code Review group is happy to help more people get involved.

Code Drupal Drupal Planet Project Review Wednesday

Read This Next