Merge pull request #5116 from consuldemocracy/marker_clustering

Add map markers clustering feature

Gemfile

@@ -35,7 +35,6 @@ gem "initialjs-rails", "~> 0.2.0.9"
 gem "invisible_captcha", "~> 2.1.0"
 gem "jquery-fileupload-rails"
 gem "kaminari", "~> 1.2.2"
-gem "leaflet-rails", "~> 1.9.3"
 gem "mini_magick", "~> 4.12.0"
 gem "omniauth", "~> 2.1.1"
 gem "omniauth-facebook", "~> 9.0.0"

Gemfile.lock

@@ -317,8 +317,6 @@ GEM
     language_server-protocol (3.17.0.3)
     launchy (2.5.2)
       addressable (~> 2.8)
-    leaflet-rails (1.9.3)
-      rails (>= 4.2.0)
     letter_opener (1.8.1)
       launchy (>= 2.2, < 3)
     letter_opener_web (2.0.0)
@@ -740,7 +738,6 @@ DEPENDENCIES
   kaminari (~> 1.2.2)
   knapsack_pro (~> 5.7.0)
   launchy (~> 2.5.2)
-  leaflet-rails (~> 1.9.3)
   letter_opener_web (~> 2.0.0)
   mdl (~> 0.12.0)
   mini_magick (~> 4.12.0)

app/assets/javascripts/application.js

@@ -104,7 +104,8 @@
 //= require tree_navigator
 //= require tag_autocomplete
 //= require polls_admin
-//= require leaflet
+//= require leaflet/dist/leaflet
+//= require leaflet.markercluster/dist/leaflet.markercluster
 //= require map
 //= require polls
 //= require sortable

app/assets/javascripts/map.js

@@ -15,12 +15,18 @@
       App.Map.maps = [];
     },
     initializeMap: function(element) {
-      var createMarker, editable, investmentsMarkers, markerData, map, marker,
+      var createMarker, editable, investmentsMarkers, map, marker, markerClustering,
-        markerIcon, moveOrPlaceMarker, removeMarker, removeMarkerSelector;
+        markerData, markerIcon, markers, moveOrPlaceMarker, removeMarker, removeMarkerSelector;
       App.Map.cleanInvestmentCoordinates(element);
       removeMarkerSelector = $(element).data("marker-remove-selector");
       investmentsMarkers = $(element).data("marker-investments-coordinates");
       editable = $(element).data("marker-editable");
+      markerClustering = $(element).data("marker-clustering");
+      if (markerClustering) {
+        markers = L.markerClusterGroup({ chunkedLoading: true });
+      } else {
+        markers = L.layerGroup();
+      }
       marker = null;
       markerIcon = L.divIcon({
         className: "map-marker",
@@ -40,7 +46,7 @@
             App.Map.updateFormfields(map, newMarker);
           });
         }
-        newMarker.addTo(map);
+        markers.addLayer(newMarker);
         return newMarker;
       };
       removeMarker = function() {
@@ -79,6 +85,7 @@
 
       App.Map.addInvestmentsMarkers(investmentsMarkers, createMarker);
       App.Map.addGeozones(map);
+      map.addLayer(markers);
     },
     leafletMap: function(element) {
       var centerData, mapCenterLatLng, map;

app/assets/stylesheets/application.scss

@@ -11,7 +11,9 @@
 @import "jquery-ui/themes/base/autocomplete";
 @import "jquery-ui/themes/base/datepicker";
 @import "jquery-ui/themes/base/sortable";
-@import "leaflet";
+@import "leaflet/dist/leaflet";
+@import "leaflet.markercluster/dist/MarkerCluster";
+@import "leaflet.markercluster/dist/MarkerCluster.Default";
 
 @import "foundation_and_overrides";
 @import "fonts";

app/components/admin/settings/map_tab_component.html.erb

@@ -5,6 +5,7 @@
     <% settings.each do |key| %>
       <%= render Admin::Settings::RowComponent.new(key, tab: tab) %>
     <% end %>
+    <%= render Admin::Settings::RowComponent.new("map.feature.marker_clustering", type: :feature, tab: tab) %>
   <% end %>
 
   <p><%= t("admin.settings.index.map.help") %></p>

app/components/shared/map_location_component.rb

@@ -58,6 +58,7 @@ class Shared::MapLocationComponent < ApplicationComponent
         marker_investments_coordinates: investments_coordinates,
         marker_latitude: map_location.latitude.presence,
         marker_longitude: map_location.longitude.presence,
+        marker_clustering: feature?("map.feature.marker_clustering"),
         geozones: geozones_data
       }.merge(input_selectors)
     end

