From 994e15666e5edcc4bf1c74c8bfd33176031ef69b Mon Sep 17 00:00:00 2001
From: rgarcia <voodoorai2000@gmail.com>
Date: Thu, 25 Jan 2018 21:17:25 +0100
Subject: [PATCH] Skip invalid map markers

Why:
Sometimes the latitude or longitude it passed to the map as *********
instead of the actual latitude or longitud. The asterisks are not a
string, breaking the whole array
https://github.com/consul/consul/issues/2380

What:
This commits skips invalid markers and displays the rest

How:
- Substituting the mysterious asterisks for null
(cleanInvestmentCoordinates)

- Validating the coordinates are numbers before trying to pain
them(validCoordinates)

- Adding a numeric function to validate the latitude and longitude
(isNumeric)
https://stackoverflow.com/questions/9716468/is-there-any-function-like-i
snumeric-in-javascript-to-validate-numbers#answer-9716488
---
 app/assets/javascripts/map.js.coffee  | 15 ++++++++
 spec/features/budgets/budgets_spec.rb | 52 +++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/app/assets/javascripts/map.js.coffee b/app/assets/javascripts/map.js.coffee
index ef2c27e28..429b042ca 100644
--- a/app/assets/javascripts/map.js.coffee
+++ b/app/assets/javascripts/map.js.coffee
@@ -12,6 +12,7 @@ App.Map =
           App.Map.toogleMap()
 
   initializeMap: (element) ->
+    App.Map.cleanInvestmentCoordinates(element)
 
     mapCenterLatitude        = $(element).data('map-center-latitude')
     mapCenterLongitude       = $(element).data('map-center-longitude')
@@ -102,7 +103,21 @@ App.Map =
           marker.options['id'] = i.investment_id
 
           marker.on 'click', openMarkerPopup
+          add_marker=createMarker(i.lat , i.long)
+          add_marker.bindPopup(contentPopup(i.investment_title, i.investment_id, i.budget_id))
 
   toogleMap: ->
       $('.map').toggle()
       $('.js-location-map-remove-marker').toggle()
+
+  cleanInvestmentCoordinates: (element) ->
+    markers = $(element).attr('data-marker-investments-coordinates')
+    if markers?
+      clean_markers = markers.replace(/-?(\*+)/g, null)
+      $(element).attr('data-marker-investments-coordinates', clean_markers)
+
+  validCoordinates: (coordinates) ->
+    App.Map.isNumeric(coordinates.lat) && App.Map.isNumeric(coordinates.long)
+
+  isNumeric: (n) ->
+    !isNaN(parseFloat(n)) && isFinite(n)
diff --git a/spec/features/budgets/budgets_spec.rb b/spec/features/budgets/budgets_spec.rb
index cd1ee6cd5..713eb6bfe 100644
--- a/spec/features/budgets/budgets_spec.rb
+++ b/spec/features/budgets/budgets_spec.rb
@@ -191,6 +191,58 @@ feature 'Budgets' do
     expect(page).to have_css(".phase.active", count: 1)
   end
 
+  context "Index map" do
+
+    let(:group) { create(:budget_group, budget: budget) }
+    let(:heading) { create(:budget_heading, group: group) }
+
+    before do
+      Setting['feature.map'] = true
+    end
+
+    scenario "Display investment's map location markers" , :js do
+      investment1 = create(:budget_investment, heading: heading)
+      investment2 = create(:budget_investment, heading: heading)
+      investment3 = create(:budget_investment, heading: heading)
+
+      investment1.create_map_location(longitude: 40.1234, latitude: 3.1234)
+      investment2.create_map_location(longitude: 40.1235, latitude: 3.1235)
+      investment3.create_map_location(longitude: 40.1236, latitude: 3.1236)
+
+      visit budgets_path
+
+      within ".map_location" do
+        expect(page).to have_css(".map-icon", count: 3)
+      end
+    end
+
+    scenario "Skip invalid map markers" , :js do
+      map_locations = []
+      map_locations << { longitude: 40.123456789, latitude: 3.12345678 }
+      map_locations << { longitude: 40.123456789, latitude: "*******" }
+      map_locations << { longitude: "**********", latitude: 3.12345678 }
+
+      budget_map_locations = map_locations.map do |map_location|
+        {
+          lat: map_location[:latitude],
+          long: map_location[:longitude],
+          investment_title: "#{rand(999)}",
+          investment_id: "#{rand(999)}",
+          budget_id: budget.id
+        }
+      end
+
+      allow_any_instance_of(BudgetsHelper).
+      to receive(:current_budget_map_locations).and_return(budget_map_locations)
+
+      visit budgets_path
+
+      within ".map_location" do
+        expect(page).to have_css(".map-icon", count: 1)
+      end
+    end
+  end
+
   context 'Show' do
 
     scenario "List all groups" do