From c40697cf615eb7850a1f8460bfc43d40d9201a11 Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Fri, 26 Jun 2020 15:45:58 +0100 Subject: [PATCH 1/4] If no enterprises have been geocoded yet make sure Open Street Map still displays correctly. Before it would display a gray/blank div instead of map because the map latitude, longitude couldn't be calculated without geocoded enterprises. This adds a setting so the default coordinates can be set even if no geocoded enterprises present. --- .../directives/open_street_map.js.coffee | 41 ++++++++++++++----- app/models/content_configuration.rb | 2 + app/models/preference_sections/map_section.rb | 4 +- .../api/open_street_map_config_serializer.rb | 12 +++++- 4 files changed, 46 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/darkswarm/directives/open_street_map.js.coffee b/app/assets/javascripts/darkswarm/directives/open_street_map.js.coffee index 7dbdc73669..e9be4acb88 100644 --- a/app/assets/javascripts/darkswarm/directives/open_street_map.js.coffee +++ b/app/assets/javascripts/darkswarm/directives/open_street_map.js.coffee @@ -61,28 +61,47 @@ Darkswarm.directive 'ofnOpenStreetMap', ($window, Enterprises, EnterpriseModal, displayMap = -> setMapDimensions() - averageLatitude = averageAngle("latitude") - averageLongitude = averageAngle("longitude") zoomLevel = 6 map = L.map('open-street-map') L.tileLayer.provider(openStreetMapProviderName, openStreetMapProviderOptions).addTo(map) - map.setView([averageLatitude, averageLongitude], zoomLevel) + map.setView([initialLatitude(), initialLongitude()], zoomLevel) displayEnterprises = -> - for enterprise in Enterprises.enterprises - if enterprise.latitude? && enterprise.longitude? - marker = buildMarker(enterprise, { lat: enterprise.latitude, lng: enterprise.longitude }, enterprise.name).addTo(map) - enterpriseNames.push(enterpriseName(enterprise)) - markers.push(marker) + for enterprise in geocodedEnterprises() + marker = buildMarker(enterprise, { lat: enterprise.latitude, lng: enterprise.longitude }, enterprise.name).addTo(map) + enterpriseNames.push(enterpriseName(enterprise)) + markers.push(marker) + + disableSearchField = () => + $('#open-street-map--search input').prop("disabled", true) displaySearchField = () -> new Autocomplete('#open-street-map--search', onSubmit: goToEnterprise search: searchEnterprises ) - overwriteInlinePositionRelativeToPositionSearchField = -> - $('#open-street-map--search').css("position", "absolute") - overwriteInlinePositionRelativeToPositionSearchField() + overwriteInlinePositionRelativeToAbsoluteOnSearchField() + if geocodedEnterprises().length == 0 + disableSearchField() + + geocodedEnterprises = () -> + Enterprises.enterprises.filter (enterprise) -> + enterprise.latitude? && enterprise.longitude? + + initialLatitude = () -> + if geocodedEnterprises().length > 0 + averageAngle("latitude") + else + openStreetMapConfig.open_street_map_default_latitude || -37.4713077 + + initialLongitude = () -> + if geocodedEnterprises().length > 0 + averageAngle("longitude") + else + openStreetMapConfig.open_street_map_default_longitude || 144.7851531 + + overwriteInlinePositionRelativeToAbsoluteOnSearchField = -> + $('#open-street-map--search').css("position", "absolute") searchEnterprises = (input) -> if input.length < 1 diff --git a/app/models/content_configuration.rb b/app/models/content_configuration.rb index 29ab8f3308..245140f712 100644 --- a/app/models/content_configuration.rb +++ b/app/models/content_configuration.rb @@ -21,6 +21,8 @@ class ContentConfiguration < Spree::Preferences::FileConfiguration preference :open_street_map_enabled, :boolean, default: false preference :open_street_map_provider_name, :string, default: "OpenStreetMap.Mapnik" preference :open_street_map_provider_options, :text, default: "{}" + preference :open_street_map_default_latitude, :string, default: "-37.4713077" + preference :open_street_map_default_longitude, :string, default: "144.7851531" # Producer sign-up page # All the following defaults using I18n don't work. diff --git a/app/models/preference_sections/map_section.rb b/app/models/preference_sections/map_section.rb index cc16939c36..deed329b44 100644 --- a/app/models/preference_sections/map_section.rb +++ b/app/models/preference_sections/map_section.rb @@ -10,7 +10,9 @@ module PreferenceSections [ :open_street_map_enabled, :open_street_map_provider_name, - :open_street_map_provider_options + :open_street_map_provider_options, + :open_street_map_default_latitude, + :open_street_map_default_longitude ] end end diff --git a/app/serializers/api/open_street_map_config_serializer.rb b/app/serializers/api/open_street_map_config_serializer.rb index 8c5aec5dbb..b24cdcbd41 100644 --- a/app/serializers/api/open_street_map_config_serializer.rb +++ b/app/serializers/api/open_street_map_config_serializer.rb @@ -4,7 +4,9 @@ module Api class OpenStreetMapConfigSerializer < ActiveModel::Serializer attributes :open_street_map_enabled, :open_street_map_provider_name, - :open_street_map_provider_options + :open_street_map_provider_options, + :open_street_map_default_latitude, + :open_street_map_default_longitude def open_street_map_enabled ContentConfig.open_street_map_enabled @@ -17,5 +19,13 @@ module Api def open_street_map_provider_options ContentConfig.open_street_map_provider_options.to_json end + + def open_street_map_default_latitude + ContentConfig.open_street_map_default_latitude + end + + def open_street_map_default_longitude + ContentConfig.open_street_map_default_longitude + end end end From 1199a356c4fe734b924efa06db9d14d1ad25d376 Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Fri, 26 Jun 2020 22:26:39 +0100 Subject: [PATCH 2/4] Extract out a JS service for calculating where to centre the map when given a set of coordinates. Also removing the hardcoded default latitude/longitude from open_street_map directive because it's probably not very likely that it will be needed. --- .../directives/open_street_map.js.coffee | 38 ++-------- .../services/map_centre_calculator.js.coffee | 29 ++++++++ .../map_centre_calculator_spec.js.coffee | 70 +++++++++++++++++++ 3 files changed, 104 insertions(+), 33 deletions(-) create mode 100644 app/assets/javascripts/darkswarm/services/map_centre_calculator.js.coffee create mode 100644 spec/javascripts/unit/darkswarm/services/map_centre_calculator_spec.js.coffee diff --git a/app/assets/javascripts/darkswarm/directives/open_street_map.js.coffee b/app/assets/javascripts/darkswarm/directives/open_street_map.js.coffee index e9be4acb88..832878cd50 100644 --- a/app/assets/javascripts/darkswarm/directives/open_street_map.js.coffee +++ b/app/assets/javascripts/darkswarm/directives/open_street_map.js.coffee @@ -1,4 +1,4 @@ -Darkswarm.directive 'ofnOpenStreetMap', ($window, Enterprises, EnterpriseModal, availableCountries, openStreetMapConfig) -> +Darkswarm.directive 'ofnOpenStreetMap', ($window, MapCentreCalculator, Enterprises, EnterpriseModal, availableCountries, openStreetMapConfig) -> restrict: 'E' replace: true scope: true @@ -11,34 +11,6 @@ Darkswarm.directive 'ofnOpenStreetMap', ($window, Enterprises, EnterpriseModal, openStreetMapProviderName = openStreetMapConfig.open_street_map_provider_name openStreetMapProviderOptions = JSON.parse(openStreetMapConfig.open_street_map_provider_options) - average = (values) -> - total = values.reduce (sum, value) -> - sum = sum + value - , 0 - total / values.length - - averageAngle = (angleName) -> - positiveAngles = [] - negativeAngles = [] - for enterprise in Enterprises.enterprises - if enterprise.latitude? && enterprise.longitude? - if enterprise[angleName] > 0 - positiveAngles.push(enterprise[angleName]) - else - negativeAngles.push(enterprise[angleName]) - - averageNegativeAngle = average(negativeAngles) - averagePositiveAngle = average(positiveAngles) - - if negativeAngles.length == 0 - averagePositiveAngle - else if positiveAngles.length == 0 - averageNegativeAngle - else if averagePositiveAngle > averageNegativeAngle - averagePositiveAngle - averageNegativeAngle - else - averageNegativeAngle - averagePositiveAngle - buildMarker = (enterprise, latlng, title) -> icon = L.icon iconUrl: enterprise.icon @@ -90,15 +62,15 @@ Darkswarm.directive 'ofnOpenStreetMap', ($window, Enterprises, EnterpriseModal, initialLatitude = () -> if geocodedEnterprises().length > 0 - averageAngle("latitude") + MapCentreCalculator.calculate_latitude(geocodedEnterprises()) else - openStreetMapConfig.open_street_map_default_latitude || -37.4713077 + openStreetMapConfig.open_street_map_default_latitude initialLongitude = () -> if geocodedEnterprises().length > 0 - averageAngle("longitude") + MapCentreCalculator.calculate_longitude(geocodedEnterprises()) else - openStreetMapConfig.open_street_map_default_longitude || 144.7851531 + openStreetMapConfig.open_street_map_default_longitude overwriteInlinePositionRelativeToAbsoluteOnSearchField = -> $('#open-street-map--search').css("position", "absolute") diff --git a/app/assets/javascripts/darkswarm/services/map_centre_calculator.js.coffee b/app/assets/javascripts/darkswarm/services/map_centre_calculator.js.coffee new file mode 100644 index 0000000000..f1023ce2d6 --- /dev/null +++ b/app/assets/javascripts/darkswarm/services/map_centre_calculator.js.coffee @@ -0,0 +1,29 @@ +Darkswarm.factory 'MapCentreCalculator', -> + new class MapCentreCalculator + calculate_latitude: (coordinates) => + @_calculate("latitude", coordinates) + + calculate_longitude: (coordinates) => + @_calculate("longitude", coordinates) + + _calculate: (angleName, coordinates) => + positiveAngles = [] + negativeAngles = [] + angles = [] + + for coordinate in coordinates + angles.push(coordinate[angleName]) + if coordinate[angleName] > 0 + positiveAngles.push(coordinate[angleName]) + else + negativeAngles.push(coordinate[angleName]) + + minimumAngle = Math.min.apply(null, angles) + maximumAngle = Math.max.apply(null, angles) + + distanceBetweenMinimumAndMaximum = if maximumAngle > minimumAngle + maximumAngle - minimumAngle + else + minimumAngle - maximumAngle + + minimumAngle + (distanceBetweenMinimumAndMaximum / 2) diff --git a/spec/javascripts/unit/darkswarm/services/map_centre_calculator_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/map_centre_calculator_spec.js.coffee new file mode 100644 index 0000000000..00e747a0a8 --- /dev/null +++ b/spec/javascripts/unit/darkswarm/services/map_centre_calculator_spec.js.coffee @@ -0,0 +1,70 @@ +describe 'MapCentreCalculator service', -> + MapCentreCalculator = null + defaultLongitude = null + defaultLatitude = null + + beforeEach -> + module 'Darkswarm' + defaultLongitude = -6 + defaultLatitude = 53 + + inject (_MapCentreCalculator_)-> + MapCentreCalculator = _MapCentreCalculator_ + + describe "calculate_latitude", -> + it "calculates the center latitude", -> + coordinates = [ + { latitude: 53, longitude: defaultLongitude }, + { latitude: 54, longitude: defaultLongitude } + ] + + expect(MapCentreCalculator.calculate_latitude(coordinates)).toEqual 53.5 + + describe "calculate_longitude", -> + it "calculates the center longitude", -> + coordinates = [ + { latitude: defaultLatitude, longitude: -6 }, + { latitude: defaultLatitude, longitude: -7 } + ] + + expect(MapCentreCalculator.calculate_longitude(coordinates)).toEqual -6.5 + + describe "_calculate", -> + it "calculates the average angle correctly when given a single angle", -> + coordinates = [ + { latitude: defaultLatitude, longitude: -7 } + ] + + expect(MapCentreCalculator._calculate("longitude", coordinates)).toEqual -7 + + it "calculates the centre correctly when given a set of positive angles", -> + coordinates = [ + { latitude: 53, longitude: defaultLongitude }, + { latitude: 54, longitude: defaultLongitude } + ] + + expect(MapCentreCalculator._calculate("latitude", coordinates)).toEqual 53.5 + + it "calculates the centre correctly when given a set of negative angles", -> + coordinates = [ + { latitude: defaultLatitude, longitude: -6 }, + { latitude: defaultLatitude, longitude: -7 } + ] + + expect(MapCentreCalculator._calculate("longitude", coordinates)).toEqual -6.5 + + it "calculates the centre correctly when given a mixture of positive and negative angles and the centre is positive", -> + coordinates = [ + { latitude: defaultLatitude, longitude: 7 }, + { latitude: defaultLatitude, longitude: -4 } + ] + + expect(MapCentreCalculator._calculate("longitude", coordinates)).toEqual 1.5 + + it "calculates the centre correctly when given a mixture of positive and negative angles and the centre is negative", -> + coordinates = [ + { latitude: defaultLatitude, longitude: 4 }, + { latitude: defaultLatitude, longitude: -7 } + ] + + expect(MapCentreCalculator._calculate("longitude", coordinates)).toEqual -1.5 From cc317bc8c970266344490cbad8626d7a235ecef6 Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Fri, 24 Jul 2020 16:00:30 +0100 Subject: [PATCH 3/4] Move the :initialLatitude and :initialLongitude methods from the OpenStreetMap service to the MapCenterCalculator service. --- .../directives/open_street_map.js.coffee | 22 ++---------- .../darkswarm/services/enterprises.js.coffee | 5 +++ .../darkswarm/services/map.js.coffee | 4 +-- .../services/map_centre_calculator.js.coffee | 17 +++++++--- .../services/enterprise_spec.js.coffee | 8 +++-- .../map_centre_calculator_spec.js.coffee | 34 ++++++++++++++----- 6 files changed, 52 insertions(+), 38 deletions(-) diff --git a/app/assets/javascripts/darkswarm/directives/open_street_map.js.coffee b/app/assets/javascripts/darkswarm/directives/open_street_map.js.coffee index 832878cd50..32f0d921b0 100644 --- a/app/assets/javascripts/darkswarm/directives/open_street_map.js.coffee +++ b/app/assets/javascripts/darkswarm/directives/open_street_map.js.coffee @@ -36,10 +36,10 @@ Darkswarm.directive 'ofnOpenStreetMap', ($window, MapCentreCalculator, Enterpris zoomLevel = 6 map = L.map('open-street-map') L.tileLayer.provider(openStreetMapProviderName, openStreetMapProviderOptions).addTo(map) - map.setView([initialLatitude(), initialLongitude()], zoomLevel) + map.setView([MapCentreCalculator.initialLatitude(), MapCentreCalculator.initialLongitude()], zoomLevel) displayEnterprises = -> - for enterprise in geocodedEnterprises() + for enterprise in Enterprises.geocodedEnterprises() marker = buildMarker(enterprise, { lat: enterprise.latitude, lng: enterprise.longitude }, enterprise.name).addTo(map) enterpriseNames.push(enterpriseName(enterprise)) markers.push(marker) @@ -53,25 +53,9 @@ Darkswarm.directive 'ofnOpenStreetMap', ($window, MapCentreCalculator, Enterpris search: searchEnterprises ) overwriteInlinePositionRelativeToAbsoluteOnSearchField() - if geocodedEnterprises().length == 0 + if Enterprises.geocodedEnterprises().length == 0 disableSearchField() - geocodedEnterprises = () -> - Enterprises.enterprises.filter (enterprise) -> - enterprise.latitude? && enterprise.longitude? - - initialLatitude = () -> - if geocodedEnterprises().length > 0 - MapCentreCalculator.calculate_latitude(geocodedEnterprises()) - else - openStreetMapConfig.open_street_map_default_latitude - - initialLongitude = () -> - if geocodedEnterprises().length > 0 - MapCentreCalculator.calculate_longitude(geocodedEnterprises()) - else - openStreetMapConfig.open_street_map_default_longitude - overwriteInlinePositionRelativeToAbsoluteOnSearchField = -> $('#open-street-map--search').css("position", "absolute") diff --git a/app/assets/javascripts/darkswarm/services/enterprises.js.coffee b/app/assets/javascripts/darkswarm/services/enterprises.js.coffee index 843749c2e6..d8347c7e41 100644 --- a/app/assets/javascripts/darkswarm/services/enterprises.js.coffee +++ b/app/assets/javascripts/darkswarm/services/enterprises.js.coffee @@ -84,3 +84,8 @@ Darkswarm.factory 'Enterprises', (enterprises, ShopsResource, CurrentHub, Taxons resetDistance: -> enterprise.distance = null for enterprise in @enterprises + + geocodedEnterprises: => + @enterprises.filter (enterprise) -> + enterprise.latitude? && enterprise.longitude? + diff --git a/app/assets/javascripts/darkswarm/services/map.js.coffee b/app/assets/javascripts/darkswarm/services/map.js.coffee index 2758f299f8..9aa23a7155 100644 --- a/app/assets/javascripts/darkswarm/services/map.js.coffee +++ b/app/assets/javascripts/darkswarm/services/map.js.coffee @@ -2,9 +2,7 @@ Darkswarm.factory "OfnMap", (Enterprises, EnterpriseListModal, MapConfiguration) new class OfnMap constructor: -> @coordinates = {} - @enterprises = Enterprises.enterprises.filter (enterprise) -> - # Remove enterprises w/o lat or long - enterprise.latitude != null || enterprise.longitude != null + @enterprises = Enterprises.geocodedEnterprises() @enterprises = @enterprise_markers(@enterprises) enterprise_markers: (enterprises) -> diff --git a/app/assets/javascripts/darkswarm/services/map_centre_calculator.js.coffee b/app/assets/javascripts/darkswarm/services/map_centre_calculator.js.coffee index f1023ce2d6..85ca34ff54 100644 --- a/app/assets/javascripts/darkswarm/services/map_centre_calculator.js.coffee +++ b/app/assets/javascripts/darkswarm/services/map_centre_calculator.js.coffee @@ -1,10 +1,17 @@ -Darkswarm.factory 'MapCentreCalculator', -> +Darkswarm.factory 'MapCentreCalculator', (Enterprises, openStreetMapConfig) -> new class MapCentreCalculator - calculate_latitude: (coordinates) => - @_calculate("latitude", coordinates) - calculate_longitude: (coordinates) => - @_calculate("longitude", coordinates) + initialLatitude: => + if Enterprises.geocodedEnterprises().length > 0 + @_calculate("latitude", Enterprises.geocodedEnterprises()) + else + openStreetMapConfig.open_street_map_default_latitude + + initialLongitude: => + if Enterprises.geocodedEnterprises().length > 0 + @_calculate("longitude", Enterprises.geocodedEnterprises()) + else + openStreetMapConfig.open_street_map_default_longitude _calculate: (angleName, coordinates) => positiveAngles = [] diff --git a/spec/javascripts/unit/darkswarm/services/enterprise_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/enterprise_spec.js.coffee index c95fbdf81b..088ea359e0 100644 --- a/spec/javascripts/unit/darkswarm/services/enterprise_spec.js.coffee +++ b/spec/javascripts/unit/darkswarm/services/enterprise_spec.js.coffee @@ -24,7 +24,7 @@ describe "Enterprises service", -> {id: 5, visible: true, name: 'e', category: "producer_hub", hubs: [{id: 1}]}, {id: 6, visible: true, name: 'f', category: "producer_shop", hubs: [{id: 2}]}, {id: 7, visible: true, name: 'g', category: "producer", hubs: [{id: 2}]} - {id: 8, visible: true, name: 'h', category: "producer", hubs: [{id: 2}]} + {id: 8, visible: true, name: 'h', category: "producer", hubs: [{id: 2}], latitude: 76.26, longitude: -42.66 } ] H1: 0 beforeEach -> @@ -142,4 +142,8 @@ describe "Enterprises service", -> it "resets the distance", -> Enterprises.resetDistance() for e in Enterprises.enterprises - expect(e.distance).toBeNull() \ No newline at end of file + expect(e.distance).toBeNull() + + describe "geocodedEnterprises", -> + it "only returns enterprises which have a latitude and longitude", -> + expect(Enterprises.geocodedEnterprises()).toEqual [Enterprises.enterprises[7]] diff --git a/spec/javascripts/unit/darkswarm/services/map_centre_calculator_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/map_centre_calculator_spec.js.coffee index 00e747a0a8..0d18541442 100644 --- a/spec/javascripts/unit/darkswarm/services/map_centre_calculator_spec.js.coffee +++ b/spec/javascripts/unit/darkswarm/services/map_centre_calculator_spec.js.coffee @@ -1,5 +1,6 @@ describe 'MapCentreCalculator service', -> MapCentreCalculator = null + Enterprises = null defaultLongitude = null defaultLatitude = null @@ -7,27 +8,42 @@ describe 'MapCentreCalculator service', -> module 'Darkswarm' defaultLongitude = -6 defaultLatitude = 53 + angular.module('Darkswarm').value 'openStreetMapConfig', { + open_street_map_default_latitude: 76.26, + open_street_map_default_longitude: -42.66 + } - inject (_MapCentreCalculator_)-> + inject (_MapCentreCalculator_, _Enterprises_)-> MapCentreCalculator = _MapCentreCalculator_ + Enterprises = _Enterprises_ - describe "calculate_latitude", -> - it "calculates the center latitude", -> - coordinates = [ + describe "initialLatitude", -> + it "calculates the center latitude of any present geocoded enterprises", -> + Enterprises.geocodedEnterprises = -> [ { latitude: 53, longitude: defaultLongitude }, { latitude: 54, longitude: defaultLongitude } ] - expect(MapCentreCalculator.calculate_latitude(coordinates)).toEqual 53.5 + expect(MapCentreCalculator.initialLatitude()).toEqual 53.5 - describe "calculate_longitude", -> - it "calculates the center longitude", -> - coordinates = [ + it "returns the default configured latitude when there are no geocoded enterprises present", -> + Enterprises.geocodedEnterprises = -> [] + + expect(MapCentreCalculator.initialLatitude()).toEqual 76.26 + + describe "initialLongitude", -> + it "calculates the center longitude of any present geocoded enterprises", -> + Enterprises.geocodedEnterprises = -> [ { latitude: defaultLatitude, longitude: -6 }, { latitude: defaultLatitude, longitude: -7 } ] - expect(MapCentreCalculator.calculate_longitude(coordinates)).toEqual -6.5 + expect(MapCentreCalculator.initialLongitude()).toEqual -6.5 + + it "returns the default configured longitude when there are no geocoded enterprises present", -> + Enterprises.geocodedEnterprises = -> [] + + expect(MapCentreCalculator.initialLongitude()).toEqual -42.66 describe "_calculate", -> it "calculates the average angle correctly when given a single angle", -> From e6ab2ae7539568cdbf78e937be126c967cee795d Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Mon, 3 Aug 2020 15:12:40 +0100 Subject: [PATCH 4/4] Remove unused positiveAngles and negativeAngles arrays from map centre calculator service. I forgot to remove these when I was refactoring this earlier. --- .../darkswarm/services/map_centre_calculator.js.coffee | 6 ------ 1 file changed, 6 deletions(-) diff --git a/app/assets/javascripts/darkswarm/services/map_centre_calculator.js.coffee b/app/assets/javascripts/darkswarm/services/map_centre_calculator.js.coffee index 85ca34ff54..978afb5858 100644 --- a/app/assets/javascripts/darkswarm/services/map_centre_calculator.js.coffee +++ b/app/assets/javascripts/darkswarm/services/map_centre_calculator.js.coffee @@ -14,16 +14,10 @@ Darkswarm.factory 'MapCentreCalculator', (Enterprises, openStreetMapConfig) -> openStreetMapConfig.open_street_map_default_longitude _calculate: (angleName, coordinates) => - positiveAngles = [] - negativeAngles = [] angles = [] for coordinate in coordinates angles.push(coordinate[angleName]) - if coordinate[angleName] > 0 - positiveAngles.push(coordinate[angleName]) - else - negativeAngles.push(coordinate[angleName]) minimumAngle = Math.min.apply(null, angles) maximumAngle = Math.max.apply(null, angles)