Commerce Direct Debit

Manual Review:

Everything looks good except below

1) All the user provided text should run through the appropriate sanitization function in commerce_directdebit_submit_form().

Please try to resolve automated review.

As to the above comment, there's nowhere that I can see that user input is being output in such a way that it would need sanitization. The only place I can see it being used is as the #default_value for form elements, but you shouldn't be sanitizing user input there.

Also, your automated code review is a bit aggressive; it's reporting as errors coding standards that have only been adopted as of Drupal 8. See http://drupal.org/node/1354#param-return-data-type for more info. (But if you're going to document parameter types, the $payment_method instance is actually an array.)

A few other points of review for this module:

1. In commerce_directdebit_commerce_payment_method_info(), what you're defining as the title should really be the display_title. The title itself would just be "Direct debit." See the full parameter list in commerce_payment.api.php.
2. In commerce_directdebit_logo(), you should use t() on the title.
3. I'm not sure that your validate function is really doing anything since you have those various fields marked as #required in the form array.
4. You're creating a successful transaction in commerce_directdebit_transaction() even though no actual payment has been collected. This is dangerous, because sites may have sensitive order fulfillment logic set to run on the event When an order is first paid in full. As written, your module will trigger that event even if the customer enters bogus bank information. I'm not sure of the security implications of storing bank account numbers like this in the database - seems dicey to me - but if this is legit for off-line processing, what your module should do is create a transaction with the COMMERCE_PAYMENT_STATUS_PENDING status and a form for the payment to be marked as accepted by administrators that would update it to COMMERCE_PAYMENT_STATUS_SUCCESS (or I suppose a way to say it failed and update it accordingly, too).

Looking better, Garrett. I'd say the workflow now for transactions in checkout is fine as far as it goes, but it would really be helpful to add a "local action" form on direct debit payment transactions where these could be marked as successful or failed. I wouldn't say that needs to hold up a release, but it should at least be in the roadmap for folks that can't just use a simple Rules based solution. For an example, you can look at the Authorize.Net module's capture form for pending credit card transactions.

Also, this is stylistic, but since your fields for direct debit names / numbers are self-explanatory, you probably don't need the #description text. Up to you, but it just looked like they were restating the field #title.

Cool, not sure how many people need to sign off on these, but it just be RTBC as far as I'm concerned.

please keep security tags for statistics

Can you explain to me what the meaning of that tag is? I assumed it was because there was a security issue detected with the project in question, but it was a false claim. The text comment #1 indicated needed sanitization did not need sanitization - it was for default values in a form field. :-/

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

Also removing the falsely assigned security tag.

Thanks for your contribution, garrettc!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on irc in #drupal-contribute and #drupal-commerce. So, come hang out and get involved!

Thanks rszrama for your awesome review and guidance to garrettc!!