[D7] Commerce RedeCard

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

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.

Hi,

Here my quick first review of your module. I continue my tests.

RedeCard review (branch 7.x-1.x)

Js/print_cupom.js
- You should use behaviors (https://drupal.org/node/756722) to run print_cupom() when user clicks on button (.super-button). It’s better to separate php and js code (commerce_redecard.module l.910).

TODO (file)
- Please remove this file on 7.x-1.x branch or write todo list in english.

README.txt
- You should explain how to install and enable your module.

Commerce_redecard.info
- You should remove l.4 with commerce dependency because commerce_payment module already has got dependency to commerce module.

Commerce_redecard.module
- You should use « Drupal standards for in-line code comments » everywhere (https://drupal.org/node/1354): l.38, l.41, l.44, …

Review with coder module (see attached file)

Have a good day.

- Unused variable $msgret_final (commerce_redecard.module)
- You should patch your module to resolve undefined variables (l.47, 48, 49) when redecard payment is disable and when user is on commerce-redecard/notification page
- Please check restriction access to commerce-redecard/notification page (hook_permission https://drupal.org/node/1118210)
- There are some sentenses which aren’t translated « Parece que houve algo de errado … ». You should use t() function and add theme/template (https://api.drupal.org/api/drupal/modules!system!system.api.php/function...).

When I want to fill in payment settings (admin/commerce/config/payment-methods/manage/commerce_payment_redecard/edit/3) I have some notices. Please see attached file.

Fixing some metadata on this issue.

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

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

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

Adding similar module.

@julitroalves, where do you posted your fixes?

.module file:
Update/remove this comment: // If received a POST is a notification API request from Pagseguro.

if (isset($_GET) && !empty($_GET['NUMCV']) && !empty($_GET['NUMCV']) && !empty($_GET['NUMAUTOR']) && !empty($_GET['NUMSQN'])) {
No need for repetition of NUMCV.

      $codret = (int) filter_input(INPUT_GET, 'CODRET', FILTER_SANITIZE_NUMBER_INT);
      $order_id = (int) filter_input(INPUT_GET, 'NUMPEDIDO', FILTER_SANITIZE_NUMBER_INT);

I haven't used filter_input before, should the GET parameter be checked for !empty first?

if (!is_object($order)) {
        return '<h1>' . t('We have a error') . '</h1><p>' . t('Order not found. Please does other order.') . '</p>';
      }

This error message doesn't make sense. Should it read "Please do another order"?

Line 86 perhaps the error would more appropriately read:
Something is wrong! Error description:

Perhaps this message (line 91) should read:
'Something is wrong! It appears that an unexpected error occurred. Please contact shop owner.

Lines 105-114. Consider using drupal_http_build_query
Also applies to lines 903-909.

Line 117: consider checking for HTTP success of 200 or other validation.

Line 131: // Deading session variables that.
This comment doesn't make sense.

hook_mail implementation. I'd suggest you implement this as a rule. That's what commerce is built on and it will give site builders more flexibility to adjust the wording and whether they even want to send an email.

    drupal_set_message(
      t('RedeCard Integration is not configured for use. You can')
        . l(
            t('access Redecard Payment Method configuration'),
            'admin/commerce/config/payment-methods/manage/commerce_payment_redecard'
          )
        . t('to do it now.'),
      'error'
    );

Concatenating strings into t() is considered bad. It makes it difficult to create translations. Rather use variable replacement.

Line 929: drupal_set_message(t("Data transaction isn\'t available to this order. Please do your order again."), 'error');
Escaping the single quote (') isn't necessary.

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.