Allow people to set enterprise latitude/longitude manually or automatically.

This for new changes to the enterprise registration/signup flow where a map will be displayed when people are filling in their address. On this map people can check the geocoder has geocoded their address correctly and if not they can manually adjust their latitude/longitude on the map.

But currently every time someone changes their address in the Admin > Enterprise > Address section the address would automatically be geocoded so this could overwrite the latitude/longitude that was set during sign up. To prevent the latitude/longitude from being overwritten this add's a checkbox which people need to explicity click if they want their address to be automatically geocoded, otherwise it will just use the manually configured latitude/longitude.

Note this new feature which allows people to select their location on a map during registration only works with Google maps so far. So if an instance is using Open Street Map this change also adds support for passing a :use_geocoder parameter to the Api::EnterprisesController during registration so that the address will be geocoded on the backend without the use of a map.

app/assets/javascripts/darkswarm/all.js.coffee

@@ -59,4 +59,4 @@
 $ ->
   # Hacky fix for issue - http://foundation.zurb.com/forum/posts/2112-foundation-5100-syntax-error-in-js
   Foundation.set_namespace ""
-  $(document).foundation()
+  $(document).foundation()

app/assets/javascripts/darkswarm/controllers/registration/registration_controller.js.coffee

@@ -6,7 +6,6 @@ Darkswarm.controller "RegistrationCtrl", ($scope, RegistrationService, Enterpris
   $scope.latLong = null
   $scope.addressConfirmed
   $scope.steps = ['details', 'contact', 'type', 'about', 'images', 'social']
-  $scope.enableMapConfirm = GmapsGeo and GmapsGeo.OK
 
   # Filter countries without states since the form requires a state to be selected.
   # Consider changing the form to require a state only if a country requires them (Spree option).

app/assets/javascripts/darkswarm/services/enterprise_registration_service.js.coffee

@@ -21,6 +21,7 @@ Darkswarm.factory "EnterpriseRegistrationService", ($http, RegistrationService,
         url: "/api/enterprises"
         data:
           enterprise: @prepare()
+          use_geocoder: @useGeocoder()
         params:
           token: spreeApiKey
       ).success((data) =>
@@ -45,6 +46,7 @@ Darkswarm.factory "EnterpriseRegistrationService", ($http, RegistrationService,
         url: "/api/enterprises/#{@enterprise.id}"
         data:
           enterprise: @prepare()
+          use_geocoder: @useGeocoder()
         params:
           token: spreeApiKey
       ).success((data) ->
@@ -63,3 +65,7 @@ Darkswarm.factory "EnterpriseRegistrationService", ($http, RegistrationService,
       enterprise.address_attributes = @enterprise.address if @enterprise.address?
       enterprise.address_attributes.country_id = @enterprise.country.id if @enterprise.country?
       enterprise
+
+    useGeocoder: =>
+      if @enterprise.address? && !@enterprise.address.latitude? && !@enterprise.address.longitude?
+        return "1"

app/controllers/admin/enterprises_controller.rb

@@ -4,6 +4,8 @@ require 'open_food_network/order_cycle_permissions'
 
 module Admin
   class EnterprisesController < Admin::ResourceController
+    include GeocodeEnterpriseAddress
+
     # These need to run before #load_resource so that @object is initialised with sanitised values
     prepend_before_action :override_owner, only: :create
     prepend_before_action :override_sells, only: :create
@@ -22,6 +24,9 @@ module Admin
     before_action :load_properties, only: [:edit, :update]
     before_action :setup_property, only: [:edit]
 
+    update.after  :geocode_address_if_use_geocoder
+    create.after  :geocode_address_if_use_geocoder
+
     helper 'spree/products'
     include OrderCyclesHelper
 

app/controllers/api/enterprises_controller.rb

@@ -1,5 +1,7 @@
 module Api
   class EnterprisesController < Api::BaseController
+    include GeocodeEnterpriseAddress
+
     before_action :override_owner, only: [:create, :update]
     before_action :check_type, only: :update
     before_action :override_sells, only: [:create, :update]
@@ -14,6 +16,7 @@ module Api
       user_ids = enterprise_params.delete(:user_ids)
       @enterprise = Enterprise.new(enterprise_params)
       if @enterprise.save
+        geocode_address_if_use_geocoder
         @enterprise.user_ids = user_ids
         render json: @enterprise.id, status: :created
       else
@@ -26,6 +29,7 @@ module Api
       authorize! :update, @enterprise
 
       if @enterprise.update(enterprise_params)
+        geocode_address_if_use_geocoder
         render json: @enterprise.id, status: :ok
       else
         invalid_resource!(@enterprise)

app/controllers/concerns/geocode_enterprise_address.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+module GeocodeEnterpriseAddress
+  extend ActiveSupport::Concern
+
+  def geocode_address_if_use_geocoder
+    AddressGeocoder.new(@enterprise.address).geocode if params[:use_geocoder] == "1"
+  end
+end

app/helpers/map_helper.rb

@@ -1,2 +1,13 @@
 module MapHelper
+  def using_google_maps?
+    ENV["GOOGLE_MAPS_API_KEY"].present? || google_maps_configured_with_geocoder_api_key?
+  end
+
+  private
+
+  def google_maps_configured_with_geocoder_api_key?
+    ENV["GEOCODER_API_KEY"].present? && (
+      ENV["GEOCODER_SERVICE"].to_s == "google" || ENV["GEOCODER_SERVICE"].blank?
+    )
+  end
 end

app/models/enterprise.rb

@@ -410,10 +410,6 @@ class Enterprise < ActiveRecord::Base
     address.firstname = address.lastname = address.phone = 'unused' if address.present?
   end
 
-  def geocode_address
-    address.geocode if address.andand.changed?
-  end
-
   def ensure_owner_is_manager
     users << owner unless users.include?(owner)
   end

app/models/spree/address.rb

@@ -10,24 +10,18 @@ module Spree
     has_one :enterprise, dependent: :restrict_with_exception
     has_many :shipments
 
-    before_validation :geocode, if: :use_geocoder?
-
     validates :firstname, :lastname, :address1, :city, :country, presence: true
     validates :zipcode, presence: true, if: :require_zipcode?
     validates :phone, presence: true, if: :require_phone?
 
     validate :state_validate
 
-    attr_accessor :use_geocoder
-
     after_save :touch_enterprise
 
     alias_attribute :first_name, :firstname
     alias_attribute :last_name, :lastname
     delegate :name, to: :state, prefix: true, allow_nil: true
 
-    geocoded_by :geocode_address
-
     def self.default
       country = begin
                   Spree::Country.find(Spree::Config[:default_country_id])
@@ -93,10 +87,6 @@ module Spree
       }
     end
 
-    def geocode_address
-      render_address([address1, address2, zipcode, city, country.andand.name, state.andand.name])
-    end
-
     def full_address
       render_address([address1, address2, city, zipcode, state.andand.name])
     end
@@ -111,10 +101,6 @@ module Spree
 
     private
 
-    def use_geocoder?
-      @use_geocoder == true || @use_geocoder == 'true' || @use_geocoder == '1'
-    end
-
     def require_phone?
       true
     end

app/services/address_geocoder.rb

@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+class AddressGeocoder
+  def initialize(address)
+    @address = address
+  end
+
+  def geocode
+    latitude, longitude = Geocoder.coordinates(geocode_address)
+
+    return unless latitude.present? && longitude.present?
+
+    address.update(latitude: latitude, longitude: longitude)
+  end
+
+  private
+
+  attr_reader :address
+
+  def geocode_address
+    [
+      address.address1,
+      address.address2,
+      address.zipcode,
+      address.city,
+      address.country.andand.name,
+      address.state.andand.name
+    ].select(&:present?).join(', ')
+  end
+end

app/services/permitted_attributes/address.rb

@@ -7,7 +7,7 @@ module PermittedAttributes
         :firstname, :lastname, :address1, :address2,
         :city, :country_id, :state_id, :zipcode,
         :phone, :state_name, :alternative_phone, :company,
-        :latitude, :longitude, :use_geocoder
+        :latitude, :longitude
       ]
     end
   end

app/views/admin/enterprises/_new_form.html.haml

@@ -113,12 +113,11 @@
     .three.columns.alpha
       = " ".html_safe
     .five.columns.omega
-      = af.check_box :use_geocoder
-      = af.label :use_geocoder, t('use_geocoder')
+      = check_box_tag "use_geocoder"
+      = label_tag "use_geocoder", t('use_geocoder')
 .row
   .twelve.columns.alpha
      
 .row
   .twelve.columns.alpha
-    = render partial: "spree/admin/shared/new_resource_links"
-    
+    = render partial: "spree/admin/shared/new_resource_links"

app/views/admin/enterprises/form/_address.html.haml

@@ -51,5 +51,5 @@
   .three.columns.alpha
     = " ".html_safe
   .five.columns.omega
-    = af.check_box :use_geocoder
-    = af.label :use_geocoder, t('use_geocoder')
+    = check_box_tag "use_geocoder"
+    = label_tag "use_geocoder", t('use_geocoder')

app/views/layouts/registration.html.haml

@@ -35,5 +35,4 @@
     = yield :injection_data
 
     = render "layouts/i18n_script"
-    = render "layouts/bugherd_script"
-    
+    = render "layouts/bugherd_script"

app/views/registration/steps/_details.html.haml

@@ -56,25 +56,26 @@
                 %select.chunky{ id: 'enterprise_country', name: 'country', required: true, ng: { init: "setDefaultCountry(#{Spree::Config[:default_country_id]})", model: 'enterprise.country', options: 'c as c.name for c in countries' } }
                 %span.error{ ng: { show: "details.country.$error.required && submitted" } }
                   = t(".country_field_error")
-      %div{ ng: {if: "enableMapConfirm"}}
-        .center{ ng: {if: "latLong"}} 
-          %strong {{'registration.steps.details.drag_pin' | t}}
-        .small-12.medium-9.large-12.columns.end
-          .map-container--registration{id: "registration-map", ng: {if: "!!map"}}
-            %ui-gmap-google-map{ center: "map.center", zoom: "map.zoom"}
-              %ui-gmap-marker{idKey: 1, coords: "latLong", options: '{ draggable: true}'}
-            
-        .center
-          %input.button.primary{ type: "button", value: "{{'registration.steps.details.locate_address' | t}}", ng: { click: "locateAddress()" } }
-        .center{ ng: {if: "latLong"}} 
-          .field
-            %input{ type: 'checkbox', id: 'confirm_address', name: 'confirm_address', ng: { model: 'addressConfirmed', change: 'confirmAddressChange(confirmAddress)'} }
-            %label{ for: 'confirm_address' } {{'registration.steps.details.confirm_address' | t}}
-        .small-12.medium-9.large-12.columns.end{ ng: {if: "latLong"}} 
-          .field
-            %strong {{'registration.steps.details.drag_map_marker' | t}}
+      - if using_google_maps?
+        %div
+          .center{ ng: {if: "latLong"}}
+            %strong {{'registration.steps.details.drag_pin' | t}}
+          .small-12.medium-9.large-12.columns.end
+            .map-container--registration{id: "registration-map", ng: {if: "!!map"}}
+              %ui-gmap-google-map{ center: "map.center", zoom: "map.zoom"}
+                %ui-gmap-marker{idKey: 1, coords: "latLong", options: '{ draggable: true}'}
+
+          .center
+            %input.button.primary{ type: "button", value: "{{'registration.steps.details.locate_address' | t}}", ng: { click: "locateAddress()" } }
+          .center{ ng: {if: "latLong"}}
+            .field
+              %input{ type: 'checkbox', id: 'confirm_address', name: 'confirm_address', ng: { model: 'addressConfirmed', change: 'confirmAddressChange(confirmAddress)'} }
+              %label{ for: 'confirm_address' } {{'registration.steps.details.confirm_address' | t}}
+          .small-12.medium-9.large-12.columns.end{ ng: {if: "latLong"}}
+            .field
+              %strong {{'registration.steps.details.drag_map_marker' | t}}
+
       .row.buttons
         .small-12.columns
           %hr
           %input.button.primary.right{ type: "submit", value: "{{'continue' | t}}" }
-          

spec/controllers/admin/enterprises_controller_spec.rb

@@ -13,13 +13,14 @@ describe Admin::EnterprisesController, type: :controller do
 
   let(:distributor) { create(:distributor_enterprise, owner: distributor_owner ) }
   let(:supplier) { create(:supplier_enterprise, owner: supplier_owner) }
+  let(:country) { Spree::Country.find_by name: 'Australia' }
+  let(:state) { Spree::State.find_by name: 'Victoria' }
+  let(:address_params) { { address1: 'a', city: 'a', zipcode: 'a', country_id: country.id, state_id: state.id } }
 
   before { @request.env['HTTP_REFERER'] = 'http://test.com/' }
 
   describe "creating an enterprise" do
-    let(:country) { Spree::Country.find_by name: 'Australia' }
-    let(:state) { Spree::State.find_by name: 'Victoria' }
-    let(:enterprise_params) { { enterprise: { name: 'zzz', permalink: 'zzz', is_primary_producer: '0', address_attributes: { address1: 'a', city: 'a', zipcode: 'a', country_id: country.id, state_id: state.id } } } }
+    let(:enterprise_params) { { enterprise: { name: 'zzz', permalink: 'zzz', is_primary_producer: '0', address_attributes: address_params } } }
 
     it "grants management permission if the current user is an enterprise user" do
       allow(controller).to receive_messages spree_current_user: distributor_manager
@@ -99,6 +100,27 @@ describe Admin::EnterprisesController, type: :controller do
         expect(enterprise.sells).to eq('any')
       end
     end
+
+    context "geocoding" do
+      before do
+        allow(controller).to receive_messages spree_current_user: admin_user
+        enterprise_params[:enterprise][:owner_id] = admin_user
+      end
+
+      it "geocodes the address when the :use_geocoder parameter is set" do
+        expect_any_instance_of(AddressGeocoder).to receive(:geocode)
+        enterprise_params[:use_geocoder] = "1"
+
+        spree_put :create, enterprise_params
+      end
+
+      it "doesn't geocode the address when the :use_geocoder parameter is not set" do
+        expect_any_instance_of(AddressGeocoder).not_to receive(:geocode)
+        enterprise_params[:use_geocoder] = "0"
+
+        spree_put :create, enterprise_params
+      end
+    end
   end
 
   describe "updating an enterprise" do
@@ -291,6 +313,26 @@ describe Admin::EnterprisesController, type: :controller do
         expect(distributor.users).to include user
       end
     end
+
+    context "geocoding" do
+      before do
+        allow(controller).to receive_messages spree_current_user: profile_enterprise.owner
+      end
+
+      it "geocodes the address when the :use_geocoder parameter is set" do
+        expect_any_instance_of(AddressGeocoder).to receive(:geocode)
+        enterprise_params = { id: profile_enterprise, enterprise: {}, use_geocoder: "1" }
+
+        spree_put :update, enterprise_params
+      end
+
+      it "doesn't geocode the address when the :use_geocoder parameter is not set" do
+        expect_any_instance_of(AddressGeocoder).not_to receive(:geocode)
+        enterprise_params = { id: profile_enterprise, enterprise: {}, use_geocoder: "0" }
+
+        spree_put :update, enterprise_params
+      end
+    end
   end
 
   describe "register" do

spec/controllers/api/enterprises_controller_spec.rb

@@ -49,6 +49,20 @@ describe Api::EnterprisesController, type: :controller do
         enterprise = Enterprise.last
         expect(enterprise.user_ids).to match_array([enterprise_owner.id, manager1.id, manager2.id])
       end
+
+      context "geocoding" do
+        it "geocodes the address when the :use_geocoder parameter is set" do
+          expect_any_instance_of(AddressGeocoder).to receive(:geocode)
+
+          api_post :create, { enterprise: new_enterprise_params, use_geocoder: "1" }
+        end
+
+        it "doesn't geocode the address when the :use_geocoder parameter is not set" do
+          expect_any_instance_of(AddressGeocoder).not_to receive(:geocode)
+
+          api_post :create, { enterprise: new_enterprise_params, use_geocoder: "0" }
+        end
+      end
     end
   end
 

spec/features/consumer/registration_spec.rb

@@ -15,6 +15,7 @@ feature "Registration", js: true do
       albania = Spree::Country.create!({ name: "Albania", iso3: "ALB", iso: "AL", iso_name: "ALBANIA", numcode: "8" })
       Spree::State.create!({ name: "Berat", abbr: "BRA", country: albania })
       Spree::Country.create!({ name: "Chad", iso3: "TCD", iso: "TD", iso_name: "CHAD", numcode: "148" })
+      AddressGeocoder.any_instance.stub(:geocode)
     end
 
     after do

spec/models/spree/addresses_spec.rb

@@ -42,24 +42,6 @@ describe Spree::Address do
     end
   end
 
-  describe "geocode address" do
-    it "should include address1, address2, zipcode, city, state and country" do
-      expect(address.geocode_address).to include(address.address1)
-      expect(address.geocode_address).to include(address.address2)
-      expect(address.geocode_address).to include(address.zipcode)
-      expect(address.geocode_address).to include(address.city)
-      expect(address.geocode_address).to include(address.state.name)
-      expect(address.geocode_address).to include(address.country.name)
-    end
-
-    it "should not include empty fields" do
-      address.address2 = nil
-      address.city = ""
-
-      expect(address.geocode_address.split(',').length).to eql(4)
-    end
-  end
-
   describe "full address" do
     let(:address) { FactoryBot.build(:address) }
 

spec/services/address_geocoder_spec.rb

@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe AddressGeocoder do
+  let(:australia) { Spree::Country.find_or_create_by!(name: "Australia") }
+  let(:victoria) { Spree::State.find_or_create_by(name: "Victoria", country: australia) }
+  let(:address) do
+    create(:address,
+      address1: "12 Galvin Street",
+      address2: "Unit 1",
+      city: "Altona",
+      country: australia,
+      state: victoria,
+      zipcode: 3018,
+      latitude: nil,
+      longitude: nil)
+  end
+
+  it "formats the address into a single comma separated string when passing it to the geocoder" do
+    expect(Geocoder).to receive(:coordinates).with("12 Galvin Street, Unit 1, 3018, Altona, Australia, Victoria")
+
+    AddressGeocoder.new(address).geocode
+  end
+
+  describe "when the geocoder can determine the latitude and longitude" do
+    it "updates the address's latitude and longitude" do
+      allow(Geocoder).to receive(:coordinates).and_return([-37.47, 144.78])
+
+      AddressGeocoder.new(address).geocode
+
+      expect(address.latitude).to eq(-37.47)
+      expect(address.longitude).to eq(144.78)
+    end
+  end
+
+  describe "when the geocoder cannot determine the latitude and longitude" do
+    it "doesn't update the address's latitude and longitude" do
+      allow(Geocoder).to receive(:coordinates).and_return([nil, nil])
+
+      AddressGeocoder.new(address).geocode
+
+      expect(address.latitude).to be_nil
+      expect(address.longitude).to be_nil
+    end
+  end
+end