Handle invalid phone errors from GOV.UK Notify

This ensures that we handle the `InvalidPhoneError: Not a UK mobile
number` error from GOV.UK Notify by recording the response as a
permanent failure (which is accurate since retrying is not going to
succeed). This also means the job finishes and doesn't get put back on
the retry queue, and also allows the nurses to see useful feedback.

Sentry-Issue: 6235087330

Author: thomasleese

app/components/app_parent_summary_component.rb

@@ -108,20 +108,19 @@ def call
 
   delegate :govuk_summary_list, to: :helpers
 
+  EMAIL_FAILURE_TEXTS = {
+    "permanent_failure" => "Email address does not exist",
+    "temporary_failure" => "Inbox not accepting messages right now"
+  }.freeze
+
   def email_address
     delivery_status = @parent.email_delivery_status
 
     elements = [
       tag.p(@parent.email, class: "nhsuk-body nhsuk-u-margin-0"),
-      if delivery_status == "permanent_failure"
-        render AppStatusComponent.new(
-                 text: "Email address does not exist",
-                 colour: "red",
-                 small: true
-               )
-      elsif delivery_status == "temporary_failure"
+      if (failure_text = EMAIL_FAILURE_TEXTS[delivery_status])
         render AppStatusComponent.new(
-                 text: "Inbox not accepting messages right now",
+                 text: failure_text,
                  colour: "red",
                  small: true
                )
@@ -131,20 +130,21 @@ def email_address
     safe_join(elements)
   end
 
+  PHONE_FAILURE_TEXTS = {
+    "not_uk_mobile_number_failure" =>
+      "Phone number is a landline not accepting text messages",
+    "permanent_failure" => "Phone number does not exist",
+    "temporary_failure" => "Inbox not accepting messages right now"
+  }.freeze
+
   def phone_number
     delivery_status = @parent.sms_delivery_status
 
     elements = [
       tag.p(@parent.phone, class: "nhsuk-body nhsuk-u-margin-0"),
-      if delivery_status == "permanent_failure"
-        render AppStatusComponent.new(
-                 text: "Phone number does not exist",
-                 colour: "red",
-                 small: true
-               )
-      elsif delivery_status == "temporary_failure"
+      if (failure_text = PHONE_FAILURE_TEXTS[delivery_status])
         render AppStatusComponent.new(
-                 text: "Inbox not accepting messages right now",
+                 text: failure_text,
                  colour: "red",
                  small: true
                )

app/jobs/sms_delivery_job.rb

@@ -3,6 +3,8 @@
 class SMSDeliveryJob < NotifyDeliveryJob
   include NotifyThrottlingConcern
 
+  INVALID_UK_MOBILE_NUMBER_ERROR = "InvalidPhoneError: Not a UK mobile number"
+
   def perform(
     template_name,
     consent: nil,
@@ -43,30 +45,33 @@ def perform(
       template_id:
     }
 
-    delivery_id =
+    delivery_id, delivery_status =
       if self.class.send_via_notify?
         begin
-          self.class.client.send_sms(**args).id
+          [self.class.client.send_sms(**args).id, "sending"]
         rescue Notifications::Client::BadRequestError => e
           if !Rails.env.production? &&
                e.message.include?(TEAM_ONLY_API_KEY_MESSAGE)
             # Prevent retries and job failures.
             Sentry.capture_exception(e)
+          elsif e.message == INVALID_UK_MOBILE_NUMBER_ERROR
+            [nil, "not_uk_mobile_number_failure"]
           else
             raise
           end
         end
       elsif self.class.send_via_test?
         self.class.deliveries << args
-        SecureRandom.uuid
+        [SecureRandom.uuid, "delivered"]
       else
         Rails.logger.info "Sending SMS to #{phone_number} with template #{template_id}"
-        nil
+        [nil, "delivered"]
       end
 
     NotifyLogEntry.create!(
       consent_form: personalisation.consent_form,
       delivery_id:,
+      delivery_status:,
       parent: personalisation.parent,
       patient: personalisation.patient,
       programme_ids: personalisation.programmes.map(&:id),

app/models/notify_log_entry.rb

@@ -49,7 +49,8 @@ class NotifyLogEntry < ApplicationRecord
          delivered: 1,
          permanent_failure: 2,
          temporary_failure: 3,
-         technical_failure: 4
+         technical_failure: 4,
+         not_uk_mobile_number_failure: 5
        }
 
   validates :recipient, presence: true

spec/jobs/sms_delivery_job_spec.rb

@@ -97,6 +97,36 @@
       end
     end
 
+    context "when the parent phone number is invalid" do
+      before do
+        allow(notifications_client).to receive(:send_sms).and_raise(
+          Notifications::Client::BadRequestError.new(
+            OpenStruct.new(
+              code: 400,
+              body: "InvalidPhoneError: Not a UK mobile number"
+            )
+          )
+        )
+      end
+
+      it "creates a log entry for the failure" do
+        expect { perform_now }.to change(NotifyLogEntry, :count).by(1)
+
+        notify_log_entry = NotifyLogEntry.last
+        expect(notify_log_entry).to be_sms
+        expect(notify_log_entry).to be_not_uk_mobile_number_failure
+        expect(notify_log_entry.delivery_id).to be_nil
+        expect(notify_log_entry.recipient).to eq("01234 567890")
+        expect(notify_log_entry.template_id).to eq(
+          GOVUK_NOTIFY_SMS_TEMPLATES[template_name]
+        )
+        expect(notify_log_entry.parent).to eq(parent)
+        expect(notify_log_entry.patient).to eq(patient)
+        expect(notify_log_entry.programme_ids).to eq(programmes.map(&:id))
+        expect(notify_log_entry.sent_by).to eq(sent_by)
+      end
+    end
+
     context "with a consent form" do
       let(:consent_form) do
         create(:consent_form, session:, parent_phone: "01234567890")
@@ -137,6 +167,35 @@
           perform_now
         end
       end
+
+      context "when the parent phone number is invalid" do
+        before do
+          allow(notifications_client).to receive(:send_sms).and_raise(
+            Notifications::Client::BadRequestError.new(
+              OpenStruct.new(
+                code: 400,
+                body: "InvalidPhoneError: Not a UK mobile number"
+              )
+            )
+          )
+        end
+
+        it "creates a log entry for the failure" do
+          expect { perform_now }.to change(NotifyLogEntry, :count).by(1)
+
+          notify_log_entry = NotifyLogEntry.last
+          expect(notify_log_entry).to be_sms
+          expect(notify_log_entry).to be_not_uk_mobile_number_failure
+          expect(notify_log_entry.delivery_id).to be_nil
+          expect(notify_log_entry.recipient).to eq("01234 567890")
+          expect(notify_log_entry.template_id).to eq(
+            GOVUK_NOTIFY_SMS_TEMPLATES[template_name]
+          )
+          expect(notify_log_entry.consent_form).to eq(consent_form)
+          expect(notify_log_entry.programme_ids).to eq(programmes.map(&:id))
+          expect(notify_log_entry.sent_by).to eq(sent_by)
+        end
+      end
     end
   end