This is an Ubercart payment gateway module for the Stripe payment processor.

It is unique because there haven't yet been any modules for Ubercart which use Stripe as the payment processor. It also has support for the uc_recurring module so people can create recurring subscriptions using Stripe's subscription capabilities.

The sandbox page is: http://drupal.org/sandbox/victorquinn/1339840
And the git repo: git clone http://git.drupal.org/sandbox/victorquinn/1339840.git uc_stripe

It is currently for Drupal 6, but a Drupal 7 will come shortly.

CommentFileSizeAuthor
#22 drupalcs-result.txt590 bytesklausi
#17 1339850-watchdog.patch937 bytesTR
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

doitDave’s picture

Status: Needs review » Needs work

Hi,

Automated review (Please keep in mind that this is primarily a high level check that does not replace but, after all, eases the review process. There is no guarantee that no other issues could show up in a more in-depth manual follow-up review.)

Review of the 6.x-1.x-dev branch:

  • Run coder to check your style, some issues were found (please check the Drupal coding standards):
    Severity minor, Drupal Commenting Standards, Internationalization, Drupal Security Checks, Drupal SQL Standards, Drupal Coding Standards
    
    sites/all/modules/pareview_temp/test_candidate/uc_stripe.module:
     +2: [minor] Commits to the Git repository do not require the CVS $Id$ keyword in each file.
     +15: [minor] Comment should be read "Implements hook_foo()."
     +30: [minor] Comment should be read "Implements hook_foo()."
     +52: [minor] Comment should be read "Implements hook_foo()."
     +77: [critical] Potential problem: when FAPI element '#type' is set to 'markup' (default), '#value' only accepts filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
     +80: [minor] Comment should be read "Implements hook_foo()."
     +100: [minor] Comment should be read "Implements hook_foo()."
     +120: [minor] Comment should be read "Implements hook_foo()."
     +132: [minor] Comment should be read "Implements hook_foo()."
     +220: [normal] use a space between the closing parenthesis and the open bracket
     +232: [minor] in most cases, replace the string function with the drupal_ equivalent string functions
     +233: [normal] Arrays should be formatted with a space separating each element and assignment operator
     +303: [normal] String concatenation should be formatted with a space separating the operators (dot .) and the surrounding terms
     +399: [normal] Control statements should have one space between the control keyword and opening parenthesis
     +400: [critical] table names should be enclosed in {curly_brackets}
     +407: [normal] The $message argument to drupal_set_message() should be enclosed within t() so that it is translatable.
     +414: [normal] String concatenation should be formatted with a space separating the operators (dot .) and the surrounding terms
     +419: [normal] String concatenation should be formatted with a space separating the operators (dot .) and the surrounding terms
     +449: [minor] You may not want to use SELECT COUNT(*), if all you want to do is check for the existance of any rows, rather than the actual number of rows.
     +463: [normal] The $message argument to drupal_set_message() should be enclosed within t() so that it is translatable.
     +493: [normal] The $message argument to drupal_set_message() should be enclosed within t() so that it is translatable.
     +493: [critical] Potential problem: drupal_set_message() only accepts filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
     +547: [critical] Potential problem: drupal_set_message() only accepts filtered text, be sure all !placeholders for $variables in t() are fully sanitized using check_plain(), filter_xss() or similar.
     +591: [normal] The $message argument to drupal_set_message() should be enclosed within t() so that it is translatable.
     +591: [critical] Potential problem: drupal_set_message() only accepts filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
     +639: [minor] You may not want to use SELECT COUNT(*), if all you want to do is check for the existance of any rows, rather than the actual number of rows.
     +646: [minor] in most cases, replace the string function with the drupal_ equivalent string functions
    
    Status Messages:
     Coder found 1 projects, 1 files, 5 critical warnings, 10 normal warnings, 12 minor warnings, 0 warnings were flagged to be ignored
    
  • README.txt is missing, see the guidelines for in-project documentation.
  • Remove "version" from the info file, it will be added by drupal.org packaging automatically.
  • @file doc block is missing in the install file, see http://drupal.org/node/1354#files .
  • ./uc_stripe.module: comment lines should break at 80 characters, see http://drupal.org/node/1354#general
      // We want to programmatically create a Stripe subscription plan for this item.
      // If no match, this recurring item doesn't correlate to any Stripe subscription plan
      // Tricky to find the nid of the recurring product from a cart of other things.
          // If this product is recurring, we want to add the info to its title display
    
  • Comments should be on a separate line before the code line, see http://drupal.org/node/1354#inline
    ./uc_stripe.module:46:    ), // Use the default user operation defined in uc_recurring.
    
  • ./uc_stripe.module: all functions should have doxygen doc blocks, see http://drupal.org/node/1354#functions
    
    function uc_stripe_product_feature_validate($form, &$form_state) {
    --
    
    function uc_stripe_process($order, &$fee) {
    
  • Remove all old CVS $Id tags, they are not needed anymore.
    uc_stripe.module:2://$Id$
    
  • Do not use t() in hook_schema(), this will only generate overhead for translators.
        'description' => t('Stores customer ids and plan names for Stripe subscriptions.'),
            'description' => t('The recurring fee id.'),
            'description' => t('Whether or not this subscription is currently active.'),
            'description' => t('The uid of this user.'),
            'description' => t('The order_id of the purchase.'),
            'description' => t('The nid of this subscription product.'),
            'description' => t('The Stripe customer id.'),
            'description' => t('The corresponding plan id.'),
    
  • All text files should end in a single newline (\n). See http://drupal.org/node/318#indenting
    ./uc_stripe.install ./uc_stripe.info ./css/uc_stripe.css ./uc_stripe.module
    

This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.

Manual additions:

  • Please have a look at http://drupal.org/node/1127732 and related documents as your master branch contains files and your major branch does not follow the naming conventions.
  • Your readme.md is obviously intended to be your readme.txt; keep an eye on line lengths please!
  • Why do you prevent "powered by" from the translation process? (line 69/.module)
    • OK, more in a second round.

      Hth, dave

mail@victorquinn.com’s picture

Greetings doitDave, I believe I have made all of the above changes. Let me know if anything further is required!

doitDave’s picture

Hi again,

Automated review (Please keep in mind that this is primarily a high level check that does not replace but, after all, eases the review process. There is no guarantee that no other issues could show up in a more in-depth manual follow-up review.)

Review of the 6.x-1.x-dev branch:

  • Run coder to check your style, some issues were found (please check the Drupal coding standards):
    Severity minor, Drupal Commenting Standards, Internationalization, Drupal Security Checks, Drupal SQL Standards, Drupal Coding Standards
    
    sites/all/modules/pareview_temp/test_candidate/uc_stripe.module:
     +68: [normal] String concatenation should be formatted with a space separating the operators (dot .) and the surrounding terms
     +76: [critical] Potential problem: when FAPI element '#type' is set to 'markup' (default), '#value' only accepts filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
     +107: [minor] You may not want to use SELECT COUNT(*), if all you want to do is check for the existance of any rows, rather than the actual number of rows.
     +139: [minor] You may not want to use SELECT COUNT(*), if all you want to do is check for the existance of any rows, rather than the actual number of rows.
     +243: [minor] in most cases, replace the string function with the drupal_ equivalent string functions
     +314: [normal] String concatenation should be formatted with a space separating the operators (dot .) and the surrounding terms
     +422: [normal] The $message argument to drupal_set_message() should be enclosed within t() so that it is translatable.
     +429: [normal] String concatenation should be formatted with a space separating the operators (dot .) and the surrounding terms
     +434: [normal] String concatenation should be formatted with a space separating the operators (dot .) and the surrounding terms
     +464: [minor] You may not want to use SELECT COUNT(*), if all you want to do is check for the existance of any rows, rather than the actual number of rows.
     +478: [normal] The $message argument to drupal_set_message() should be enclosed within t() so that it is translatable.
     +508: [normal] The $message argument to drupal_set_message() should be enclosed within t() so that it is translatable.
     +508: [critical] Potential problem: drupal_set_message() only accepts filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
     +562: [critical] Potential problem: drupal_set_message() only accepts filtered text, be sure all !placeholders for $variables in t() are fully sanitized using check_plain(), filter_xss() or similar.
     +606: [normal] The $message argument to drupal_set_message() should be enclosed within t() so that it is translatable.
     +606: [critical] Potential problem: drupal_set_message() only accepts filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
     +654: [minor] You may not want to use SELECT COUNT(*), if all you want to do is check for the existance of any rows, rather than the actual number of rows.
     +661: [minor] in most cases, replace the string function with the drupal_ equivalent string functions
    
    Status Messages:
     Coder found 1 projects, 1 files, 4 critical warnings, 8 normal warnings, 6 minor warnings, 0 warnings were flagged to be ignored
    
  • ./uc_stripe.module: comment lines should break at 80 characters, see http://drupal.org/node/1354#general
          // If this product is recurring, we want to add the info to its title display
    
  • All text files should end in a single newline (\n). See http://drupal.org/node/318#indenting
    ./uc_stripe.install ./README.txt ./uc_stripe.info ./css/uc_stripe.css ./uc_stripe.module
    

This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.

  • As the automated review shows almost the same issues as before, are you sure you pushed your changes correctly?
  • The master branch is empty with a readme as it should be :) But you chose a major branch name that does not apply to the standards; correct would be 6.x-1.x in your case. (The rest will be tagged when creating snapshots etc. later on.)

Don't forget to re-set the status to "needs review" as soon as you think it's time :)

