[D8] GoogleMap Block

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

Fixed the git clone URL in the issue summary for non-maintainer users.

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 N Pandey ,

There are some issues at certain places.

You have used services directly on some places like EditLocation.php \Drupal::service('path.current')->getPath(). while you can inject these services directly.

And also you have used direct sql queries which are not recomended in Drupal 8.

Manual Review

Apart from what is mentioned in #5 and #6, i have a suggestion.

1. The @file doc block MUST be present for all PHP files, with one exception: files that contain a namespaced class/interface/trait, whose file name is the class name with a .php extension, and whose file path is closely related to the namespace (under PSR-4 or a similar standard), SHOULD NOT have a @file documentation block.

   I think you could remove @file comment from namespaced classes.

2.    $map_height = \Drupal::config('googlemap_block.settings')->get('map_height');
       $map_width = \Drupal::config('googlemap_block.settings')->get('map_width');
       $map_zoom_level = \Drupal::config('googlemap_block.settings')->get('map_zoom_level');
       $lat = \Drupal::config('googlemap_block.settings')->get('lat');
       $long = \Drupal::config('googlemap_block.settings')->get('long');
   

   Use dependency injection instead.

After the changes in #5, #6 . I guess this would be good to go :)

Hi @B N Pandey ,

There are some issues at some places like.

1. In EditLocation.php file you have used

$redirect = new RedirectResponse($base_url . '/admin/structure/gmap-location');
    $redirect->send();

Correct way to redirect form is
$form_state->setRedirectUrl($url);

2. Same issue in file "MapLocationDelete.php"

$redirect = new RedirectResponse($base_url . '/admin/structure/gmap-location');
    $redirect->send();

Correct way to redirect form is
$form_state->setRedirectUrl($url);

3. In file "GoogleMapBlock.php" you have used the code like

   $map_height = \Drupal::config('googlemap_block.settings')->get('map_height');
    $map_width = \Drupal::config('googlemap_block.settings')->get('map_width');
    $map_zoom_level = \Drupal::config('googlemap_block.settings')->get('map_zoom_level');
    $lat = \Drupal::config('googlemap_block.settings')->get('lat');
    $long = \Drupal::config('googlemap_block.settings')->get('long');

Recomanded way to avoid the create the configurtion object again and again for each variable is

   $config = \Drupal::config('googlemap_block.settings');
    $map_height = $config->get('map_height');
    $map_width = $config->get('map_width');
    $map_zoom_level = $config->get('map_zoom_level');
    $lat = $config->get('lat');
    $long = $config->get('long');

Hi B N Panday,

Please fix the text in file "EditLocation.php" at line number 60 current text is "Add Location" please replace with "Edit Location"

Please fix this rest works fine.

Hi, @B N Pandey
Please, use dependency injection instead of \Drupal calls according to Drupal Best Practices (e.g: GoogleMapBlock.php [line:34]).
Thanks)

Automated Review

A warnings was found by pareview.sh (https://pareview.sh/node/1046)

• DrupalPractice: avoid static call, do dependency injection instead

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.

The README.txt is very empty, you should add some help for futur developer and maintenances.Also, all but the most trivial modules should implement hook_help().

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

1. (+) Fix a typo in file EditLocation.php "Add Location" -> "Edit Location"
2. (+) Use translation for drupal_set_message. I saw some code such drupal_set_message($location_name . " location has been Updated") that must be drupal_set_message($this->t('@location location has been Updated', ['@location' => $location_name]))
3. Just a recommendation, avoid static call use injection instead
4. To follow best practice in the Drupal Community, you should create an Entity for your Locations instead of custom table google_map_location_list. Create a custom content entity
5. Just a recommendation, should create a permission to manage the locations

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.

This review uses the Project Application Review Template.

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.

Hi @BN Pandey
This module works for me. I review this module manually and did not find any error.

Thanks

Thank you for your contribution!

I updated your account so you can opt into security advisory coverage now.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

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

Thanks go the dedicated reviewer(s) as well.