app/helpers/settings_helper.rb

@@ -9,7 +9,7 @@ module SettingsHelper
   end
 
   def feature?(name)
-    setting["feature.#{name}"].presence || setting["process.#{name}"].presence
+    setting["feature.#{name}"].presence || setting["process.#{name}"].presence || setting[name].presence
   end
 
   def setting

app/models/setting.rb

@@ -99,6 +99,7 @@ class Setting < ApplicationRecord
         "map.latitude": 51.48,
         "map.longitude": 0.0,
         "map.zoom": 10,
+        "map.feature.marker_clustering": false,
         "process.debates": true,
         "process.proposals": true,
         "process.polls": true,

config/locales/en/settings.yml

@@ -169,6 +169,9 @@ en:
         valid: "Condition for detecting a valid response"
         valid_description: "What response path has to come informed to be considered a valid response and user verified"
     map:
+      feature:
+        marker_clustering: "Marker clustering"
+        marker_clustering_description: "Enables map markers clusterization. Useful when there are a lot of markers to show."
       latitude: "Latitude"
       latitude_description: "Latitude to show the map position"
       longitude: "Longitude"

config/locales/es/settings.yml

@@ -169,6 +169,9 @@ es:
         valid: "Condición para detectar una respuesta válida"
         valid_description: "Que ruta de la respuesta tiene que venir informado para considerarse una respuesta válida"
     map:
+      feature:
+        marker_clustering: "Agrupación de marcadores"
+        marker_clustering_description: "Activa la agrupación de marcadores en el mapa. Útil cuando hay muchos marcadores que mostrar."
       latitude: "Latitud"
       latitude_description: "Latitud para mostrar la posición del mapa"
       longitude: "Longitud"

db/dev_seeds/budgets.rb

@@ -57,8 +57,8 @@ section "Creating Budgets" do
       {
         price: 1000000,
         population: 1000000,
-        latitude: "40.416775",
+        latitude: Setting["map.latitude"],
-        longitude: "-3.703790"
+        longitude: Setting["map.longitude"]
       }.merge(
         random_locales_attributes(name: -> { I18n.t("seeds.budgets.groups.all_city") })
       )
@@ -84,8 +84,8 @@ section "Creating Budgets" do
     ].each do |heading_params|
       districts_group.headings.create!(heading_params.merge(
         price: rand(5..10) * 100000,
-        latitude: "40.416775",
+        latitude: Setting["map.latitude"],
-        longitude: "-3.703790"
+        longitude: Setting["map.longitude"]
       ))
     end
   end

package-lock.json

@@ -8,7 +8,9 @@
       "dependencies": {
         "jquery": "^3.7.1",
         "jquery-ui": "^1.13.2",
-        "jquery-ujs": "^1.2.3"
+        "jquery-ujs": "^1.2.3",
+        "leaflet": "^1.9.4",
+        "leaflet.markercluster": "^1.5.3"
       }
     },
     "node_modules/jquery": {
@@ -31,6 +33,19 @@
       "peerDependencies": {
         "jquery": ">=1.8.0"
       }
+    },
+    "node_modules/leaflet": {
+      "version": "1.9.4",
+      "resolved": "https://registry.npmjs.org/leaflet/-/leaflet-1.9.4.tgz",
+      "integrity": "sha512-nxS1ynzJOmOlHp+iL3FyWqK89GtNL8U8rvlMOsQdTTssxZwCXh8N2NB3GDQOL+YR3XnWyZAxwQixURb+FA74PA=="
+    },
+    "node_modules/leaflet.markercluster": {
+      "version": "1.5.3",
+      "resolved": "https://registry.npmjs.org/leaflet.markercluster/-/leaflet.markercluster-1.5.3.tgz",
+      "integrity": "sha512-vPTw/Bndq7eQHjLBVlWpnGeLa3t+3zGiuM7fJwCkiMFq+nmRuG3RI3f7f4N4TDX7T4NpbAXpR2+NTRSEGfCSeA==",
+      "peerDependencies": {
+        "leaflet": "^1.3.1"
+      }
     }
   }
 }

