[D7] Commerce Netbilling payment gateway integration

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

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.

Automatic Review:

1. Please fix some errors and warnings identified by http://pareview.sh/.
http://pareview.sh/pareview/httpgitdrupalorgsandboxbradjones12359757git

FILE: /var/www/drupal-7-pareview/pareview_temp/commerce_netbilling.api.php
--------------------------------------------------------------------------------
FOUND 11 ERRORS AFFECTING 9 LINES
--------------------------------------------------------------------------------
29 | ERROR | Type hint "array" missing for $request
29 | ERROR | Type hint "array" missing for $response
30 | ERROR | Inline comments must end in full-stops, exclamation marks, or
| | question marks
45 | ERROR | Type hint "array" missing for $request
45 | ERROR | Type hint "array" missing for $response
46 | ERROR | Inline comments must end in full-stops, exclamation marks, or
| | question marks
59 | ERROR | Type hint "array" missing for $request
60 | ERROR | Inline comments must end in full-stops, exclamation marks, or
| | question marks
64 | ERROR | Doc comment short description must end with a full stop
72 | ERROR | Type hint "array" missing for $request
73 | ERROR | Inline comments must end in full-stops, exclamation marks, or
| | question marks
--------------------------------------------------------------------------------

FILE: /var/www/drupal-7-pareview/pareview_temp/commerce_netbilling.module
--------------------------------------------------------------------------------
FOUND 14 ERRORS AND 9 WARNINGS AFFECTING 23 LINES
--------------------------------------------------------------------------------
149 | WARNING | Line exceeds 80 characters; contains 106 characters
240 | ERROR | Inline comments must end in full-stops, exclamation marks, or
| | question marks
254 | WARNING | Line exceeds 80 characters; contains 92 characters
273 | ERROR | Inline comments must end in full-stops, exclamation marks, or
| | question marks
276 | ERROR | Inline comments must end in full-stops, exclamation marks, or
| | question marks
296 | ERROR | Inline comments must end in full-stops, exclamation marks, or
| | question marks
322 | WARNING | Line exceeds 80 characters; contains 86 characters
332 | ERROR | Inline comments must end in full-stops, exclamation marks, or
| | question marks
337 | ERROR | Inline comments must end in full-stops, exclamation marks, or
| | question marks
342 | ERROR | Inline comments must end in full-stops, exclamation marks, or
| | question marks
354 | WARNING | Line exceeds 80 characters; contains 121 characters
393 | ERROR | Type hint "array" missing for $payment_method
399 | ERROR | Inline comments must end in full-stops, exclamation marks, or
| | question marks
426 | ERROR | If the line declaring an array spans longer than 80
| | characters, each element should be broken into its own line
436 | WARNING | Line exceeds 80 characters; contains 98 characters
437 | WARNING | Line exceeds 80 characters; contains 93 characters
450 | ERROR | If the line declaring an array spans longer than 80
| | characters, each element should be broken into its own line
456 | ERROR | If the line declaring an array spans longer than 80
| | characters, each element should be broken into its own line
536 | WARNING | Line exceeds 80 characters; contains 83 characters
537 | WARNING | Line exceeds 80 characters; contains 81 characters
555 | WARNING | Line exceeds 80 characters; contains 99 characters
569 | ERROR | Parameter comment must end with a full stop
576 | ERROR | Type hint "array" missing for $items
--------------------------------------------------------------------------------

2. Please add Readme.txt in source. I can't find any readme file in source.

Manual Review:

You used variable_get function but there isn't any using of variable_set in your module.
Please confirm it.
And variables used in your module should be removed in hook_uninstall.

function commerce_netbilling_default_settings() {
  return array(
    'endpoint_prefix' => COMMERCE_NETBILLING_ENDPOINT_PREFIX,
    // If Netbilling Membership module is enabled and configured,
    // use its account as default.
    'account_id' => variable_get('netbilling_membership_account_id', ''),
    'dynamic_security_code' => '',
    'txn_type' => 'S',
    'disable_email_receipts' => TRUE,
    'log' => array('request' => '0', 'response' => '0'),
    'card_types' => array(),
    'additional_text' => '',
  );
}

Hi Brad,

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

Manual Reviews

1. Dependency module is not added in .info file.

2. After module installation, No idea where to go and find settings.

3. README.md does not contain installtion instructions.

4. Unnecessary hooks are created, Please find below.

line 59-61
function hook_commerce_netbilling_build_order_request_alter(&$request, $order) {
  // Add elements to $request here.
}

line 72-74
function hook_commerce_netbilling_request_alter(&$request) {
  // Add elements to $request here.
}

Line 160:

$form['credit_card']['number']['#description'] = commerce_netbilling_oxford($card_type_names) . ' ' . t('accepted') . '.';

I would suggest keeping the return value of function commerce_netbilling_oxford in a separated variable. Then, you could do:

  $value = commerce_netbilling_oxford($card_type_names);
  $form['credit_card']['number']['#description'] = t('@value acepted.', array('@value' => $value));

Cheers!

Manual review:
I have installed and active this module. and i get this error:
get type null object at commerce_netbilling.module(4778)

I look for it in your code, and take a long time debug:
$result = drupal_http_request($url, $options);

$result = null, when you use parse_str($result->data, $payload), it cause error. Please give me an advise to fixed this.

I think we need if(!empty($result->data)) is better right?

Hello there,
Thanks for the work on this module.

The module does not check to see if it has been properly configured and thereafter return the appropriate message.

I have just installed the module and I did not add any configuration. I tried a payment and of course it did not go through. I got an error message saying:

Sorry, we do not accept Visa cards.
Please provide a card.

This does not properly inform the user about the cause of the problem.

The module should check to make sure that the most required fields have been set. Thus, I would like to suggest something like the following to be added from commerce_netbilling.module:151

  if (empty($payment_method['settings']['endpoint_prefix']) ||
    empty($payment_method['settings']['account_id']) ||
    empty($any_other_setting_that_is_required)) {
    drupal_set_message(t('Commerce Netbilling has not been properly configured'), 'error');
    return array();
  }

Although this is an unlikely scenario it will not take much to handle.

Regards,
Feyisayo

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

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