mail@victorquinn.com’s picture

Status: Needs work » Needs review

Ok, I have removed the 6.x-1.x-dev branch, replaced it with a 6.x-1.x branch, and pushed the changes appropriately this time.

The only thing I'm unsure of is the "All text files should end in a single newline (\n)." thing. I opened each file one by one and each ends in a newline already. So after the final curly brace, there is one new line, then the end of the file. I'm not sure what I'm doing wrong here. I use emacs and it's set to Unix mode, so it's not a problem of having a Windows newline or something, and Googling it for and searching Drupal.org got me nowhere. So I'm not sure how to fix a problem I cannot see.

If I can have some guidance on this newline thing, that would be great. Otherwise, things should be good to go.

doitDave’s picture

Status: Needs review » Needs work

Hi Victor!

Automated review (Please keep in mind that this is primarily a high level check that does not replace but, after all, eases the review process. There is no guarantee that no other issues could show up in a more in-depth manual follow-up review.)

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

  • Run coder to check your style, some issues were found (please check the Drupal coding standards):
    Severity minor, Drupal Commenting Standards, Internationalization, Drupal Security Checks, Drupal SQL Standards, Drupal Coding Standards
    
    sites/all/modules/pareview_temp/test_candidate/uc_stripe.module:
     +462: [minor] You may not want to use SELECT COUNT(*), if all you want to do is check for the existance of any rows, rather than the actual number of rows.
    
    Status Messages:
     Coder found 1 projects, 1 files, 1 minor warnings, 0 warnings were flagged to be ignored
    
  • Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards):
    
    FILE: ...b/dp709/sites/all/modules/pareview_temp/test_candidate/uc_stripe.module
    --------------------------------------------------------------------------------
    FOUND 16 ERROR(S) AND 12 WARNING(S) AFFECTING 24 LINE(S)
    --------------------------------------------------------------------------------
       6 | WARNING | Line exceeds 80 characters; contains 81 characters
     179 | ERROR   | An operator statement must be followed by a single space
     179 | ERROR   | There must be a single space before an operator statement
     180 | ERROR   | An operator statement must be followed by a single space
     180 | ERROR   | There must be a single space before an operator statement
     216 | WARNING | Line exceeds 80 characters; contains 81 characters
     217 | WARNING | Line exceeds 80 characters; contains 98 characters
     219 | WARNING | Line exceeds 80 characters; contains 82 characters
     221 | WARNING | Line exceeds 80 characters; contains 88 characters
     222 | WARNING | Line exceeds 80 characters; contains 88 characters
     223 | WARNING | Line exceeds 80 characters; contains 90 characters
     224 | WARNING | Line exceeds 80 characters; contains 88 characters
     279 | ERROR   | There must be a single space before an operator statement
     286 | ERROR   | There must be a single space before an operator statement
     294 | ERROR   | There must be a single space before an operator statement
     306 | ERROR   | You must use "/**" style comments for a function comment
     316 | ERROR   | An operator statement must be followed by a single space
     316 | ERROR   | There must be a single space before an operator statement
     529 | WARNING | A comma should follow the last multiline array item. Found:
         |         | uid
     554 | WARNING | Line exceeds 80 characters; contains 96 characters
     555 | WARNING | Line exceeds 80 characters; contains 97 characters
     557 | ERROR   | You must use "/**" style comments for a function comment
     619 | ERROR   | You must use "/**" style comments for a function comment
     670 | ERROR   | You must use "/**" style comments for a function comment
     673 | ERROR   | "require_once" is a statement not a function; no parentheses
         |         | are required
     673 | ERROR   | File is being conditionally included; use "include_once"
         |         | instead
     697 | WARNING | Line exceeds 80 characters; contains 104 characters
     699 | ERROR   | You must use "/**" style comments for a function comment
    --------------------------------------------------------------------------------
    
  • All text files should end in a single newline (\n). See http://drupal.org/node/318#indenting
    ./uc_stripe.install ./README.txt ./uc_stripe.info ./css/uc_stripe.css ./uc_stripe.module
    

