Skip to content

Commit

Permalink
fix/opted out phone numbers in pinpoint (#2207)
Browse files Browse the repository at this point in the history
  • Loading branch information
sastels authored Jul 8, 2024
1 parent 447c69d commit 45442d0
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 3 deletions.
13 changes: 11 additions & 2 deletions app/clients/sms/aws_pinpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ def get_name(self):
def send_sms(self, to, content, reference, multi=True, sender=None, template_id=None):
messageType = "TRANSACTIONAL"
matched = False
opted_out = False
response = {}

if template_id is not None and str(template_id) in self.current_app.config["AWS_PINPOINT_SC_TEMPLATE_IDS"]:
pool_id = self.current_app.config["AWS_PINPOINT_SC_POOL_ID"]
Expand All @@ -32,6 +34,7 @@ def send_sms(self, to, content, reference, multi=True, sender=None, template_id=

for match in phonenumbers.PhoneNumberMatcher(to, "US"):
matched = True
opted_out = False
to = phonenumbers.format_number(match.number, phonenumbers.PhoneNumberFormat.E164)
destinationNumber = to

Expand All @@ -44,15 +47,21 @@ def send_sms(self, to, content, reference, multi=True, sender=None, template_id=
MessageType=messageType,
ConfigurationSetName=self.current_app.config["AWS_PINPOINT_CONFIGURATION_SET_NAME"],
)
except self._client.exceptions.ConflictException as e:
if e.response.get("Reason") == "DESTINATION_PHONE_NUMBER_OPTED_OUT":
opted_out = True
else:
raise e

except Exception as e:
self.statsd_client.incr("clients.pinpoint.error")
raise Exception(e)
raise e
finally:
elapsed_time = monotonic() - start_time
self.current_app.logger.info("AWS Pinpoint request finished in {}".format(elapsed_time))
self.statsd_client.timing("clients.pinpoint.request-time", elapsed_time)
self.statsd_client.incr("clients.pinpoint.success")
return response["MessageId"]
return "opted_out" if opted_out else response.get("MessageId")

if not matched:
self.statsd_client.incr("clients.pinpoint.error")
Expand Down
14 changes: 13 additions & 1 deletion app/delivery/send_to_providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
EMAIL_TYPE,
KEY_TYPE_TEST,
NOTIFICATION_CONTAINS_PII,
NOTIFICATION_PERMANENT_FAILURE,
NOTIFICATION_SENDING,
NOTIFICATION_SENT,
NOTIFICATION_TECHNICAL_FAILURE,
Expand Down Expand Up @@ -118,7 +119,10 @@ def send_sms_to_provider(notification):
else:
notification.reference = reference
notification.billable_units = template.fragment_count
update_notification_to_sending(notification, provider)
if reference == "opted_out":
update_notification_to_opted_out(notification, provider)
else:
update_notification_to_sending(notification, provider)

# Record StatsD stats to compute SLOs
statsd_client.timing_with_dates("sms.total-time", notification.sent_at, notification.created_at)
Expand Down Expand Up @@ -340,6 +344,14 @@ def update_notification_to_sending(notification, provider):
dao_update_notification(notification)


def update_notification_to_opted_out(notification, provider):
notification.sent_at = datetime.utcnow()
notification.sent_by = provider.get_name()
notification.status = NOTIFICATION_PERMANENT_FAILURE
notification.provider_response = "Phone number is opted out"
dao_update_notification(notification)


def provider_to_use(
notification_type: str,
notification_id: UUID,
Expand Down
12 changes: 12 additions & 0 deletions tests/app/clients/test_aws_pinpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,15 @@ def test_send_sms_returns_raises_error_if_there_is_no_valid_number_is_found(noti
aws_pinpoint_client.send_sms(to, content, reference)

assert "No valid numbers found for SMS delivery" in str(excinfo.value)


def test_handles_opted_out_numbers(notify_api, mocker, sample_template):
conflict_error = aws_pinpoint_client._client.exceptions.ConflictException(
error_response={"Reason": "DESTINATION_PHONE_NUMBER_OPTED_OUT"}, operation_name="send_text_message"
)
mocker.patch("app.aws_pinpoint_client._client.send_text_message", side_effect=conflict_error)

to = "6135555555"
content = "foo"
reference = "ref"
assert aws_pinpoint_client.send_sms(to, content, reference=reference, template_id=sample_template.id) == "opted_out"
25 changes: 25 additions & 0 deletions tests/app/delivery/test_send_to_providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,31 @@ def test_should_return_highest_priority_active_provider(restore_provider_details
assert send_to_providers.provider_to_use("sms", "1234").name == first.identifier


def test_should_handle_opted_out_phone_numbers_if_using_pinpoint(notify_api, sample_template, mocker):
mocker.patch("app.aws_pinpoint_client.send_sms", return_value="opted_out")
db_notification = save_notification(
create_notification(
template=sample_template,
to_field="+16135551234",
status="created",
reply_to_text=sample_template.service.get_default_sms_sender(),
)
)

with set_config_values(
notify_api,
{
"AWS_PINPOINT_SC_POOL_ID": "sc_pool_id",
"AWS_PINPOINT_DEFAULT_POOL_ID": "default_pool_id",
},
):
send_to_providers.send_sms_to_provider(db_notification)

notification = Notification.query.filter_by(id=db_notification.id).one()
assert notification.status == "permanent-failure"
assert notification.provider_response == "Phone number is opted out"


def test_should_send_personalised_template_to_correct_sms_provider_and_persist(sample_sms_template_with_html, mocker):
db_notification = save_notification(
create_notification(
Expand Down

0 comments on commit 45442d0

Please sign in to comment.