Handle invalid phone errors from GOV.UK Notify

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