package.json

@@ -3,6 +3,8 @@
   "dependencies": {
     "jquery": "^3.7.1",
     "jquery-ui": "^1.13.2",
-    "jquery-ujs": "^1.2.3"
+    "jquery-ujs": "^1.2.3",
+    "leaflet": "^1.9.4",
+    "leaflet.markercluster": "^1.5.3"
   }
 }

spec/factories/administration.rb

@@ -74,9 +74,9 @@ FactoryBot.define do
   end
 
   factory :map_location do
-    latitude { 51.48 }
+    latitude { Setting["map.latitude"] }
-    longitude { 0.0 }
+    longitude { Setting["map.longitude"] }
-    zoom { 10 }
+    zoom { Setting["map.zoom"] }
 
     trait :proposal_map_location do
       proposal

spec/factories/budgets.rb

@@ -91,8 +91,8 @@ FactoryBot.define do
     sequence(:name) { |n| "Heading #{n}" }
     price { 1000000 }
     population { 1234 }
-    latitude { "40.416775" }
+    latitude { Setting["map.latitude"] }
-    longitude { "-3.703790" }
+    longitude { Setting["map.longitude"] }
 
     transient { budget { nil } }
     group { association :budget_group, budget: budget || association(:budget) }
@@ -198,7 +198,11 @@ FactoryBot.define do
     end
 
     trait :with_map_location do
-      map_location
+      map_location do
+        association :map_location,
+                    longitude: heading.longitude.to_f + rand(-0.0001..0.0001),
+                    latitude: heading.latitude.to_f + rand(-0.0001..0.0001)
+      end
     end
 
     trait :flagged do

spec/helpers/settings_helper_spec.rb

@@ -13,15 +13,46 @@ RSpec.describe SettingsHelper do
   end
 
   describe "#feature?" do
-    it "returns presence of feature flag setting value" do
+    it "finds settings by the given name prefixed with 'feature.' and returns its presence" do
       Setting["feature.f1"] = "active"
-      Setting["feature.f2"] = ""
+      Setting["feature.f2"] = true
-      Setting["feature.f3"] = nil
+      Setting["feature.f3"] = false
+      Setting["feature.f4"] = ""
+      Setting["feature.f5"] = nil
 
       expect(feature?("f1")).to eq("active")
-      expect(feature?("f2")).to be nil
+      expect(feature?("f2")).to eq("t")
       expect(feature?("f3")).to be nil
       expect(feature?("f4")).to be nil
+      expect(feature?("f5")).to be nil
+    end
+
+    it "finds settings by the given name prefixed with 'process.' and returns its presence" do
+      Setting["process.p1"] = "active"
+      Setting["process.p2"] = true
+      Setting["process.p3"] = false
+      Setting["process.p4"] = ""
+      Setting["process.p5"] = nil
+
+      expect(feature?("p1")).to eq("active")
+      expect(feature?("p2")).to eq("t")
+      expect(feature?("p3")).to be nil
+      expect(feature?("p4")).to be nil
+      expect(feature?("p5")).to be nil
+    end
+
+    it "finds settings by the full key name and returns its presence" do
+      Setting["map.feature.f1"] = "active"
+      Setting["map.feature.f2"] = true
+      Setting["map.feature.f3"] = false
+      Setting["map.feature.f4"] = ""
+      Setting["map.feature.f5"] = nil
+
+      expect(feature?("map.feature.f1")).to eq("active")
+      expect(feature?("map.feature.f2")).to eq("t")
+      expect(feature?("map.feature.f3")).to be nil
+      expect(feature?("map.feature.f4")).to be nil
+      expect(feature?("map.feature.f5")).to be nil
     end
   end
 end

