mirror of
https://github.com/GSA/notifications-api.git
synced 2026-02-01 07:35:34 -05:00
Be agnostic about format when comparing phone #s
If a service is in restricted mode then a user can’t send messages to anyone other than themselves and members of their team. To do this the API has to compare the numbers they are sending to with those of their team members. It will (falsely) say the numbers do not match if they are in a different format, eg 07700 900849 vs +447700900849 This commit uses the code we use elsewhere for formatting phone numbers to make sure that both numbers are in a consistent format before doing a comparison. I have a strong preference for doing it this way, rather than formatting numbers before we store them: 1. https://en.wikipedia.org/wiki/Robustness_principle 2. It’s confusing to a user to see their own phone number formatted in a different way to that which they entered it, and the alternative, storing the phone number in two different formats is grim
This commit is contained in:
@@ -296,13 +296,13 @@ def test_should_send_sms_without_personalisation(sample_template, mocker):
|
||||
|
||||
|
||||
def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notify_db_session, mocker):
|
||||
user = sample_user(notify_db, notify_db_session, mobile_numnber="+441234123123")
|
||||
user = sample_user(notify_db, notify_db_session, mobile_numnber="07700 900890")
|
||||
service = sample_service(notify_db, notify_db_session, user=user, restricted=True)
|
||||
template = sample_template(notify_db, notify_db_session, service=service)
|
||||
|
||||
notification = {
|
||||
"template": template.id,
|
||||
"to": "+441234123123"
|
||||
"to": "+447700900890" # The user’s own number, but in a different format
|
||||
}
|
||||
mocker.patch('app.encryption.decrypt', return_value=notification)
|
||||
mocker.patch('app.firetext_client.send_sms')
|
||||
@@ -317,17 +317,17 @@ def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notif
|
||||
now.strftime(DATETIME_FORMAT)
|
||||
)
|
||||
|
||||
firetext_client.send_sms.assert_called_once_with("+441234123123", "Sample service: This is a template")
|
||||
firetext_client.send_sms.assert_called_once_with("+447700900890", "Sample service: This is a template")
|
||||
|
||||
|
||||
def test_should_not_send_sms_if_restricted_service_and_invalid_number(notify_db, notify_db_session, mocker):
|
||||
user = sample_user(notify_db, notify_db_session, mobile_numnber="+441234123123")
|
||||
user = sample_user(notify_db, notify_db_session, mobile_numnber="07700 900205")
|
||||
service = sample_service(notify_db, notify_db_session, user=user, restricted=True)
|
||||
template = sample_template(notify_db, notify_db_session, service=service)
|
||||
|
||||
notification = {
|
||||
"template": template.id,
|
||||
"to": "+440000000000"
|
||||
"to": "07700 900849"
|
||||
}
|
||||
mocker.patch('app.encryption.decrypt', return_value=notification)
|
||||
mocker.patch('app.firetext_client.send_sms')
|
||||
|
||||
Reference in New Issue
Block a user