Add and use PollingAlertCheckWorker

- AlertCheckWorker likely to cause false alarms due to the callbacks being 429'd, so add a backup where if we have no delivered emails, we actively poll for them until we find a delivered one.

app/services/check_notify_email_service.rb

@@ -0,0 +1,35 @@
+class CheckNotifyEmailService
+  def initialize(status)
+    @status = status
+    @client = Notifications::Client.new(Rails.application.credentials.notify_api_key)
+  end
+
+  def present?(reference)
+    results = @client.get_notifications(
+      template_type: "email",
+      status: @status,
+      reference:,
+    )
+
+    results.collection.count.positive?
+  rescue Notifications::Client::RequestError, Net::OpenTimeout, Net::ReadTimeout, SocketError => e
+    Rails.logger.debug("Unable to contact Notify to determine status of email reference #{reference}: #{e}")
+  end
+
+private
+
+  attr_reader :email, :client
+
+  def undeliverable_failure?(error)
+    return false unless error.is_a?(Notifications::Client::BadRequestError)
+
+    return true if error.message.end_with?("Not a valid email address")
+
+    # This is a stop-gap fix for an issue where this system can produce emails
+    # that are so long they exceed Notify's limit of 2MB. As emails of this
+    # size will never succeed we mark them as failures. Ideally this will
+    # eventually be resolved by some product thinking that only allows the
+    # system to create emails within a reasonable length.
+    error.message.start_with?("BadRequestError: Your message is too long")
+  end
+end

app/workers/polling_alert_check_worker.rb

@@ -0,0 +1,33 @@
+class PollingAlertCheckWorker < ApplicationWorker
+  include SearchAlertList
+
+  def perform(document_type)
+    content_items = get_alert_content_items(document_type:)
+    delivered = 0
+    content_items.each do |ci|
+      if any_emails_delivered_for?(ci[:content_id], ci[:valid_from], document_type)
+        delivered += 1
+      else
+        Rails.logger.warn("Couldn't find any delivered emails for #{document_type.titleize} records with content id #{ci[:content_id]} (at #{ci[:url]})")
+      end
+    end
+
+    Rails.logger.info("Checking #{document_type.titleize} records: #{delivered} out of #{content_items.count} alerts have been delivered to at least one recipient")
+
+    Rails.cache.write("current_#{document_type}s", content_items.count, expires_in: 15.minutes)
+    Rails.cache.write("delivered_#{document_type}s", delivered, expires_in: 15.minutes)
+  end
+
+  def any_emails_delivered_for?(content_id, valid_from, document_type)
+    return true if Email.where("notify_status = 'delivered' AND content_id = ? AND created_at > ?", content_id, valid_from).exists?
+
+    Rails.logger.info("First pass couldn't find any delivered emails for #{document_type.titleize} records with content id #{content_id}, reverting to polling")
+
+    service = CheckNotifyEmailService.new("delivered")
+    Email.sent.where("content_id = ? AND created_at > ? AND notify_status IS NULL", content_id, valid_from).find_each do |email|
+      return true if service.present?(email.id)
+    end
+
+    false
+  end
+end

config/sidekiq.yml

@@ -41,9 +41,9 @@
       class: RecoverLostJobsWorker
     check_medical_safety_alerts:
       every: '15m'
-      class: AlertCheckWorker
+      class: PollingAlertCheckWorker
       args:  ["medical_safety_alert"]
     check_travel_advice_alerts:
       every: '15m'
-      class: AlertCheckWorker
+      class: PollingAlertCheckWorker
       args:  ["travel_advice"]

spec/support/notify_request_helpers.rb

@@ -4,6 +4,31 @@ def stub_notify
     stub_request(:post, /notifications\.service\.gov\.uk/).to_return(body:)
   end
 
+  def stub_notify_email_delivery_status_delivered(reference)
+    stub_request(:get, "https://api.notifications.service.gov.uk/v2/notifications?reference=#{reference}&status=delivered&template_type=email").to_return(
+      body: {
+        notifications: [
+          {
+            "id": "740e5834-3a29-46b4-9a6f-16142fde533a",
+            "reference": reference,
+            "type": "email",
+            "status": "delivered",
+          },
+        ],
+        "links": {},
+      }.to_json,
+    )
+  end
+
+  def stub_notify_email_delivery_status_delivered_empty(reference)
+    stub_request(:get, "https://api.notifications.service.gov.uk/v2/notifications?reference=#{reference}&status=delivered&template_type=email").to_return(
+      body: {
+        notifications: [],
+        "links": {},
+      }.to_json,
+    )
+  end
+
   def expect_an_email_was_sent(subject: /.*/, address: "[email protected]")
     request_data = nil
 

