[D7] Commerce Moolah

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Nice work ! :)

Great module !

function commerce_moolah_process_ipn() {
  // Retrieve the IPN from $_GET if the caller did not supply an IPN array.open 
  // Note that Drupal has already run stripslashes() on the contents of the
  // $_GET array at this point, so we don't need to worry about them.
  $ipn = $_GET;

  // Exit now if the request is not a  $_GET.
  if (empty($ipn)) {
    watchdog('commerce_moolah', 'IPN URL accessed with no GET data submitted.', array(), WATCHDOG_WARNING);
    return FALSE;
  }

Since $_GET['q'] will be set on every page load (including if clean URLs are enabled), this if clause will never fire. Perhaps you should check if particular $_GET parameters are empty instead.

  // Skip the requirements check if SimpleTest is installed to avoid multiple
  // cURL rows.
  if (module_exists('simpletest')) {
    return;
  }

…what.

Later in hour hook_requirements implementation, you check for function_exists('curl_init');. Why are you checking for the existence of the cURL library if your code uses drupal_http_request() anyway?

$_GET will still contain values on a POST request on a Drupal site. If you really want to confirm that a request was made using the GET request method, the most correct way is to check $_SERVER['REQUEST_METHOD'] === 'GET'.

I think this looks good. If I had the capabilities to approve you, I would. :D

Two more minor nitpicks I may have missed before:

One, in _commerce_moolah_get_currencies(), the currency names should probably be translable (use the t() function).

Two, regarding…

      '#description' => t('Your @currency guid from <a href="@url">this page</a>, leave blank if you don\'t want to use this currency.', array('@currency' => $key, '@url' => url('https://moolah.io/merchant'))),

Generally, with URLs, you should use !placeholder instead of @placeholder (so !url instead of @url), since you probably don't want them escaped. Check out the documentation for format_string() for the difference between the three different kinds of placeholders.

To the moon!

I am getting the following two errors multiple times at /admin/commerce/config/payment-methods/manage/commerce_payment_moolah/edit/3:

Notice: Undefined variable: settings in commerce_moolah_default_settings() (line 52 of /home/galaxy/public_html/sites/all/modules/commerce_moolah/commerce_moolah.module).

Notice: Undefined index: bitcoin_guid in commerce_moolah_settings_form() (line 80 of /home/galaxy/public_html/sites/all/modules/commerce_moolah/commerce_moolah.module). (except one for each coin for this error)

It seems to cause the guid not to save when I try to.

Let me know if you need any more info.

I look forward to using this module :)

Great your fix solved it :)

However when I was making a test payment I got stuck at where it says "Please wait while you are redirected to the payment server. If nothing happens within 10 seconds, please click on the button below." and there was no button to press.

I tried again and it seems to be working this time. However that button the message refers to was nowhere to be seen though so that might be something to look at.

Also slightly off topic but related, the button on the page that appears at moolah.io that says "Pay with Auroracoin/Bitcoin/etc..", what is it supposed to do exactly? I am paying with auroracoin but it is unclear to me what it is supposed to do.

Thanks for your hard work on this module though.

EDIT: After finishing the payment by clicking the "Click here to return to the merchant." moolah.io gives me I am returned back to the page that redirects me to moolah.io causing a loop. I also get the following error on that page:

Notice: Undefined index: payment in theme_commerce_checkout_progress_list() (line 173 of public_html/profiles/commerce_kickstart/modules/contrib/commerce_checkout_progress/commerce_checkout_progress.module).

I should note that I am running Drupal commerce 1.4 if that changes anything.

gaetanp, sorry for the delay. The review queue got backed up. I can't approve your project, but can do a thorough review, bu

Automated Review

FILE: /var/www/drupal-7-pareview/pareview_temp/README.txt
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 3 WARNING(S) AFFECTING 3 LINE(S)
--------------------------------------------------------------------------------
31 | WARNING | Line exceeds 80 characters; contains 115 characters
35 | WARNING | Line exceeds 80 characters; contains 90 characters
38 | WARNING | Line exceeds 80 characters; contains 84 characters
--------------------------------------------------------------------------------

Nothing major, but linewrap these.

Manual Review

1. Project page looks OK.

2. README.txt mentions curl, but you aren't using any curl functions.

3. The module uses entity_metadata_wrapper(), but you are not declaring a dependency on entity. Commerce does require this, but you should really add this to the .info file in case something changes with Commerce.

4. Your code doesn't use proper docblocks.

5. Can you add element validation to $form['api_key'] and $form['ipn_secret']?

6. In commerce_moolah_submit_form_submit, line 150, you shouldn't combine the logic like that. The default value that core uses is 'Drupal'.

7. On commerce_moolah_submit_form_submit, line 142, use REQUEST_TIME.

8. In commerce_moolah_submit_form_submit, you should have a comment to the Moola.io API docs for reference.

9. In commerce_moolah_submit_form_submit, line 172, you don't need to call url(). drupal_http_request() can take the query options.

10. You should also check the $response->code to make sure it is 200 before proceeding, and handle the error if it isn't.

11. In commerce_moolah_get_transaction_from_remote(), you don't need all the fields from the table, just transaction_id.

12. In commerce_moolah_settings_form, build the link outside of t() with l() and pass it in via a placeholder.

I'm sending this back to Needs Work for 2, 3, 6, 8, 9, 12. These aren't major issues, but I think there are enough small things before a project administrator takes a look. I am doing a lot of reviews, and once you take care of these I can give you a prompt review and RBTC if nothing else comes up.

I changed the status so it gets in the queue. Also, I was wrong about #12. Sorry about that. See https://api.drupal.org/api/drupal/includes!common.inc/function/l/7 That won't be a show stopper, though, if it doesn't get changed before another review.

Review of the 7.x-1.x branch:

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
  

  FILE: /home/klausi/pareview_temp/README.txt
  --------------------------------------------------------------------------------
  FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
  --------------------------------------------------------------------------------
   29 | WARNING | Line exceeds 80 characters; contains 115 characters
   33 | WARNING | Line exceeds 80 characters; contains 90 characters
   36 | WARNING | Line exceeds 80 characters; contains 84 characters
  --------------------------------------------------------------------------------
  

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. You have to get a review bonus to get a review from me.

• manual review:
• commerce_moolah.module:142: If you want to reference links in your doc comments you should use @see, see https://www.drupal.org/node/1354#see

But otherwise looks RTBC to me. Assigning to mpdonadio as he might have time to take a final look at this.

Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Automated Review

Review of the 7.x-1.x branch:

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
• DrupalPractice has found some issues with your code, but could be false positives, and may be duplicate results from Coder Sniffer. See attachment.

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. You have to get a review bonus to get a review from me.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No multiple applications

Yes

No duplication

Yes

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements

3rd party code

Yes: Follows the guidelines for 3rd party code.

README.txt

Yes: Follows the guidelines for in-project documentation and the README.txt Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes.

Coding style & Drupal API usage

You can use drupal_http_build_query instead of http_build_query (line 175), or use the second paramter to drupal_http_request (line 180).

Use drupal_json_decode() instead of json_decode (line 187).

You need to translate the titles for 'api_key' and 'ipn_secret' in commerce_moolah_settings_form(). Long term, will there be any way to validate these?

No blocking issues. Agree with @klausi that this is ready for prime time.

Thanks for your contribution, gaetanp!

I updated your account so you can 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. So, come hang out and stay involved!

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.

Thanks to the dedicated reviewer(s) as well.