spec/system/budgets/budgets_spec.rb

@@ -260,13 +260,7 @@ describe "Budgets" do
     end
 
     scenario "Display investment's map location markers" do
-      investment1 = create(:budget_investment, heading: heading)
+      create_list(:budget_investment, 3, :with_map_location, heading: heading)
-      investment2 = create(:budget_investment, heading: heading)
-      investment3 = create(:budget_investment, heading: heading)
-
-      create(:map_location, longitude: 40.1234, latitude: -3.634, investment: investment1)
-      create(:map_location, longitude: 40.1235, latitude: -3.635, investment: investment2)
-      create(:map_location, longitude: 40.1236, latitude: -3.636, investment: investment3)
 
       visit budgets_path
 
@@ -277,16 +271,7 @@ describe "Budgets" do
 
     scenario "Display all investment's map location if there are no selected" do
       budget.update!(phase: :publishing_prices)
-
+      create_list(:budget_investment, 4, :with_map_location, heading: heading)
-      investment1 = create(:budget_investment, heading: heading)
-      investment2 = create(:budget_investment, heading: heading)
-      investment3 = create(:budget_investment, heading: heading)
-      investment4 = create(:budget_investment, heading: heading)
-
-      investment1.create_map_location(longitude: 40.1234, latitude: 3.1234, zoom: 10)
-      investment2.create_map_location(longitude: 40.1235, latitude: 3.1235, zoom: 10)
-      investment3.create_map_location(longitude: 40.1236, latitude: 3.1236, zoom: 10)
-      investment4.create_map_location(longitude: 40.1240, latitude: 3.1240, zoom: 10)
 
       visit budgets_path
 
@@ -297,16 +282,8 @@ describe "Budgets" do
 
     scenario "Display only selected investment's map location from publishing prices phase" do
       budget.update!(phase: :publishing_prices)
-
+      create_list(:budget_investment, 2, :selected, :with_map_location, heading: heading)
-      investment1 = create(:budget_investment, :selected, heading: heading)
+      create_list(:budget_investment, 2, :with_map_location, heading: heading)
-      investment2 = create(:budget_investment, :selected, heading: heading)
-      investment3 = create(:budget_investment, heading: heading)
-      investment4 = create(:budget_investment, heading: heading)
-
-      investment1.create_map_location(longitude: 40.1234, latitude: 3.1234, zoom: 10)
-      investment2.create_map_location(longitude: 40.1235, latitude: 3.1235, zoom: 10)
-      investment3.create_map_location(longitude: 40.1236, latitude: 3.1236, zoom: 10)
-      investment4.create_map_location(longitude: 40.1240, latitude: 3.1240, zoom: 10)
 
       visit budgets_path
 
@@ -320,9 +297,9 @@ describe "Budgets" do
 
       investment = create(:budget_investment, heading: heading)
 