spec/workers/polling_alert_check_worker_spec.rb

@@ -0,0 +1,88 @@
+RSpec.describe PollingAlertCheckWorker do
+  include SearchAlertListHelpers
+  include NotifyRequestHelpers
+
+  describe "#perform", caching: true do
+    def perform
+      described_class.new.perform("medical_safety_alert")
+    end
+
+    context "there are no alerts older than an hour" do
+      before { stub_medical_safety_alert_feed(content_id: SecureRandom.uuid, age: 30.minutes) }
+
+      it "should put 0/0 in the cache" do
+        expect(Rails.cache).to receive(:write).with("current_medical_safety_alerts", 0, expires_in: 15.minutes)
+        expect(Rails.cache).to receive(:write).with("delivered_medical_safety_alerts", 0, expires_in: 15.minutes)
+        perform
+      end
+    end
+
+    context "there are no alerts younger than 2 days" do
+      before { stub_medical_safety_alert_feed(content_id: SecureRandom.uuid, age: 3.days) }
+
+      it "should put 0/0 in the cache" do
+        expect(Rails.cache).to receive(:write).with("current_medical_safety_alerts", 0, expires_in: 15.minutes)
+        expect(Rails.cache).to receive(:write).with("delivered_medical_safety_alerts", 0, expires_in: 15.minutes)
+        perform
+      end
+    end
+
+    context "there is a valid alert with delivered emails" do
+      before do
+        content_id = SecureRandom.uuid
+        stub_medical_safety_alert_feed(content_id:, age: 2.hours)
+        create(:email, content_id:, notify_status: "delivered", status: "sent")
+      end
+
+      it "should put 1/1 in the cache" do
+        expect(Rails.cache).to receive(:write).with("current_medical_safety_alerts", 1, expires_in: 15.minutes)
+        expect(Rails.cache).to receive(:write).with("delivered_medical_safety_alerts", 1, expires_in: 15.minutes)
+        perform
+      end
+    end
+
+    context "there is a valid alert with undelivered emails" do
+      before do
+        content_id = SecureRandom.uuid
+        stub_medical_safety_alert_feed(content_id:, age: 2.hours)
+        email = create(:email, content_id:, notify_status: nil, status: "sent")
+        stub_notify_email_delivery_status_delivered_empty(email.id)
+      end
+
+      it "should put 1/0 in the cache" do
+        expect(Rails.cache).to receive(:write).with("current_medical_safety_alerts", 1, expires_in: 15.minutes)
+        expect(Rails.cache).to receive(:write).with("delivered_medical_safety_alerts", 0, expires_in: 15.minutes)
+        perform
+      end
+    end
+
+    context "there is a valid alert with apparently undelivered emails but they actually were delivered" do
+      before do
+        content_id = SecureRandom.uuid
+        stub_medical_safety_alert_feed(content_id:, age: 2.hours)
+        email = create(:email, content_id:, notify_status: nil, status: "sent")
+        stub_notify_email_delivery_status_delivered(email.id)
+      end
+
+      it "should put 1/0 in the cache" do
+        expect(Rails.cache).to receive(:write).with("current_medical_safety_alerts", 1, expires_in: 15.minutes)
+        expect(Rails.cache).to receive(:write).with("delivered_medical_safety_alerts", 1, expires_in: 15.minutes)
+        perform
+      end
+    end
+
+    context "there is a valid alert with delivered emails that are too old to be valid" do
+      before do
+        content_id = SecureRandom.uuid
+        stub_medical_safety_alert_feed(content_id:, age: 2.hours)
+        create(:email, content_id:, notify_status: "delivered", created_at: Time.zone.now - 3.hours, status: "sent")
+      end
+
+      it "should put 1/0 in the cache" do
+        expect(Rails.cache).to receive(:write).with("current_medical_safety_alerts", 1, expires_in: 15.minutes)
+        expect(Rails.cache).to receive(:write).with("delivered_medical_safety_alerts", 0, expires_in: 15.minutes)
+        perform
+      end
+    end
+  end
+end