Merge pull request #7464 from DFE-Digital/track-analytics-on-google-g…

…eocoding-api-hits

Track analytics on google geocoding api hits

app/controllers/api/location_suggestion_controller.rb

@@ -2,7 +2,7 @@ class Api::LocationSuggestionController < Api::ApplicationController
   before_action :verify_json_request, only: %i[show]
   before_action :check_valid_params, only: %i[show]
 
-  # Try to reduce our Google Places API bill
+  # By caching this action, we considerably reduce our Google Places API bill
   caches_action :show
 
   def show

config/analytics_custom_events.yml

@@ -4,6 +4,7 @@ shared:
   - equal_opportunities_report_published
   - failed_dsi_sign_in_attempt
   - form_validation_failed
+  - google_geocoding_api_hit
   - job_alert_subscription_created
   - job_alert_subscription_unsubscribed
   - job_alert_subscription_updated

lib/geocoding.rb

@@ -15,8 +15,11 @@ def coordinates
     return self.class.test_coordinates if Rails.application.config.geocoder_lookup == :test
 
     Rails.cache.fetch([:geocoding, location], expires_in: CACHE_DURATION, skip_nil: true) do
-      Geocoder.coordinates(location, lookup: :google, components: "country:gb")
+      Geocoder.coordinates(location, lookup: :google, components: "country:gb").tap do |coords|
+        trigger_google_geocoding_api_hit_event(type: :coordinates, location:, result: coords)
+      end
     rescue Geocoder::OverQueryLimitError
+      trigger_google_geocoding_api_hit_event(type: :coordinates, location:, result: "OVER_QUERY_LIMIT")
       Rails.logger.error("Google Geocoding API responded with OVER_QUERY_LIMIT")
       Geocoder.coordinates(location, lookup: :uk_ordnance_survey_names)
     end || no_coordinates_match
@@ -27,15 +30,31 @@ def postcode_from_coordinates
 
     Rails.cache.fetch([:postcode_from_coords, location], expires_in: CACHE_DURATION, skip_nil: true) do
       result = Geocoder.search(location, lookup: :google).first
-      result.data["address_components"].find { |line| "postal_code".in?(line["types"]) }&.dig("short_name") unless result.nil?
+      if result.present?
+        postcode = result.data["address_components"].find { |line| "postal_code".in?(line["types"]) }&.dig("short_name")
+        trigger_google_geocoding_api_hit_event(type: :postcode, location:, result: postcode)
+        postcode
+      else
+        trigger_google_geocoding_api_hit_event(type: :postcode, location:, result: nil)
+        nil
+      end
     rescue Geocoder::OverQueryLimitError
+      trigger_google_geocoding_api_hit_event(type: :postcode, location:, result: "OVER_QUERY_LIMIT")
       Rails.logger.error("Google Geocoding API responded with OVER_QUERY_LIMIT")
       fallback_postcode_from_coords
     end || no_postcode_match
   end
 
   private
 
+  def trigger_google_geocoding_api_hit_event(type:, location:, result:)
+    event = DfE::Analytics::Event.new
+      .with_type(:google_geocoding_api_hit)
+      .with_data(data: { type:, location: location.to_s, result: result&.to_s })
+
+    DfE::Analytics::SendEvents.do([event])
+  end
+
   def fallback_postcode_from_coords(service: :nominatim)
     result = Geocoder.search(location, lookup: service).first.data
     if result["error"].present?

spec/lib/geocoding_spec.rb

@@ -1,7 +1,8 @@
 require "rails_helper"
 require "geocoding"
 
-RSpec.describe Geocoding, geocode: true do
+# rubocop:disable RSpec/ExpectActual
+RSpec.describe Geocoding, :dfe_analytics, geocode: true do
   subject { described_class.new(location) }
 
   let(:google_coordinates) { [54.5399146, -1.0435559] }
@@ -27,19 +28,38 @@
         expect(Geocoder).not_to receive(:coordinates)
         subject.coordinates
       end