This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.

Regarding the newlines, check my comment here: http://drupal.org/coding-standards#comment-5262644 as it has cost me lots of sleep myself... ;( Sticking to that, you will be on the safe side.

TR’s picture

@victorquinn: All the files in your sandbox repository currently have the correct newline file endings - there is nothing you need to change in regards to newlines. But the other coding style issues pointed out above should be fixed.

Re: #5: http://drupal.org/coding-standards#comment-5262644 is wrong, and I have replied in that thread.

mail@victorquinn.com’s picture

Status: Needs work » Needs review

Ok, took another pass, I believe I fixed everything now.

raynimmo’s picture

Automated review
Please keep in mind that this is primarily a high level check that does not replace but eases the review process. There is no guarantee that no other issues could show up in a more in-depth manual follow-up review.

Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards):

FILE: ...hemedev\sites\default\modules\reviews\ubercart_stripe\uc_stripe.install
--------------------------------------------------------------------------------
FOUND 3 ERROR(S) AFFECTING 3 LINE(S)
--------------------------------------------------------------------------------
  1 | ERROR | End of line character is invalid; expected "\n" but found "\r\n"
 14 | ERROR | 2 spaces found before inline comment; expected "// off because
    |       | the renewal payments are handled by Stripe and initiating" but
    |       | found "//  off because the renewal payments are handled by Stripe
    |       | and initiating"
 15 | ERROR | 2 spaces found before inline comment; expected "// them
    |       | separately will cause a mess." but found "//  them separately
    |       | will cause a mess."
