The NTH Mobile module enables payments in Drupal through NTH Mobile. The module provides events that other modules can subscribe to and build their business logic around.

Only Premium SMS is supported at this moment.

Project link

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

Git instructions

git clone --branch '1.0.x' https://git.drupalcode.org/project/nth_mobile.git

PAReview Link:

http://pareview.net/r/488

CommentFileSizeAuthor
#6 nth-mobile-admin-config.png45.49 KBtim-diels

Comments

tim-diels created an issue. See original summary.

avpaderno’s picture

Issue summary: View changes

Thank you for applying!
Only reviews for other Drupal.org security advisory coverage applications are relevant for us.

tim-diels’s picture

Ah sorry didn't know that, thanks for that feedback. Is there any more information you need to help in the review?

avpaderno’s picture

Status: Needs review » Needs work
  • What follows is a quick review of the project; it doesn't mean to be complete
  • For every point, the review doesn't make a complete list of lines that should be fixed, but an example of what is wrong in the code
  • A review is about code that doesn't follow the coding standards, contains possible security issue, or doesn't correctly use the Drupal API; if a point isn't about that, it makes it clear
  /**
   * The entity type manager.
   *
   * @var \Drupal\Core\Entity\EntityTypeManagerInterface
   */
  protected $entityTypeManager;

That property is already defined from the parent class; there is no need to define it again.

nth_mobile.admin_config:
  path: '/admin/config/services/nth-mobile'
  defaults:
    _title: 'NTH Mobile'
    _controller: '\Drupal\system\Controller\SystemController::systemAdminMenuBlockPage'
  requirements:
    _permission: 'administer nth_mobile configuration'

What is supposed to show, that route? It seem the route for the settings form, but it doesn't have any associated form.

    $entity = \Drupal::entityTypeManager()
      ->getStorage('nth_mobile_psms_transaction')
      ->load($data->retryMessageRef);

Classes that implement ContainerFactoryPluginInterface don't use \Drupal.

rajeshreeputra’s picture

Issue summary: View changes

Added PAReview link: http://pareview.net/r/488

tim-diels’s picture

Status: Needs work » Needs review
StatusFileSize
new45.49 KB

@apaderno thank you very much for the review, I really appreciate the time you took to analyse the module.
@Rajeshreeputra thanks for the PAReview link, that service didn't work a couple of weeks ago.

I'll do my best to answer with as much information as possible to help in understanding the module.

The route nth_mobile.admin_config shows a list of sub routes like the nth_mobile_psms.settings_form from the submodule NTH Mobile PSMS and can have more sub routes when more modules or routes to current modules are added in the future. I added a screenshot displaying what is shown on the page.

Only local images are allowed.

The raised issues are fixed with #3246568: Issues raised by the Drupal.org security advisory coverage applications in the 1.0.x-dev branch

marijan gudelj’s picture

@tim-diels before I go fully in the review mode can you please help me with something ?
Does this payment gateway need the Commerce module or any other E-commerce module?
Because I cannot see how you log the payment statuses. Also without any e-commerce module I fail to see the use case here.

tim-diels’s picture

@marijan This module does not need any commerce related modules. It provides webhooks with events to allow to interact with NTH Mobile. A developer needs to build logic using eventsubscribers. There is an example module explaining what someone needs to do. It is possible to use this in conjunction with commerce modules. But I was not in the need to make that for my use case. If one needs that, I can create a submodule for that or one can create a module on itself. I can always show you in private how I implemented this module for one of my clients.

avpaderno’s picture

Assigned: Unassigned » avpaderno
Status: Needs review » 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.

tim-diels’s picture

Thank you very much for the approval!

Status: Fixed » Closed (fixed)

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