+
+      it "does not trigger a Google Geocoding API hit event" do
+        subject.coordinates
+        expect(:google_geocoding_api_hit).not_to have_been_enqueued_as_analytics_event
+      end
     end
 
     context "when location does not have a cache entry" do
       context "when Geocoder with `lookup: :google` returns a valid response", vcr: { cassette_name: "geocoder_google_valid" } do
         it "returns the coordinates" do
           expect(subject.coordinates).to eq(google_coordinates)
         end
+
+        it "triggers a Google Geocoding API hit event" do
+          subject.coordinates
+          expect(:google_geocoding_api_hit).to have_been_enqueued_as_analytics_event(
+            with_data: { type: "coordinates", location: location, result: google_coordinates.to_s },
+          )
+        end
       end
 
       context "when Geocoder with `lookup: :google` returns an empty response", vcr: { cassette_name: "geocoder_google_empty" } do
         it "returns no_match" do
           expect(subject.coordinates).to eq(no_match)
         end
+
+        it "triggers a Google Geocoding API hit event" do
+          subject.coordinates
+          expect(:google_geocoding_api_hit).to have_been_enqueued_as_analytics_event(
+            with_data: { type: "coordinates", location: location, result: nil },
+          )
+        end
       end
 
       context "when Geocoder with `lookup: :google` returns status OVER_QUERY_LIMIT", vcr: { cassette_name: "geocoder_google_over_query_limit_os_valid" } do
@@ -48,6 +68,13 @@
           subject.coordinates
         end
 
+        it "triggers a Google Geocoding API hit event" do
+          subject.coordinates
+          expect(:google_geocoding_api_hit).to have_been_enqueued_as_analytics_event(
+            with_data: { type: "coordinates", location: location, result: "OVER_QUERY_LIMIT" },
+          )
+        end
+
         context "when Geocoder with `lookup: :uk_ordnance_survey_names` returns a valid response" do
           it "returns the coordinates" do
             expect(subject.coordinates.map { |coord| coord.round(7) }).to eq(os_coordinates)
@@ -85,19 +112,38 @@
         expect(Geocoder).not_to receive(:search)
         subject.postcode_from_coordinates
       end
+
+      it "does not trigger a Google Geocoding API hit event" do
+        subject.postcode_from_coordinates
+        expect(:google_geocoding_api_hit).not_to have_been_enqueued_as_analytics_event
+      end
     end
 
     context "when the coordinates do not have a cache entry" do
       context "when Geocoder with `lookup: :google` returns a valid response", vcr: { cassette_name: "geocoder_google_postcode_lookup_valid" } do
         it "returns the postcode" do
           expect(subject.postcode_from_coordinates).to eq(postcode)
         end
+
+        it "triggers a Google Geocoding API hit event" do
+          subject.postcode_from_coordinates
+          expect(:google_geocoding_api_hit).to have_been_enqueued_as_analytics_event(
+            with_data: { type: "postcode", location: google_coordinates.to_s, result: postcode },
+          )
+        end
       end
 
       context "when Geocoder with `lookup: :google` returns an empty response", vcr: { cassette_name: "geocoder_google_postcode_lookup_empty" } do
         it "returns no match" do
           expect(subject.postcode_from_coordinates).to be_nil
         end
+
+        it "triggers a Google Geocoding API hit event" do
+          subject.postcode_from_coordinates
+          expect(:google_geocoding_api_hit).to have_been_enqueued_as_analytics_event(
+            with_data: { type: "postcode", location: google_coordinates.to_s, result: nil },
+          )
+        end
       end
     end
 
@@ -107,6 +153,13 @@
         subject.postcode_from_coordinates
       end
 
+      it "triggers a Google Geocoding API hit event" do
+        subject.postcode_from_coordinates
+        expect(:google_geocoding_api_hit).to have_been_enqueued_as_analytics_event(
+          with_data: { type: "postcode", location: google_coordinates.to_s, result: "OVER_QUERY_LIMIT" },
+        )
+      end
+
       context "when Geocoder with `lookup: :nominatim` returns a valid response" do
         it "returns the postcode" do
           expect(subject.postcode_from_coordinates).to eq("TS14 6RD")
@@ -127,3 +180,4 @@
     end
   end
 end
+# rubocop:enable RSpec/ExpectActual