Integrates the Skipjack API to accept credit card payments in Drupal Commerce. This is a port of the uc_skipjack module, utilizing some of the core code, but payment methods are handled differently in Drupal Commerce, as well as orders being stored in a much different format.

http://drupal.org/sandbox/jazzdrive3/1617732

git clone --recursive --branch 7.x-1.x

This is for Drupal 7.x

You can view my port of the User Referral module to Drupal 7 if you need more evidence of my competence. :)

http://drupal.org/project/referral

Comments

jazzdrive3’s picture

jazzdrive3 commented
Status: Active » Needs review
jleiva’s picture

jleiva commented
Issue tags: +PAreview: review bonus

welcome,

There are still files other than README.txt in the master branch, make sure to remove them.

Drupal Code Sniffer has found some issues with your code (please check the Drupal coding standards). See report http://ventral.org/pareview/httpgitdrupalorgsandboxjazzdrive31617732git"

patrickd’s picture

patrickd commented

@ jleiva

Seems like you missunderstood the sense of the "review bonus" tag, please read https://drupal.org/node/1410826
(btw, Pasting automated reviews into applications is not enough, manual reviews are required.)

Do not paste the full results into the issue, as giant ‘wall-of-wrong’ posts are extremely demotivating to applicants. Rather provide a link or attachment of the results.

As you can still edit and shorten your comment - please replace your text with the proper link

jazzdrive3’s picture

jazzdrive3 commented
Issue tags: -PAreview: review bonus

Alright, I have made the changes so it passes that check. No errors or warnings are reported.

Thanks.

jazzdrive3’s picture

jazzdrive3 commented
Priority: Normal » Major
jazzdrive3’s picture

jazzdrive3 commented
Priority: Major » Critical

4+ weeks.

jazzdrive3’s picture

jazzdrive3 commented
Issue tags: +PAreview: review bonus

My 3 project reviews:

http://drupal.org/node/1497114#comment-6227296
http://drupal.org/node/1661210#comment-6166798 and http://drupal.org/node/1661210#comment-6170134
http://drupal.org/node/1713614#comment-6320172

Please get this approved. This module is currently working great on a live site as we speak. I also have at least two more complete modules I'd like to release and start getting feedback on.

Thanks!

patrickd’s picture

patrickd commented
Issue tags: -PAreview: review bonus

Most of your reviews are just pointing to automated reviews or cite the output of it.

Real manual reviews of api usage, security and best practices is a requirement for the review bonus program.
Your reviews don't state anything about these points

Sorry, removing tag

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Priority: Critical » Normal
Status: Needs review » Reviewed & tested by the community
Issue tags: +PAReview: Commerce

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

manual review:

  1. commerce_skipjack_settings_form(): you should check for cURL in hook_requirements(), see simpletest.module for example: http://api.drupal.org/api/drupal/modules!simpletest!simpletest.install/f...
  2. "'development' => 'Development',": all user facing text must run through t() for translation.
  3. "drupal_set_message(t('cURL error: @error', array('@error' => $error)), 'error');": do not print the detailed cURL error to the enduser! That should go into the log only and you should provide a more generic message that does not reveal any information about your setup.

Not sure if showing the cURL error is a security blocker (information leakage), I give you the benefit of the doubt and say this is RTBC. Please fix it anyway.

jazzdrive3’s picture

jazzdrive3 commented

klausi,

Thank you very much!

Set the correct default branch and deleted master, although it only had the README.txt file.

I also removed the detail curl error message and provide a more generic one, while only placing the detailed one in the log.

I'll be implementing the hook_requirements soon.

Thanks again.

patrickd’s picture

patrickd commented
Status: Reviewed & tested by the community » Fixed

Your project page is not very detailed, please take a moment to make your project page follow the tips for a great project page.

Using hook_requirements() in a .install is the more common way to checking your requirements, you can have a look at eg. the virustotal module, which also requires curl.

But I couldn't find any serious issues,

Thanks for your contribution!

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:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get 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.

Automatically closed -- issue fixed for 2 weeks with no activity.