From dde5b96c62073344bc7660b958353e48552bf52a Mon Sep 17 00:00:00 2001 From: Ryan Westfall Date: Mon, 10 Aug 2026 12:56:18 -0500 Subject: [PATCH] Fix SMTP2GO SMS send payload for current API schema. Use destination/content with E.164 numbers and surface provider error bodies so 400s are actionable. --- site/messaging/providers/sms/smtp2go.py | 51 ++++++++++++-- site/messaging/tests.py | 93 +++++++++++++++++++++++++ 2 files changed, 139 insertions(+), 5 deletions(-) diff --git a/site/messaging/providers/sms/smtp2go.py b/site/messaging/providers/sms/smtp2go.py index a6d9fff..ea6ee66 100644 --- a/site/messaging/providers/sms/smtp2go.py +++ b/site/messaging/providers/sms/smtp2go.py @@ -1,6 +1,7 @@ """SMTP2GO SMS REST API.""" import logging +import re import requests from django.conf import settings @@ -10,6 +11,36 @@ from messaging.services import render_merge_tags logger = logging.getLogger(__name__) +def _format_destination(phone: str) -> str: + """Normalize stored phone to E.164-ish string SMTP2GO accepts.""" + digits = re.sub(r"\D", "", phone or "") + if not digits: + raise ValueError("Contact has no phone number") + if phone.strip().startswith("+") and digits: + return f"+{digits}" + # US 10-digit local numbers → +1… + if len(digits) == 10: + return f"+1{digits}" + if len(digits) == 11 and digits.startswith("1"): + return f"+{digits}" + return f"+{digits}" + + +def _provider_error_detail(response: requests.Response) -> str: + """Prefer SMTP2GO JSON error text over bare HTTP reason.""" + try: + data = response.json() + except ValueError: + text = (response.text or "").strip() + return text[:500] if text else response.reason + nested = data.get("data") if isinstance(data.get("data"), dict) else {} + err = nested.get("error") or data.get("error") or "" + code = nested.get("error_code") or data.get("error_code") or "" + if err and code: + return f"{err} ({code})" + return str(err or code or response.reason) + + def send_sms(message) -> str: contact = message.contact if not contact.phone: @@ -24,23 +55,33 @@ def send_sms(message) -> str: campaign.template.body if campaign.template else "" ) body = render_merge_tags(body, contact) + destination = _format_destination(contact.phone) + # Current SMTP2GO /v3/sms/send schema: destination[] + content. payload = { "api_key": api_key, - "to": contact.phone, - "text": body[:1600], + "destination": [destination], + "content": body[:1600], } response = requests.post( settings.SMTP2GO_SMS_API_URL, json=payload, timeout=30, ) - response.raise_for_status() + if not response.ok: + detail = _provider_error_detail(response) + raise requests.HTTPError( + f"{response.status_code} Client Error: {detail} for url: {response.url}", + response=response, + ) + data = response.json() if response.content else {} - # Prefer SMS id fields used on webhooks (`message_id` / `sms_id`). nested = data.get("data") if isinstance(data.get("data"), dict) else {} + messages = nested.get("messages") if isinstance(nested.get("messages"), list) else [] + first = messages[0] if messages and isinstance(messages[0], dict) else {} return str( - nested.get("sms_id") + first.get("message_id") + or nested.get("sms_id") or nested.get("message_id") or data.get("sms_id") or data.get("message_id") diff --git a/site/messaging/tests.py b/site/messaging/tests.py index 24209cd..4673970 100644 --- a/site/messaging/tests.py +++ b/site/messaging/tests.py @@ -1329,3 +1329,96 @@ class CampaignRecipientTableTests(TestCase): response = self.client.get(url) self.assertContains(response, "Recent PCM Integrations events") self.assertNotContains(response, "Recent SMTP2GO events") + + +class Smtp2goSmsSendTests(TestCase): + def setUp(self): + self.contact = Contact.objects.create( + phone="330-402-2675", + first_name="Rufus", + last_name="Firefly", + ) + set_channel_consent( + self.contact, Channel.SMS, opted_in=True, reason="test" + ) + self.campaign = create_campaign_draft( + name="SMS blast", + audience=Campaign.Audience.SMS_OPT_IN, + body="Hello {{first_name}}", + ) + self.message = self.campaign.messages.get(contact=self.contact) + + def test_format_destination_us_local(self): + from messaging.providers.sms.smtp2go import _format_destination + + self.assertEqual(_format_destination("330-402-2675"), "+13304022675") + self.assertEqual(_format_destination("+1 (330) 402-2675"), "+13304022675") + self.assertEqual(_format_destination("13304022675"), "+13304022675") + + def test_send_sms_uses_destination_and_content(self): + from unittest.mock import MagicMock, patch + + from messaging.providers.sms.smtp2go import send_sms + + resp = MagicMock() + resp.ok = True + resp.content = b'{"request_id":"req-1","data":{"messages":[{"message_id":"sms-abc","destination":"+13304022675","status":"queued"}],"total_sent":1}}' + resp.json.return_value = { + "request_id": "req-1", + "data": { + "messages": [ + { + "message_id": "sms-abc", + "destination": "+13304022675", + "status": "queued", + } + ], + "total_sent": 1, + }, + } + + with self.settings(SMTP2GO_SMS_API_KEY="api-test-key"): + with patch( + "messaging.providers.sms.smtp2go.requests.post", + return_value=resp, + ) as post: + provider_id = send_sms(self.message) + + self.assertEqual(provider_id, "sms-abc") + payload = post.call_args.kwargs["json"] + self.assertEqual(payload["api_key"], "api-test-key") + self.assertEqual(payload["destination"], ["+13304022675"]) + self.assertEqual(payload["content"], "Hello Rufus") + self.assertNotIn("to", payload) + self.assertNotIn("text", payload) + + def test_send_sms_includes_api_error_body(self): + from unittest.mock import MagicMock, patch + + import requests + + from messaging.providers.sms.smtp2go import send_sms + + resp = MagicMock() + resp.ok = False + resp.status_code = 400 + resp.url = "https://api.smtp2go.com/v3/sms/send" + resp.reason = "Bad Request" + resp.text = '{"data":{"error":"Missing required field","error_code":"E_ApiResponseCodes.INVALID_REQUEST"}}' + resp.json.return_value = { + "data": { + "error": "Missing required field", + "error_code": "E_ApiResponseCodes.INVALID_REQUEST", + } + } + + with self.settings(SMTP2GO_SMS_API_KEY="api-test-key"): + with patch( + "messaging.providers.sms.smtp2go.requests.post", + return_value=resp, + ): + with self.assertRaises(requests.HTTPError) as ctx: + send_sms(self.message) + + self.assertIn("Missing required field", str(ctx.exception)) + self.assertIn("INVALID_REQUEST", str(ctx.exception))