Skip to content

Commit

Permalink
Create new DRYRUN_TEST_NUMBER (#2333)
Browse files Browse the repository at this point in the history
  • Loading branch information
sastels authored Nov 6, 2024
1 parent 6bd7a65 commit 2955fd1
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 12 deletions.
4 changes: 2 additions & 2 deletions app/clients/sms/aws_pinpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def send_sms(self, to, content, reference, multi=True, sender=None, template_id=
ConfigurationSetName=self.current_app.config["AWS_PINPOINT_CONFIGURATION_SET_NAME"],
)
else:
dryrun = destinationNumber == self.current_app.config["INTERNAL_TEST_NUMBER"]
dryrun = destinationNumber == self.current_app.config["EXTERNAL_TEST_NUMBER"]
response = self._client.send_text_message(
DestinationPhoneNumber=destinationNumber,
OriginationIdentity=pool_id,
Expand All @@ -63,7 +63,7 @@ def send_sms(self, to, content, reference, multi=True, sender=None, template_id=
)
if dryrun:
self.current_app.logger.info(
f"Dry run enabled for SMS to {self.current_app.config['INTERNAL_TEST_NUMBER']} with message id {response.get('MessageId')}. SMS not sent by AWS."
f"SMS with message id {response.get('MessageId')} is sending to EXTERNAL_TEST_NUMBER. Boto call made to AWS, but not send on."
)
except self._client.exceptions.ConflictException as e:
if e.response.get("Reason") == "DESTINATION_PHONE_NUMBER_OPTED_OUT":
Expand Down
1 change: 1 addition & 0 deletions app/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,7 @@ class Config(object):

# Match with scripts/internal_stress_test/internal_stress_test.py
INTERNAL_TEST_NUMBER = "+16135550123"
EXTERNAL_TEST_NUMBER = "+16135550124"
INTERNAL_TEST_EMAIL_ADDRESS = "[email protected]"

DVLA_BUCKETS = {
Expand Down
14 changes: 8 additions & 6 deletions app/delivery/send_to_providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ def send_sms_to_provider(notification):
inactive_service_failure(notification=notification)
return

formatted_recipient = validate_and_format_phone_number(notification.to, international=notification.international)
sending_to_internal_test_number = formatted_recipient == current_app.config["INTERNAL_TEST_NUMBER"]
sending_to_dryrun_number = formatted_recipient == current_app.config["EXTERNAL_TEST_NUMBER"]

# If the notification was not sent already, the status should be created.
if notification.status == "created":
provider = provider_to_use(
Expand All @@ -93,7 +97,8 @@ def send_sms_to_provider(notification):
empty_message_failure(notification=notification)
return

if service.research_mode or notification.key_type == KEY_TYPE_TEST:
if service.research_mode or notification.key_type == KEY_TYPE_TEST or sending_to_internal_test_number:
current_app.logger.info(f"notification {notification.id} is sending to INTERNAL_TEST_NUMBER, no boto call to AWS.")
notification.reference = send_sms_response(provider.get_name(), notification.to)
update_notification_to_sending(notification, provider)
else:
Expand All @@ -106,7 +111,7 @@ def send_sms_to_provider(notification):
else:
sending_vehicle = None
reference = provider.send_sms(
to=validate_and_format_phone_number(notification.to, international=notification.international),
to=formatted_recipient,
content=str(template),
reference=str(notification.id),
sender=notification.reply_to_text,
Expand All @@ -125,10 +130,7 @@ def send_sms_to_provider(notification):
if reference == "opted_out":
update_notification_to_opted_out(notification, provider)
else:
if (
validate_and_format_phone_number(notification.to, international=notification.international)
== current_app.config["INTERNAL_TEST_NUMBER"]
):
if sending_to_dryrun_number:
send_sms_response(provider.get_name(), notification.to, reference)
update_notification_to_sending(notification, provider)

Expand Down
6 changes: 3 additions & 3 deletions tests/app/clients/test_aws_pinpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,20 +158,20 @@ def test_send_sms_sends_international_without_pool_id(notify_api, mocker, sample

@pytest.mark.serial
@pytest.mark.parametrize("template_id", [None, "uuid"])
def test_send_sms_uses_dryrun_for_tests(notify_api, mocker, sample_template, template_id):
def test_send_sms_uses_dryrun(notify_api, mocker, sample_template, template_id):
boto_mock = mocker.patch.object(aws_pinpoint_client, "_client", create=True)
mocker.patch.object(aws_pinpoint_client, "statsd_client", create=True)
content = "foo"
reference = "ref"
to = "+16135550123"
to = "+16135550111"
with set_config_values(
notify_api,
{
"AWS_PINPOINT_SC_POOL_ID": "sc_pool_id",
"AWS_PINPOINT_DEFAULT_POOL_ID": "default_pool_id",
"AWS_PINPOINT_CONFIGURATION_SET_NAME": "config_set_name",
"AWS_PINPOINT_SC_TEMPLATE_IDS": [],
"INTERNAL_TEST_NUMBER": to,
"EXTERNAL_TEST_NUMBER": to,
},
):
aws_pinpoint_client.send_sms(to, content, reference=reference, template_id=template_id)
Expand Down
20 changes: 19 additions & 1 deletion tests/app/delivery/test_send_to_providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,24 @@ def test_should_not_send_sms_message_when_message_is_empty_or_whitespace(sample_
assert Notification.query.get(notification.id).status == "technical-failure"


def test_should_not_send_sms_message_to_internal_test_number(sample_service, mocker):
template = create_template(sample_service)
notification = save_notification(
create_notification(
template=template,
to_field=Config.INTERNAL_TEST_NUMBER,
status="created",
reply_to_text=sample_service.get_default_sms_sender(),
)
)
mocker.patch("app.delivery.send_to_providers.send_sms_response", return_value="reference")
send_mock = mocker.patch("app.aws_sns_client.send_sms")
send_to_providers.send_sms_to_provider(notification)

send_mock.assert_not_called()
assert Notification.query.get(notification.id).status == "sent"


def test_send_sms_should_use_template_version_from_notification_not_latest(sample_template, mocker):
db_notification = save_notification(
create_notification(
Expand Down Expand Up @@ -584,7 +602,7 @@ def test_send_email_to_provider_should_not_send_to_provider_when_status_is_not_c
mocker.patch("app.aws_ses_client.send_email")
mocker.patch("app.delivery.send_to_providers.send_email_response")

send_to_providers.send_sms_to_provider(notification)
send_to_providers.send_email_to_provider(notification)
app.aws_ses_client.send_email.assert_not_called()
app.delivery.send_to_providers.send_email_response.assert_not_called()

Expand Down

0 comments on commit 2955fd1

Please sign in to comment.