Merge pull request #6680 from NHSDigital/fetch-location-lat-lng

Fetch latitude and longitude for locations

Author: thomasleese

app/jobs/enqueue_location_position_updater_job.rb

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+class EnqueueLocationPositionUpdaterJob < ApplicationJob
+  queue_as :third_party_data_imports
+
+  def perform
+    ids = Location.where(position: nil).has_address.ids
+    LocationPositionUpdaterJob.perform_bulk(ids.zip)
+  end
+end

app/jobs/location_position_updater_job.rb

@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+class LocationPositionUpdaterJob
+  include Sidekiq::Job
+
+  sidekiq_options queue: :third_party_data_imports, lock: :until_executing
+
+  def perform(location_id)
+    location = Location.find(location_id)
+    LocationPositionUpdater.call(location)
+  rescue LocationPositionUpdater::NoResults => e
+    Sentry.capture_exception(e, level: "warning")
+  end
+end

app/lib/location_position_updater.rb

@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+##
+# This class fetches the latitude and longitude of a location's address
+# using the +OrdnanceSurvey::PlacesAPI+ and stores it in the position column.
+class LocationPositionUpdater
+  class MissingAddress < StandardError
+  end
+
+  class NoResults < StandardError
+  end
+
+  def initialize(location)
+    @location = location
+  end
+
+  attr_reader :location
+
+  def call
+    raise MissingAddress unless location.has_address?
+
+    location.update!(position:)
+  end
+
+  def self.call(...) = new(...).call
+
+  private_class_method :new
+
+  private
+
+  def full_address = location.address_parts.join(", ")
+
+  def position
+    response = OrdnanceSurvey::PlacesAPI.find(full_address)
+
+    results = response[:results]
+    raise NoResults if results.blank?
+
+    first_result = results.first
+
+    latitude = first_result.dig(:dpa, :lat)
+    longitude = first_result.dig(:dpa, :lng)
+
+    raise NoResults if latitude.blank? || longitude.blank?
+
+    "POINT(#{longitude} #{latitude})"
+  end
+end

app/lib/mavis_cli/clinics/create.rb

@@ -26,14 +26,17 @@ def call(
       )
         MavisCLI.load_rails
 
-        Location.create!(
-          type: :community_clinic,
-          name:,
-          address_line_1:,
-          address_town:,
-          address_postcode:,
-          ods_code:
-        )
+        location =
+          Location.create!(
+            type: :community_clinic,
+            name:,
+            address_line_1:,
+            address_town:,
+            address_postcode:,
+            ods_code:
+          )
+
+        LocationPositionUpdaterJob.perform_async(location.id)
       end
     end
   end

app/models/concerns/address_concern.rb

@@ -18,6 +18,16 @@ module AddressConcern
     has_one :local_authority_from_postcode,
             through: :local_authority_postcode,
             source: :local_authority
+
+    scope :has_address,
+          -> do
+            where("address_line_1 IS NOT NULL AND address_line_1 <> ''")
+              .or(where("address_line_2 IS NOT NULL AND address_line_2 <> ''"))
+              .or(where("address_town IS NOT NULL AND address_town <> ''"))
+              .or(
+                where("address_postcode IS NOT NULL AND address_postcode <> ''")
+              )
+          end
   end
 
   def address_parts

app/models/onboarding.rb

@@ -249,6 +249,10 @@ def save!(include_previous_academic_year: false)
       end
 
       PatientTeamUpdater.call(team_scope: Team.where(id: team.id))
+
+      location_ids =
+        schools.map(&:id) + clinics.keys.filter(&:has_address?).map(&:id)
+      LocationPositionUpdaterJob.perform_bulk(location_ids.zip)
     end
   end
 
@@ -311,7 +315,7 @@ def location
       @location ||= Location.gias_school.find_by_urn_and_site(urn)
     end
 
-    delegate :status, to: :location, allow_nil: true
+    delegate :id, :status, to: :location, allow_nil: true
     delegate :team, to: :subteam
 
     def save!
@@ -347,6 +351,8 @@ class NewSchoolSite
     validates :name, presence: true
     validates :site, presence: true
 
+    delegate :id, to: :location
+
     def original_location
       @original_location ||= Location.gias_school.find_by_urn_and_site(urn)
     end

config/sidekiq.yml

@@ -39,6 +39,10 @@
       description: Send vaccination confirmation emails to parents
 
     # Nightly
+    EnqueueLocationPositionUpdaterJob:
+      cron: "0 0 * * *"
+      description:
+        Enqueue jobs to fetch latitude and longitude for locations with addresses but no position
     RemoveImportCSVJob:
       cron: "0 1 * * *"
       description: Remove CSV data from old cohort and immunisation imports

spec/factories/locations.rb

@@ -58,6 +58,14 @@
       end
     end
 
+    trait :without_address do
+      address_line_1 { nil }
+      address_line_2 { nil }
+      address_town { nil }
+      address_postcode { nil }
+      position { nil }
+    end
+
     factory :community_clinic do
       type { :community_clinic }
       name { "#{Faker::University.name} Clinic" }

spec/features/cli_clinics_create_spec.rb

@@ -7,6 +7,7 @@
     it "runs successfully" do
       when_i_run_the_command
       then_the_clinic_is_created
+      and_a_location_position_updater_job_is_enqueued
     end
   end
 
@@ -29,4 +30,10 @@ def then_the_clinic_is_created
     expect(clinic.address_town).to eq("Town")
     expect(clinic.address_postcode).to eq("SW1A 1AA")
   end
+
+  def and_a_location_position_updater_job_is_enqueued
+    expect(LocationPositionUpdaterJob).to have_enqueued_sidekiq_job(
+      Location.last.id
+    )
+  end
 end

spec/jobs/enqueue_location_position_updater_job_spec.rb

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+describe EnqueueLocationPositionUpdaterJob do
+  describe "#perform" do
+    subject(:perform_now) { described_class.perform_now }
+
+    let!(:location_with_address_no_position) do
+      create(:community_clinic, position: nil)
+    end
+
+    let!(:location_with_position) { create(:community_clinic) }
+
+    let!(:location_without_address) do
+      create(:community_clinic, :without_address)
+    end
+
+    it "enqueues jobs for locations with address but no position" do
+      expect { perform_now }.to enqueue_sidekiq_job(
+        LocationPositionUpdaterJob
+      ).with(location_with_address_no_position.id)
+    end
+
+    it "does not enqueue jobs for locations with position" do
+      expect { perform_now }.not_to enqueue_sidekiq_job(
+        LocationPositionUpdaterJob
+      ).with(location_with_position.id)
+    end
+
+    it "does not enqueue jobs for locations without address" do
+      expect { perform_now }.not_to enqueue_sidekiq_job(
+        LocationPositionUpdaterJob
+      ).with(location_without_address.id)
+    end
+  end
+end