[D7]commerce_intuit

I guess this needs review? See https://drupal.org/node/532400

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.

+1

Its good to have such module which provide Intiuit payment support. Tested in my dev site and it has just served my purpose. Thanx for coming with such module.

Nice Module. Easy to configure and use.

replaced git link

Hi, greaat module

There are not many bugs in the code, most with text lengths and other small items, I noticed that there are some places which lack the ability to translate what is being written out to the user. see:
http://awesomescreenshot.com/0401wocw1d

If I try pay-ment of without entering "valid" login details then I get these ugly error messages:
http://awesomescreenshot.com/0711wocnca

And it seems that you can not choose to enter only the information that applies to what you have selected in the top of the settings page, I will be asked to enter info for production although I well have marked that I want to test only sandbox.
http://awesomescreenshot.com/06f1wod9c1

If you could look at that, it vould be great - and I will try it out again.

Hi @cheatlex,

Thanks for reporting these issues. Have fixed these issues in commit at5f648ba47f9694849f07ab3750d614aec0626901

The second issue is related to curl not installed and that is shown as error on status report.

Hth,
Sadashiv.

Removed automated review

Sorry for the delay. Make sure to review more project applications and complete your review bonus and this will get finished faster.

There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732
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/commerce_intuit.install
  --------------------------------------------------------------------------------
  FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
  --------------------------------------------------------------------------------
   24 | ERROR | Expected 1 space after "="; 2 found
  --------------------------------------------------------------------------------
  

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:

1. commerce_intuit_submit_form_submit(): why do you check for cURL here, you are already requiring it in hook_requirements(), right?
2. "drupal_alter('commerce_intuit_txnarray', $txn_array, $order);": Hooks that are provided by a module should be documented in MODULENAME.api.php, see http://drupal.org/node/161085#api_php
3. commerce_intuit_transaction(): this is vulnerable to XSS exploits. You are passing variables with the "!" placeholder to watchdog(), which means they don't get sanitized. Since those variables stem from an untrusted third party source they have to be considered user provided input. You need to sanitize those before printing or putting into watchdog, see https://drupal.org/node/28984 . And please don't remove the security tag, we keep that for statistics and to show examples of security problems.
4. commerce_intuit_admin_settings(): the PEM path field should mention that the key should never be stored in the docroot, because then anonymous users could be able to download it from your server. You could even validate that the file is not somewhere in the DRUPAL_ROOT. The README.txt is also confusing in this regard as it recommends "PEM files need to be stored in root directory" - what root directory? Of the file system? the drupal root directory? the user's home directory?

Hi Niteshp,

I installed in my dev Commerce environment and all appears to work as intended.

Manually looking at the code, I can only see some *really* tiny picky issues such a few different uses of case when using intuit/Intuit in some of the comments and possibly missing documenting function @return value for a couple of functions where @param(s) have been included (commerce_intuit_transaction, commerce_intuit_submit_form_submit, commerce_intuit_submit_form).

Last comment is that the payment method is added as active as soon as the module is installed, meaning that it appears in the checkout before you can configure it.

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

• DrupalPractice has found some issues with your code, but could be false positives.
  

  
  FILE: /home/klausi/pareview_temp/commerce_intuit.inc
  --------------------------------------------------------------------------------
  FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
  --------------------------------------------------------------------------------
   189 | WARNING | Potential security problem: SSL peer verification must not be
       |         | disabled
  --------------------------------------------------------------------------------
  

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:

• watchdog('commerce_intuit', 'Intuit transaction response: @param', array('@param' => '<pre>' . print_r($result, TRUE) . '</pre>'));: that will kill your pre tags by escaping them, right? You could use the "!" placeholder and apply check_plain() to the result of print_r().

Otherwise looks almost ready, the security issue is a blocker right now. Sorry for not spotting that earlier. You enable peer verification and then you disable it again, what's the point?

Hi Nitesh,

(I've edited this to put the correct example error message in - Sorry!)

Recent doc changes look good.

Maybe it's intentional (if so, please ignore!) but I still see the payment method enabled by default, after installing the module. But, this means that if the method is not yet configured, and a user selects the payment method whilst purchasing, they receive 5 errors concerning getting properties for non-objects eg.: "Notice: Trying to get property of non-object in CommerceIntuit->checkSignon() (line 156 of C:\Work\drupal-7.19\sites\d7.test.localhost\modules\2104913\commerce_intuit.inc).".

Thanks,

Alex

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxNiteshP2104913git

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

Manual Review:

Please fix these errors first:
http://pareview.sh/pareview/httpgitdrupalorgsandboxniteshp2104913git

Minor coding standard errors are not application blockers, please do a real manual review.

Removed automated reviews.

Sorry for the delay. Make sure to review more project applications and get a review bonus and this will get finished faster.

manual review:

• commerce_intuit_requirements(): ''The testing framework could not be installed because the PHP !cURL library is not available.' sounds wrong, copy & paste error?

But otherwise looks RTBC to me.

Assigning to jthorson who might have time to take a final look at this.

now actually assigning.

no objections for more than a week, so ...

Thanks for your contribution, Niteshp!

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.