Paybox integration for the Drupal Commerce payment and checkout system.
This module carries part of the functionality of the module on Drupal 8/9,
Paybox Service method using Commerce Payment Gateway is implemented here.

Project link

https://www.drupal.org/project/commerce_paybox_payment

Git instructions

git clone --branch '1.0.x' git@git.drupal.org:project/commerce_paybox_payment.git

Comments

anthonychoosit created an issue. See original summary.

macsim’s picture

Status: Needs review » Needs work

This is my first review so if anything seems out of order, please reset the status to 'Needs Review.'

  1. The module doesn't provide a hook_help() nor a README file. Please check guidelines for in-project documentation and/or the README template.
  2. I ran PHPCS with both standards, and each came out clean
  3. I did NOT used the module on a project since I have no project with commerce module installed - therefore I am not able to give a feedback on the features provided by the module.

Automated review

phpcs -v --standard=Drupal commerce_paybox_payment/
Registering sniffs in the Drupal standard... DONE (120 sniffs registered)
Creating file list... DONE (6 files in queue)
Changing into directory /home/macsim/drupal_modules/commerce_paybox_payment/tests/src/Unit
Processing PaymentControllerTest.php [PHP => 1037 tokens in 139 lines]... DONE in 58ms (0 errors, 0 warnings)
Changing into directory /home/macsim/drupal_modules/commerce_paybox_payment/src/Controller
Processing PaymentController.php [PHP => 676 tokens in 87 lines]... DONE in 29ms (0 errors, 0 warnings)
Changing into directory /home/macsim/drupal_modules/commerce_paybox_payment/src/Plugin/Commerce/PaymentGateway
Processing Paybox.php [PHP => 2697 tokens in 333 lines]... DONE in 129ms (0 errors, 0 warnings)
Changing into directory /home/macsim/drupal_modules/commerce_paybox_payment/src/Access
Processing PaymentReturnAccessCheck.php [PHP => 537 tokens in 80 lines]... DONE in 26ms (0 errors, 0 warnings)
Changing into directory /home/macsim/drupal_modules/commerce_paybox_payment/src/PluginForm
Processing RedirectPayboxForm.php [PHP => 533 tokens in 71 lines]... DONE in 23ms (0 errors, 0 warnings)
Changing into directory /home/macsim/drupal_modules/commerce_paybox_payment/src
Processing SignatureChecker.php [PHP => 471 tokens in 73 lines]... DONE in 20ms (0 errors, 0 warnings)
phpcs -v --standard=DrupalPractice commerce_paybox_payment/
Registering sniffs in the DrupalPractice standard... DONE (38 sniffs registered)
Creating file list... DONE (6 files in queue)
Changing into directory /home/macsim/drupal_modules/commerce_paybox_payment/tests/src/Unit
Processing PaymentControllerTest.php [PHP => 1037 tokens in 139 lines]... DONE in 46ms (0 errors, 0 warnings)
Changing into directory /home/macsim/drupal_modules/commerce_paybox_payment/src/Controller
Processing PaymentController.php [PHP => 676 tokens in 87 lines]... DONE in 27ms (0 errors, 0 warnings)
Changing into directory /home/macsim/drupal_modules/commerce_paybox_payment/src/Plugin/Commerce/PaymentGateway
Processing Paybox.php [PHP => 2697 tokens in 333 lines]... DONE in 94ms (0 errors, 0 warnings)
Changing into directory /home/macsim/drupal_modules/commerce_paybox_payment/src/Access
Processing PaymentReturnAccessCheck.php [PHP => 537 tokens in 80 lines]... DONE in 17ms (0 errors, 0 warnings)
Changing into directory /home/macsim/drupal_modules/commerce_paybox_payment/src/PluginForm
Processing RedirectPayboxForm.php [PHP => 533 tokens in 71 lines]... DONE in 23ms (0 errors, 0 warnings)
Changing into directory /home/macsim/drupal_modules/commerce_paybox_payment/src
Processing SignatureChecker.php [PHP => 471 tokens in 73 lines]... DONE in 16ms (0 errors, 0 warnings)

Manual review

Individual user account
[Yes: Follows] the guidelines for individual user accounts.
No duplication
[Yes: Does not cause] module duplication and/or fragmentation.
Master Branch
[Yes: Follows] the guidelines for master branch.
Licensing
[Yes: Follows] the licensing requirements.
3rd party assets/code
[Yes: Follows] the guidelines for 3rd party assets/code.
README.txt/README.md
[No: Does not follow] the guidelines for in-project documentation and/or the README Template.
Code long/complex enough for review
[Yes: Follows] the guidelines for project length and complexity.
Secure code
[Yes: Meets the security requirements.]
Coding style & Drupal API usage
[List of identified issues in no particular order. Use (*) and (+) to indicate an issue importance. Replace the text below by the issues themselves:
  1. (*) The module doesn't provide a hook_help() nor a README file. Please check guidelines for in-project documentation and/or the README template

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

adelgado12’s picture

Status: Needs work » Needs review
marijan gudelj’s picture

I want to note to the review:
Individual user account: Three maintainers are there, but the review requester did most of the commits

As for the csfixer issues- they are fixed

Also if you need help implementing the refunds and onNotify feel free to contact me

avpaderno’s picture

Status: Needs review » Reviewed & tested by the community
      $this->getLogger('commerce_paybox_payment')
        ->error('Error during api return paybox on payment :payment error :error', [
          'payment' => $payment->id(),
          'error' => $error,
        ]);

With placeholder like :error, the value is is escaped with \Drupal\Component\Utility\Html::escape() and filtered for dangerous protocols using UrlHelper::stripDangerousProtocols(). Since $error isn't used as link, it's useless to filter it for dangerous protocols. The same is true for :payment. The correct placeholder for those cases is @error or @payment.

avpaderno’s picture

Status: Reviewed & tested by the community » Fixed

Thank you for your contribution! I am going to update your account.

These are some recommended readings to help with excellent maintainership:

You can find more contributors chatting on the IRC #drupal-contribute channel. 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.

I thank all the dedicated reviewers as well.

Status: Fixed » Closed (fixed)

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