--------------------------------------------------------------------------------


FILE: ...themedev\sites\default\modules\reviews\ubercart_stripe\uc_stripe.module
--------------------------------------------------------------------------------
FOUND 39 ERROR(S) AFFECTING 31 LINE(S)
--------------------------------------------------------------------------------
   1 | ERROR | End of line character is invalid; expected "\n" but found "\r\n"
  81 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
 160 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
 180 | ERROR | An operator statement must be followed by a single space
 180 | ERROR | There must be a single space before an operator statement
 181 | ERROR | An operator statement must be followed by a single space
 181 | ERROR | There must be a single space before an operator statement
 188 | ERROR | 2 spaces found before inline comment; expected "// for this
     |       | item." but found "//  for this item."
 198 | ERROR | 2 spaces found before inline comment; expected "// has a bug as
     |       | confirmed by the stripe staff. Basically, no plans" but found
     |       | "//  has a bug as confirmed by the stripe staff. Basically, no
     |       | plans"
 199 | ERROR | 2 spaces found before inline comment; expected "// with forward
     |       | slashes in their plan id can be retrieved." but found "//  with
     |       | forward slashes in their plan id can be retrieved."
 203 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
 238 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
 241 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
 318 | ERROR | Inline comments must start with a capital letter
 318 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
 334 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
 346 | ERROR | Inline comments must start with a capital letter
 346 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
 352 | ERROR | 2 spaces found before inline comment; expected "// so we will
     |       | just return a positive result when the amount is $0." but found
     |       | "//  so we will just return a positive result when the amount is
     |       | $0."
 378 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
 383 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
 390 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
 406 | ERROR | 2 spaces found before inline comment; expected "// Stripe
     |       | subscription plan" but found "//  Stripe subscription plan"
 406 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
 413 | ERROR | 2 spaces found before inline comment; expected "// other things.
     |       | Note, this will only work with a single" but found "//  other
     |       | things. Note, this will only work with a single"
 414 | ERROR | 2 spaces found before inline comment; expected "// subscription
     |       | item in the cart. If more, it throws an error." but found "// 
     |       | subscription item in the cart. If more, it throws an error."
 414 | ERROR | There must be no blank line following an inline comment
 438 | ERROR | Inline comments must start with a capital letter
 438 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
 452 | ERROR | Inline comments must start with a capital letter
 452 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
 457 | ERROR | There must be no blank line following an inline comment
 465 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
 492 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
 498 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
 581 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
 594 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
 660 | ERROR | 2 spaces found before inline comment; expected "// its title
     |       | display." but found "//  its title display."
 677 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
     |       | question marks
