[D8] GoBookings Integration

Multiple Applications

It appears that there have been multiple project applications opened under your username:

Project 1: https://www.drupal.org/node/2758935

Project 2: https://www.drupal.org/node/2758941

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

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

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

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 b.ravanbakhsh,

There are some issues in this module. Please go through the below pareview.sh URL.

http://pareview.sh/pareview/httpsgitdrupalorgsandboxbravanbakhsh2709633git

Thanks

@hiramanpatil : Looks like you forgot to change the status. Is this now RTBC after your review or are there application blockers left and this should be "needs work"?

Updating status to 'Needs work'.

Hi Have you considered adding in some testing ? https://www.drupal.org/simpletest

It's not required but would get you bonus points in the reviews eyes.

Hi,
here is my bit of review.

1. Please inject the 'config.factory' service in GobookingsSettingsForm.php
2. Replace the parameters in $config->get() with the actual field names.
i.e. $config->get('UserID')
with
$config->get('user_id')

Similarly at other places.
Replace the first parameter in set() accordingly. Make these changes in other files as well, wherever required.

3. Replace the private members with protected in Gobookings.php
4. Remove this commented part in Gobookings.php if not required.

/*
      }
      catch (ClientException $ce) {
      $this->helper->logMessage('ClientException', $this->t('Err at Client'));
      }
      catch (ServerException $se) {
      $this->helper->logMessage($this->t('There was an error at the server'));
      }
      catch (RequestException $re) {
      $this->helper->logMessage($this->t('An unknown error occurred.'));
      }

      if ($response instanceof \GuzzleHttp\Psr7\Response) {
       */

5. Give appropriate whitelines before and after comments in GobookingsHelper.php
6. Inject the logger service in Gobookings.php, instead of using static functions.

I have not gone through the functionality on Drupal site.
Please make these changes.

Removing the PAreview bonus tag as per the Drupal rules.

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.

Thank you for your reviews. When finishing your review comment also set the issue status either to "needs work" (you found some problems with the project) or "reviewed & tested by the community" (you found no major flaws).

Review of the 8.x-1.x branch (commit f974c82):

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
  

  
  FILE: /home/klausi/pareview_temp/src/GoBookingsHelper.php
  --------------------------------------------------------------------------
  FOUND 1 ERROR AFFECTING 1 LINE
  --------------------------------------------------------------------------
   17 | ERROR | [x] Equals sign not aligned with surrounding assignments;
      |       |     expected 1 space but found 2 spaces
  --------------------------------------------------------------------------
  PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
  --------------------------------------------------------------------------
  

• No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

1. project page: what is the purpose of this module? How does it work? How does it integrate into Drupal? Does it create nodes? Please fill out your project page according to https://www.drupal.org/node/997024
2. gobookings.routing.yml: your settings page uses the wrong permission. 'access administration pages' is only about viewing admin pages, but your form allows changes. Use "administer site configuration" or define your own permission instead. This is a security blocker right now. And please don't remove the security tag, we keep that for statistics and to show examples of security problems.
3. config schema for your config file is missing, see https://www.drupal.org/docs/8/api/configuration-api/configuration-schema...
4. GoBookingsHelper::tidyUpXml(): do not use regular expressions to parse XML. If you need to get rid of XML namespaces use something like http://stackoverflow.com/a/18994815/2000435
5. GoBookingsHelper::logMessage(): this is missing support for variables in log messages. Never concatenate variables directly to log messages, pass them as context/placeholders to the logger. The Gobookings class does many wrong logs that should use context instead. See https://drupalize.me/blog/201510/how-log-messages-drupal-8 for examples.
6. So this module does not do anything on its own right now, correct? A user will have to write their own module to use the API service you provide? Please also document that on the project page.

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.