This module is helping with the clustering of points on the server-side. These clusters are then displayed on a map (through other modules)
This has much better performance than client-side when there are 10,000+ points to cluster
There is the possibility to further improve performance by adding new algorithm (as plugins)
It's a port of the D7 module with various fixes & improvements, plus trying to leverage D8 concepts as much as possible.

Project link

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

Git instructions

git clone --branch 8.x-1.x https://git.drupalcode.org/project/geocluster/

PAReview Checklist

https://pareview.sh/pareview/https-git.drupal.org-project-geocluster.git...

CommentFileSizeAuthor
#8 Screenshot 2019-12-19 at 15.53.34.png329 KBvuil
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

vdsh created an issue. See original summary.

vdsh’s picture

Issue summary: View changes
vdsh’s picture

Issue summary: View changes
vuil’s picture

Status: Needs review » Needs work

Thank you for the D8 project contribution!

I saw that maintainer @das-peter of the project (module) has rights to produce Security Coverage and stable version. Am I right?

vdsh’s picture

Status: Needs work » Needs review

Hi Vuil,

And thanks for your comment. Yes, I also think das-peter has the rights to produce Security Coverage versions.

However, as he told me, he is busy with other projects and didn't have much time to work on this for now (last commit quite some time ago)

And I am also co-maintainer for another related module https://www.drupal.org/project/leaflet_geojson which is also not really maintained. So I think it's easier if I can be vetted to get Security Coverage rights and able to do releases covered by Drupal’s security advisory policy.

apaderno’s picture

Issue summary: View changes

As long as the user who applies is the only user who commits code in the branch used for the application, we don't check if the other maintainers have the role to opt into security coverage, especially in the case the project owner ha already the role to opt into security coverage.

vuil’s picture

Status: Needs review » Needs work

OK, Thank you for the contribution!

1. Please use the better construction as

$display_extenders = $config->get('display_extenders') ?? []; 

instead of

$display_extenders = $config->get('display_extenders') ? $config->get('display_extenders') : []

in geocluster.install file and in Drupal\geocluster\GeoclusterConfig class.

2. Please replace deprecated drupal_set_message() with \Drupal::messenger()->addMessage() in Drupal\geocluster\GeoclusterConfig.

3. Update your t() methods with $this->t() and add use StringTranslationTrait; statement under class declaration:

4 Please remove one of the lines

    $maxResolution = GEOFIELD_KILOMETERS * 1000 / $tile_size;
    $maxResolution = 156.412 * 1000;

in Drupal\geocluster\Utility\GeoclusterHelper

5. Please fix the __construct() method issue in Drupal\geocluster\Plugin\GeoclusterAlgorithmBase (attached screenshot).

6. There are missing @return tags in code annotations, please fix all of them.

vuil’s picture

I added the screenshot of #7 5.) issue.

vdsh’s picture

Status: Needs work » Needs review

Thanks vuil for this detailed analysis. I have corrected all the things you mentionned (and slightly cleaned further)

klausi’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for your contribution!

  1. "define('GEOCLUSTER_VIEWS_SECTION', 'style_options');": do not use define() in Drupal 8 code. Use a constant on a class instead. That way it always works with autoloading. Same for other constants.
  2. 'Drupal\geocluster\Plugin\GeoclusterAlgorithmInterface': instead of using class names in srings you should use ::class syntax. That way IDEs can check typos for you.

Otherwise looks good to me, did not see security issues.

apaderno’s picture

Assigned: Unassigned » apaderno
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.