--------------------------------------------------------------------------------

Manual Review
I realise it is not a strict prerequisite for many reviewers but you should maybe make an attempt to adhere to the CSS coding standards. Also, you include the browser specific styles -webkit and -moz but have omitted to include any for other browsers. Possibly have a read of http://reference.sitepoint.com/css/vendorspecific to make your module a bit more cross browser compatible. These are just personal points of mine though and should not hold up your project application if they are ignored.

Please ensure to change your project status back to 'needs review' once you have addressed the issues highlighted above.

Good luck with the rest of your review.

raynimmo’s picture

Status: Needs review » Needs work
mail@victorquinn.com’s picture

Status: Needs work » Needs review

Ok, made all of the changes to the comments as requested by the code sniffer.

Somewhat curious as to why all of these comment changes didn't show up on earlier passes through the code sniffer or I would have fixed them then and not wasted someone's time with another pass, but oh well.

Also made the suggested changes to the CSS to make it more cross-browser compatible -- thanks raynimmo, I had simply overlooked that!

raynimmo’s picture

The Code Sniffer and Coder modules change regularly, always make sure you are using the most up to date versions as these modules are constantly being updated with new/better rule checks. Always check out the development releases as they are usually newer than the 'stable' versions.

When I run Code Sniffer I usually pass it all file types, such as as php,module,install,js,css,inc,txt , it is possible that previous checks did not run against all of these file types and hence checks where not performed on that group of files.

raynimmo’s picture

Just ran your code through the online PAReview service that patrickd put together.

I am glad to say it didnt flag any errors. I will try and find time to do a manual review in the next few days.

Well done.

mcfilms’s picture

Perfect timing, since I was investigating using Stripe. I may be able to test this in the next couple of weeks.

jthomasbailey’s picture

That git checkout just gives me a readme that says "See major version branches", where can I find the module to test it?