-      map_locations << { longitude: 40.123456789, latitude: 3.12345678 }
+      map_locations << { longitude: -3.703790, latitude: 40.416775 }
-      map_locations << { longitude: 40.123456789, latitude: "********" }
+      map_locations << { longitude: -3.703791, latitude: "********" }
-      map_locations << { longitude: "**********", latitude: 3.12345678 }
+      map_locations << { longitude: "**********", latitude: 40.416776 }
 
       coordinates = map_locations.map do |map_location|
         {
@@ -342,6 +319,22 @@ describe "Budgets" do
         expect(page).to have_css(".map-icon", count: 1, visible: :all)
       end
     end
+
+    scenario "when the marker clustering feature is enabled the map shows clusters instead of markers" do
+      Setting["map.feature.marker_clustering"] = true
+      create_list(:budget_investment, 3, :selected, :with_map_location, heading: heading)
+
+      visit budgets_path
+
+      within ".map-location" do
+        expect(page).to have_css ".marker-cluster div span", text: "3"
+        expect(page).not_to have_css ".map-icon"
+
+        find(".marker-cluster").click
+
+        expect(page).to have_css ".map-icon", count: 3
+      end
+    end
   end
 
   context "Show" do

spec/system/budgets/investments_spec.rb

@@ -1643,20 +1643,7 @@ describe "Budget Investments" do
 
   context "sidebar map" do
     scenario "Display 6 investment's markers on sidebar map" do
-      investment1 = create(:budget_investment, heading: heading)
+      create_list(:budget_investment, 6, :with_map_location, heading: heading)
-      investment2 = create(:budget_investment, heading: heading)
-      investment3 = create(:budget_investment, heading: heading)
-      investment4 = create(:budget_investment, heading: heading)
-      investment5 = create(:budget_investment, heading: heading)
-      investment6 = create(:budget_investment, heading: heading)
-
-      create(:map_location, longitude: 40.1231, latitude: -3.636, investment: investment1)
-      create(:map_location, longitude: 40.1232, latitude: -3.635, investment: investment2)
-      create(:map_location, longitude: 40.1233, latitude: -3.634, investment: investment3)
-      create(:map_location, longitude: 40.1234, latitude: -3.633, investment: investment4)
-      create(:map_location, longitude: 40.1235, latitude: -3.632, investment: investment5)
-      create(:map_location, longitude: 40.1236, latitude: -3.631, investment: investment6)
-
       visit budget_investments_path(budget, heading_id: heading.id)
 
       within ".map-location" do
@@ -1664,36 +1651,10 @@ describe "Budget Investments" do
       end
     end
 
-    scenario "Display 2 investment's markers on sidebar map" do
-      investment1 = create(:budget_investment, heading: heading)
-      investment2 = create(:budget_investment, heading: heading)
-
-      create(:map_location, longitude: 40.1281, latitude: -3.656, investment: investment1)
-      create(:map_location, longitude: 40.1292, latitude: -3.665, investment: investment2)
-
-      visit budget_investments_path(budget, heading_id: heading.id)
-
-      within ".map-location" do
-        expect(page).to have_css(".map-icon", count: 2, visible: :all)
-      end
-    end
-
     scenario "Display only investment's related to the current heading" do
       heading_2 = create(:budget_heading, name: "Madrid", group: group)
-
+      create_list(:budget_investment, 4, :with_map_location, heading: heading)
-      investment1 = create(:budget_investment, heading: heading)
+      create_list(:budget_investment, 2, :with_map_location, heading: heading_2)
-      investment2 = create(:budget_investment, heading: heading)
-      investment3 = create(:budget_investment, heading: heading)
-      investment4 = create(:budget_investment, heading: heading)
-      investment5 = create(:budget_investment, heading: heading_2)
-      investment6 = create(:budget_investment, heading: heading_2)
-
-      create(:map_location, longitude: 40.1231, latitude: -3.636, investment: investment1)
-      create(:map_location, longitude: 40.1232, latitude: -3.685, investment: investment2)
-      create(:map_location, longitude: 40.1233, latitude: -3.664, investment: investment3)
-      create(:map_location, longitude: 40.1234, latitude: -3.673, investment: investment4)
-      create(:map_location, longitude: 40.1235, latitude: -3.672, investment: investment5)
-      create(:map_location, longitude: 40.1236, latitude: -3.621, investment: investment6)
 
       visit budget_investments_path(budget, heading_id: heading.id)
 
@@ -1704,14 +1665,7 @@ describe "Budget Investments" do
 
     scenario "Do not display investment's, since they're all related to other heading" do
       heading_2 = create(:budget_heading, name: "Madrid", group: group)
-
+      create_list(:budget_investment, 3, :with_map_location, heading: heading_2)
-      investment1 = create(:budget_investment, heading: heading_2)
-      investment2 = create(:budget_investment, heading: heading_2)
-      investment3 = create(:budget_investment, heading: heading_2)
-
-      create(:map_location, longitude: 40.1255, latitude: -3.644, investment: investment1)
-      create(:map_location, longitude: 40.1258, latitude: -3.637, investment: investment2)
-      create(:map_location, longitude: 40.1251, latitude: -3.649, investment: investment3)
 
       visit budget_investments_path(budget, heading_id: heading.id)