Merge pull request #6716 from NHSDigital/location-no-results-production

Only report `LocationPositionUpdater::NoResults` in production

Author: thomasleese

app/jobs/location_position_updater_job.rb

@@ -9,6 +9,13 @@ def perform(location_id)
     location = Location.find(location_id)
     LocationPositionUpdater.call(location)
   rescue LocationPositionUpdater::NoResults => e
-    Sentry.capture_exception(e, level: "warning")
+    if Settings.location_position_updater_job.capture_exception
+      Sentry.capture_exception(e, level: "warning")
+    else
+      Rails.logger.warn(
+        "Could not fetch position for: #{location.name} (#{location.id})"
+      )
+      Rails.logger.warn(e.backtrace.join("\n"))
+    end
   end
 end

app/lib/location_position_updater.rb

@@ -28,10 +28,10 @@ def self.call(...) = new(...).call
 
   private
 
-  def full_address = location.address_parts.join(", ")
+  def query = [location.name, location.address_parts].join(", ")
 
   def position
-    response = OrdnanceSurvey::PlacesAPI.find(full_address)
+    response = OrdnanceSurvey::PlacesAPI.find(query)
 
     results = response[:results]
     raise NoResults if results.blank?

app/lib/ordnance_survey/places_api.rb

@@ -9,8 +9,8 @@ def initialize
       @base_url = "https://api.os.uk"
     end
 
-    def find(query)
-      params = { query:, format: "json" }
+    def find(query, max_results: 1, output_srs: "EPSG:4326")
+      params = { query:, maxresults: max_results, output_srs:, format: "json" }
       response = connection.get("/search/places/v1/find", params)
       response.body.deep_transform_keys(&:downcase).deep_symbolize_keys
     end

config/settings.yml

@@ -64,6 +64,9 @@ ordnance_survey:
   api_key: <%= Rails.application.credentials.ordnance_survey&.api_key %>
   secret_key: <%= Rails.application.credentials.ordnance_survey&.secret_key %>
 
+location_position_updater_job:
+  capture_exception: true
+
 # Set a value to override the default values set in config/initializers/devise.rb
 devise:
   timeout_in_seconds:

config/settings/development.yml

@@ -29,3 +29,6 @@ reporting_api:
 
 careplus:
   base_url: http://localhost:8080
+
+location_position_updater_job:
+  capture_exception: false

config/settings/end_to_end.yml

@@ -26,3 +26,6 @@ splunk:
 reporting_api:
   client_app:
     root_url: http://localhost:4101
+
+location_position_updater_job:
+  capture_exception: false

config/settings/staging.yml

@@ -20,3 +20,6 @@ pds:
 
 careplus:
   base_url: <%= ENV.fetch("MOCK_CAREPLUS_URL", nil) %>
+
+location_position_updater_job:
+  capture_exception: false

config/settings/test.yml

@@ -27,3 +27,6 @@ reporting_api:
 
 careplus:
   base_url: http://localhost:8080
+
+location_position_updater_job:
+  capture_exception: false

spec/jobs/location_position_updater_job_spec.rb

@@ -39,16 +39,35 @@
         allow(LocationPositionUpdater).to receive(:call).and_raise(error)
       end
 
-      it "captures the exception in Sentry at warning level" do
-        expect(Sentry).to receive(:capture_exception).with(
-          error,
-          level: "warning"
-        )
-        perform
+      context "when not capturing the exception" do
+        it "doesn't capture the exception in Sentry" do
+          expect(Sentry).not_to receive(:capture_exception)
+          perform
+        end
+
+        it "does not raise the error" do
+          expect { perform }.not_to raise_error
+        end
       end
 
-      it "does not raise the error" do
-        expect { perform }.not_to raise_error
+      context "when capturing the exception" do
+        before do
+          Settings.location_position_updater_job.capture_exception = true
+        end
+
+        after { Settings.reload! }
+
+        it "captures the exception in Sentry at warning level" do
+          expect(Sentry).to receive(:capture_exception).with(
+            error,
+            level: "warning"
+          )
+          perform
+        end
+
+        it "does not raise the error" do
+          expect { perform }.not_to raise_error
+        end
       end
     end
 

spec/lib/location_position_updater_spec.rb

@@ -7,6 +7,7 @@
     let(:location) do
       create(
         :community_clinic,
+        name: "Westminster Clinic",
         address_line_1: "1 High Street",
         address_town: "London",
         address_postcode: "SW1A 1AA",
@@ -49,7 +50,7 @@
 
       it "calls the API with the address" do
         expect(OrdnanceSurvey::PlacesAPI).to receive(:find).with(
-          "1 High Street, London, SW1A 1AA"
+          "Westminster Clinic, 1 High Street, London, SW1A 1AA"
         )
         call
       end
@@ -93,6 +94,7 @@
       let(:location) do
         create(
           :community_clinic,
+          name: "Westminster Clinic",
           address_line_1: "1 High Street",
           address_town: nil,
           address_postcode: "SW1A 1AA",
@@ -115,7 +117,7 @@
 
       it "calls the API with partial address" do
         expect(OrdnanceSurvey::PlacesAPI).to receive(:find).with(
-          "1 High Street, SW1A 1AA"
+          "Westminster Clinic, 1 High Street, SW1A 1AA"
         )
         call
       end