TR’s picture

@hobgobbler: That's the way it should be. git clone will leave you on the master branch, which is unused by Drupal. Do a git branch -a to see the available branches for this project. One of them is 6.x-1.x. Then you can git checkout 6.x-1.x .

Read the contents of the "Version control" tab on the sandbox project page for details.

jthomasbailey’s picture

Thanks got it, that git is all voodoo to me :) It appears you need an SSL certificate to use it though, because of the dependency on the Credit Card module, is that right?

TR’s picture

FileSize
937 bytes

I signed up for a test account with Stripe and tried out your module. Just a couple of issues:

Two of your watchdog calls have the wrong arguments. Attached is a patch that fixes this.

My preference would be to add a hook_requirements() to check for the presence of the Stripe library when the user tries to install uc_stripe. What you do now is defer this check until you actually try to process a payment via Stripe - if the payment fails because the library isn't there the only way you learn what's wrong is by looking in the dblog (and only after the above watchdog calls are fixed :-)) You can use uc_cybersource as an example of how to check for a library from hook_requirements().

Stripe.php also requires cURL - this is not something that is built into PHP by default, and some sites might not have it. You should also check for this in hook_requirements().

In the Stripe gateway configuration, the textfield for entering the secret key should absolutely be changed to a password field. Otherwise, anyone browsing your admin interface can read off your secret key.

The Stripe.php code you're using has a comment that says it's been tested with PHP 5.2 and 5.3. Because it's object oriented code, it's highly likely that it will have problems in lower versions of PHP. Your uc_stripe.info file specifies only php=5.0. I strongly suggest you change that to php=5.2.

I would make the configuration default to using Test mode. The admin should have to consciously switch on Production transactions.

Other than that, the module works and looks like it's ready to go! I'd like to see the password field issue addressed before approving the project.

mail@victorquinn.com’s picture

@TR Thanks for the comments and the patch!

I've applied your patch, good catch there. I also changed the .info file to require PHP 5.2 and am now checking for cURL in the hook_requirements.

I also made the secret keys password fields. I actually intentionally left those as plain textfields so the admins could view them and because they are in plaintext in the Stripe console, but I didn't really consider a situation under which the person viewing the Stripe account wasn't the site admin for the Drupal site. So I have changed those to password fields as per your suggestion.

I'm still figuring out how to reliably perform the check for the Stripe library in hook_requirements() because it is expected to be in sites/all/libraries and the libraries API functions are not available in hook_requirements()** and drupal_get_path() won't really work because it's not a module or a theme. Anyway, wanted to push the rest of the changes, will update when this is done.

** at least I don't think they are, maybe they are, needs testing

mail@victorquinn.com’s picture

@hobgobbler: Yes, basically, you will need an SSL cert. I really wouldn't recommend doing anything with payment processing without it. Although we aren't storing any credit card data locally due to the Stripe magic, it's still a good idea and many customers will expect it.

You can get a free one here: http://www.startssl.com/?app=1

mail@victorquinn.com’s picture

@TR: Forgot to change the test mode default to TRUE. Now changed and committed.

TR’s picture

Status: Needs review » Reviewed & tested by the community

I think this is ready to be approved.

klausi’s picture

Status: Reviewed & tested by the community » Fixed
FileSize
590 bytes

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

This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.

manual review:

  • uc_stripe_product_feature_validate(): all user facing text should run through t() for translation, also for form errors.
  • "We apologize for the convenience.": LOL, I think this should be "inconvenience", right?
  • _uc_stripe_fancy_name(): again, run all user facing text through t().
  • _uc_stripe_post_transaction(): the success message in there looks chopped off.

But I think that are just minor issues, so ...

Thanks for your contribution, victorquinn! Welcome to the community of project contributors on drupal.org.

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

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.

As you continue to work on your module, keep in mind: Commit messages - providing history and credit and Release naming conventions.

Status: Fixed » Closed (fixed)

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