[D7] commerce_paystand

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

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.

Your code fails automated testing. Please correct these issues. (They aren't blocking, however)

http://pareview.sh/pareview/httpgitdrupalorgsandboxpaystand2311461git

Please note that LANGUAGE_NONE is always preferable to 'und'. Though you should try to ensure that your code respects localization.

Please add link to your sandbox project page.

Your sandbox project page does't have necessary infomation
See the tips for a great project page: https://www.drupal.org/node/997024

Automatic review

There are some issues reported from PA Review http://pareview.sh/pareview/httpgitdrupalorgsandboxpaystand2311461git

Manual review

This module should require module commerce_payment_ui (include in project commerce) that define menu to configure payment methods (admin/commerce/config/payment-methods)

I go to Drupal Commerce Payment Settings page (admin/commerce/config/payment-methods) but there no payment method call PayStand.

Do you implement hook_commerce_payment_method_info()?
See: http://cgit.drupalcode.org/commerce/tree/modules/payment/modules/commerc...

Line 171, file commerce_paystand.module , function commerce_paystand_psn_load()

function commerce_paystand_psn_load($id, $type = 'txn_id') {
  return db_select('commerce_paystand_psn', 'cpi')
    ->fields('cpi')
    ->condition('cpi.' . $type, $id)
    ->execute()
    ->fetchAssoc();
}

You should not use "." in condition() method. Please see https://api.drupal.org/api/drupal/includes!database!database.inc/functio...

Line 188, file commerce_paystand.module , function commerce_paystand_psn_save()

return drupal_write_record('commerce_paystand_psn', $psn, 'psn_id');

The 3rd arg is array, but your pass string to it
See: https://api.drupal.org/api/drupal/includes%21common.inc/function/drupal_...

Line 230, file commerce_paystand.module , function commerce_paystand_payment_transaction_load()

$transactions = commerce_payment_transaction_load_multiple(array(), array('remote_id' => $txn_id));

You should not pass array() to 1st arg. This will always return an empty array. You should pass FALSE to 1st arg to get all payment transactions

Line 306, file commerce_paystand.module , function commerce_paystand_fee_add_form_submit()

Why you comment all code in this function body?

Dear @paystand,
Thanks for your contribution.
I have not checked how your module works yet but from examining the .module file, I notice the following two functions don't have a body:

function commerce_paystand_fee_configuration($line_item_type) {
}
function commerce_paystand_fee_add_form_submit(&$line_item, $element, &$form_state, $form) {
}

Either add a body (which is currently commented out) or delete the functions.

Regards,
imgio

Hey,

This looks nice, good work =).

in commerce_paystand.module; function commerce_paystand_process_psn. Update the following PHP functions json_decode, json_encode to Drupal's drupal_json_encode, drupal_json_decode. You're using drupal_json_decode for file_get_contents, so maybe there's a reason. You could also consider using drupal_http_request() with $host/ $send.

in commerce_paystand_offsite.module; function commerce_paystand_offsite_settings_form. You're server options are missing t().

in commerce_paystand_offsite.module; function commerce_paystand_offsite_redirect_form. You're also using json_encode this could be updated to drupal_json_encode. Near the bottom of this function you're including some markup, split the JavaScript out and instead attached it to the form using the #attached property (you could also use drupal_add_js but the latter is better).

/**
 * Payment Form.
 */
function commerce_paystand_offsite_redirect_form($form, &$form_state, $order, $payment_method) {
  $form['#attached']['js'] = array(
    array(
     'data' => "
       var PayStand = PayStand || {};
       PayStand.checkouts = PayStand.checkouts || [];
       PayStand.load = PayStand.load || function(){};
       PayStand.checkoutUpdated = function(){};
       PayStand.checkoutComplete = function(){};
       PayStand.checkouts.push({$checkout_button_json});
       PayStand.onLoad = function(){PayStand.execute({$checkout_onload_json});};
       PayStand.script = document.createElement('script');
       PayStand.script.type = 'text/javascript';
       PayStand.script.async = true;
       PayStand.script.src = 'https://{$server}/js/checkout.js';
       var s = document.getElementsByTagName('script')[0];s.parentNode.insertBefore(PayStand.script, s);
      ",
      'type' => 'inline'
    )
  );
}

Obviously you need to pass $checkout_onload_json to your script, this could also be done via Drupal.settings.

I'm sure I missed stuff but I hope this helps, FYI the code I have posted is untested.

Hi,

The automated testing (PareView.sh) showing some issues on 'commerce_paystand_offsite.module' file.
Have a look on the attached files for more information and try to fix them.

Sanat

Removed the tag

Great, I like commerce modules :)

Watching around commerce_paystand_offsite.module, starting from line 139, there are several uses of "LANGUAGE_NONE". I would suggest using entity_metadata_wrapper, like this:

  $wrapper = entity_metadata_wrapper('commerce_order', $order);

Hello,

You have not added your Project page link: above the git clone command.
Add this link so any one can go on your project page.

Thanks

Hi paystand,

for better performance you should create a few more indices on your table commerce_paystand_psn. Since your commerce_paystand_psn_load() function allows to load a PSN by every column, you should put an index at least on every id column (org_id, consumer_id, order_id, transaction_id) and other columns that you could imagine that an other module could filter on it.

In commerce_paystand_offsite.module in function commerce_paystand_offsite_redirect_form() you should pass the text PayStand - Modern commerce with multiple payment methods and no transaction fees through t(). Generally it would be a better way to define a theme function to generate the entire markup.

Best regards

@braindrift: that sound like good suggestions for improvements, but are surely not application blockers on their own. Anything else that you found or should this be RTBC instead?

Hi!

Congrats on the project.

Just some items:

File: /commerce_paystand.install
Expected 1 blank line after function

Array indentation error: expected 6 spaces but found 8

<?php
$context = stream_context_create(array(
        'http' => array(
          'method' => 'POST',
          'header' => "Content-Type: application/json\r\n",
          'content' => drupal_json_encode($send),
        ),
    ));
?>

Try something like this:

<?php
    $context = stream_context_create(array(
      'http' => array(
        'method' => 'POST',
        'header' => "Content-Type: application/json\r\n",
        'content' => drupal_json_encode($send),
      ),
    ));
?>

Regards.

@renatog: minor coding standard issues are surely not application blockers. Anything else that you found or should this be RTBC instead?

It's working fine now. Great Work.

@paystand - You can now promote this to a full project yourself.

Thank you for your contribution!

I updated your account so you can opt into security advisory coverage now.

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!

Thank you, 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 go the dedicated reviewer(s) as well.

Assigning credits.