From 5462b0be4b3f879184a5d53d9ca07ba42b69d0b5 Mon Sep 17 00:00:00 2001
From: Rob Galanakis <rob.galanakis@gmail.com>
Date: Mon, 25 May 2026 08:50:38 -0700
Subject: [PATCH] Improve marketing SMS dispatch resilience

Skip deleted users or invalid phone numbers.

Refactor code for clarity.

Consider ignoring errors and re-queueing them for later.
---
 lib/suma/marketing/sms_dispatch.rb       | 72 ++++++++++++------------
 spec/suma/marketing/sms_dispatch_spec.rb | 15 +++++
 2 files changed, 52 insertions(+), 35 deletions(-)

diff --git a/lib/suma/marketing/sms_dispatch.rb b/lib/suma/marketing/sms_dispatch.rb
index 8dc663de0..a9d7512a2 100644
--- a/lib/suma/marketing/sms_dispatch.rb
+++ b/lib/suma/marketing/sms_dispatch.rb
@@ -34,41 +34,7 @@ def send_all
           broadcast: dispatch.sms_broadcast.label,
         }
         Suma::Logutil.with_tags(log_tags) do
-          if dispatch.sms_broadcast.sending_number.blank?
-            self.logger.info("sms_dispatch_no_marketing_number")
-            dispatch.cancel.save_changes
-            next
-          end
-          if dispatch.member.message_preferences!.sms_undeliverable?
-            dispatch.cancel.save_changes
-            next
-          end
-          opted_out = dispatch.sms_broadcast.preferences_optout_field.present? &&
-            dispatch.member.message_preferences.opted_out?(dispatch.sms_broadcast.preferences_optout_field,
-                                                           :sms,)
-          if opted_out
-            dispatch.cancel.save_changes
-            next
-          end
-          body = dispatch.sms_broadcast.render(member: dispatch.member, language: nil)
-          if body.blank?
-            dispatch.cancel.save_changes
-            next
-          end
-          begin
-            sw_resp = Suma::Signalwire.send_sms(
-              Suma::PhoneNumber.format_e164(dispatch.sms_broadcast.sending_number),
-              Suma::PhoneNumber.format_e164(dispatch.member.phone),
-              body,
-            )
-          rescue Twilio::REST::RestError => e
-            self.logger.error("dispatch_marketing_broadcast_error", e)
-            Sentry.capture_exception(e, tags: log_tags)
-            next
-          end
-          self.logger.info("dispatched_marketing_broadcast", signalwire_message_id: sw_resp.sid)
-          dispatch.set_sent(sw_resp.sid)
-          dispatch.save_changes
+          dispatch.dispatch!(log_tags:)
         end
       end
     end
@@ -99,6 +65,42 @@ def cancel(at: Time.now)
     return self
   end
 
+  def cancel!(at: Time.now) = self.cancel(at:).save_changes
+
+  def dispatch!(log_tags: {})
+    if self.sms_broadcast.sending_number.blank?
+      self.logger.info("sms_dispatch_no_marketing_number")
+      return self.cancel!
+    end
+    return self.cancel! if self.member.soft_deleted?
+    begin
+      member_number = Suma::PhoneNumber.format_e164(self.member.phone)
+    rescue Suma::PhoneNumber::BadFormat
+      return self.cancel!
+    end
+    return self.cancel! if self.member.message_preferences!.sms_undeliverable?
+    opted_out = self.sms_broadcast.preferences_optout_field.present? &&
+      self.member.message_preferences.opted_out?(self.sms_broadcast.preferences_optout_field, :sms)
+    return self.cancel! if opted_out
+    body = self.sms_broadcast.render(member: self.member, language: nil)
+    return self.cancel! if body.blank?
+    begin
+      sw_resp = Suma::Signalwire.send_sms(
+        Suma::PhoneNumber.format_e164(self.sms_broadcast.sending_number),
+        member_number,
+        body,
+      )
+    rescue Twilio::REST::RestError => e
+      self.logger.error("dispatch_marketing_broadcast_error", e)
+      Sentry.capture_exception(e, tags: log_tags)
+      self.last_error = e.to_s
+      return self
+    end
+    self.logger.info("dispatched_marketing_broadcast", signalwire_message_id: sw_resp.sid)
+    self.set_sent(sw_resp.sid)
+    return self.save_changes
+  end
+
   def rel_admin_link = "/marketing-sms-dispatch/#{self.id}"
 
   def hybrid_search_fields
diff --git a/spec/suma/marketing/sms_dispatch_spec.rb b/spec/suma/marketing/sms_dispatch_spec.rb
index addb42649..c8d8bd8e0 100644
--- a/spec/suma/marketing/sms_dispatch_spec.rb
+++ b/spec/suma/marketing/sms_dispatch_spec.rb
@@ -121,6 +121,21 @@
       expect(d.refresh).to be_canceled
     end
 
+    it "cancels dispatches for deleted users" do
+      d = Suma::Fixtures.marketing_sms_dispatch.create
+      d.member.update(soft_deleted_at: Time.now)
+      described_class.send_all
+      expect(d.refresh).to be_canceled
+    end
+
+    it "cancels dispatches for invalid phone numbers" do
+      d = Suma::Fixtures.marketing_sms_dispatch.create
+      d.member.this.update(phone: Suma::PhoneNumber.faked_unreachable_phone)
+      d.refresh
+      described_class.send_all
+      expect(d.refresh).to be_canceled
+    end
+
     it "handles errors by sending them to Sentry and moving on" do
       disp = Suma::Fixtures.marketing_sms_dispatch.create
       req = stub_request(:post, "https://sumafaketest.signalwire.com/2010-04-01/Accounts/sw-test-project/Messages.json").