From f3fdd3b09b15f9ce6787073f892d3a38df5eee14 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 16 Mar 2021 14:53:42 +0000 Subject: [PATCH] Add internation api key for firetext. We want to start using Firetext for sending international SMS. They require us to use a different API key for international SMS because it requires a new code path to switch the sender ID to something that the country will accept. This PR does not include switching the sender of international SMS to Firetext but sets us up to do so. --- app/clients/sms/firetext.py | 6 ++-- app/clients/sms/mmg.py | 2 +- app/config.py | 1 + app/delivery/send_to_providers.py | 3 +- manifest.yml.j2 | 1 + tests/app/clients/test_firetext.py | 38 ++++++++++++++++---- tests/app/clients/test_mmg.py | 14 ++++---- tests/app/conftest.py | 1 + tests/app/delivery/test_send_to_providers.py | 29 +++++++++------ 9 files changed, 66 insertions(+), 29 deletions(-) diff --git a/app/clients/sms/firetext.py b/app/clients/sms/firetext.py index a93c9d6c5..39099e02a 100644 --- a/app/clients/sms/firetext.py +++ b/app/clients/sms/firetext.py @@ -66,6 +66,7 @@ class FiretextClient(SmsClient): super(SmsClient, self).__init__(*args, **kwargs) self.current_app = current_app self.api_key = current_app.config.get('FIRETEXT_API_KEY') + self.international_api_key = current_app.config.get('FIRETEXT_INTERNATIONAL_API_KEY') self.from_number = current_app.config.get('FROM_NUMBER') self.name = 'firetext' self.url = current_app.config.get('FIRETEXT_URL') @@ -91,10 +92,9 @@ class FiretextClient(SmsClient): self.statsd_client.incr("clients.firetext.error") self.current_app.logger.warning(log_message) - def send_sms(self, to, content, reference, sender=None): - + def send_sms(self, to, content, reference, international, sender=None): data = { - "apiKey": self.api_key, + "apiKey": self.international_api_key if international else self.api_key, "from": self.from_number if sender is None else sender, "to": to.replace('+', ''), "message": content, diff --git a/app/clients/sms/mmg.py b/app/clients/sms/mmg.py index e8390f260..fdaa99ccd 100644 --- a/app/clients/sms/mmg.py +++ b/app/clients/sms/mmg.py @@ -97,7 +97,7 @@ class MMGClient(SmsClient): def get_name(self): return self.name - def send_sms(self, to, content, reference, multi=True, sender=None): + def send_sms(self, to, content, reference, international, multi=True, sender=None): data = { "reqType": "BULK", "MSISDN": to, diff --git a/app/config.py b/app/config.py index 82269e82d..09465a561 100644 --- a/app/config.py +++ b/app/config.py @@ -99,6 +99,7 @@ class Config(object): # Firetext API Key FIRETEXT_API_KEY = os.getenv("FIRETEXT_API_KEY") + FIRETEXT_INTERNATIONAL_API_KEY = os.getenv("FIRETEXT_INTERNATIONAL_API_KEY", "placeholder") # Prefix to identify queues in SQS NOTIFICATION_QUEUE_PREFIX = os.getenv('NOTIFICATION_QUEUE_PREFIX') diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 1cb9896fc..0a0012c92 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -68,7 +68,8 @@ def send_sms_to_provider(notification): to=notification.normalised_to, content=str(template), reference=str(notification.id), - sender=notification.reply_to_text + sender=notification.reply_to_text, + international=notification.international ) except Exception as e: notification.billable_units = template.fragment_count diff --git a/manifest.yml.j2 b/manifest.yml.j2 index bb4eca8ba..42f052ead 100644 --- a/manifest.yml.j2 +++ b/manifest.yml.j2 @@ -138,6 +138,7 @@ applications: MMG_INBOUND_SMS_USERNAME: '{{ MMG_INBOUND_SMS_USERNAME | tojson }}' FIRETEXT_API_KEY: '{{ FIRETEXT_API_KEY }}' + FIRETEXT_INTERNATIONAL_API_KEY: '{{ FIRETEXT_INTERNATIONAL_API_KEY }}' FIRETEXT_INBOUND_SMS_AUTH: '{{ FIRETEXT_INBOUND_SMS_AUTH | tojson }}' REDIS_ENABLED: '{{ REDIS_ENABLED }}' diff --git a/tests/app/clients/test_firetext.py b/tests/app/clients/test_firetext.py index 1052cf480..a24fba3ba 100644 --- a/tests/app/clients/test_firetext.py +++ b/tests/app/clients/test_firetext.py @@ -47,7 +47,7 @@ def test_send_sms_successful_returns_firetext_response(mocker, mock_firetext_cli with requests_mock.Mocker() as request_mock: request_mock.post('https://example.com/firetext', json=response_dict, status_code=200) - response = mock_firetext_client.send_sms(to, content, reference) + response = mock_firetext_client.send_sms(to, content, reference, False) response_json = response.json() assert response.status_code == 200 @@ -65,7 +65,7 @@ def test_send_sms_calls_firetext_correctly(mocker, mock_firetext_client): with requests_mock.Mocker() as request_mock: request_mock.post('https://example.com/firetext', json=response_dict, status_code=200) - mock_firetext_client.send_sms(to, content, reference) + mock_firetext_client.send_sms(to, content, reference, False) assert request_mock.call_count == 1 assert request_mock.request_history[0].url == 'https://example.com/firetext' @@ -79,6 +79,30 @@ def test_send_sms_calls_firetext_correctly(mocker, mock_firetext_client): assert request_args['reference'][0] == reference +def test_send_sms_calls_firetext_correctly_for_international(mocker, mock_firetext_client): + to = '+607234567890' + content = 'my message' + reference = 'my reference' + response_dict = { + 'code': 0, + } + + with requests_mock.Mocker() as request_mock: + request_mock.post('https://example.com/firetext', json=response_dict, status_code=200) + mock_firetext_client.send_sms(to, content, reference, True) + + assert request_mock.call_count == 1 + assert request_mock.request_history[0].url == 'https://example.com/firetext' + assert request_mock.request_history[0].method == 'POST' + + request_args = parse_qs(request_mock.request_history[0].text) + assert request_args['apiKey'][0] == 'international' + assert request_args['from'][0] == 'bar' + assert request_args['to'][0] == '607234567890' + assert request_args['message'][0] == content + assert request_args['reference'][0] == reference + + def test_send_sms_raises_if_firetext_rejects(mocker, mock_firetext_client): to = content = reference = 'foo' response_dict = { @@ -90,7 +114,7 @@ def test_send_sms_raises_if_firetext_rejects(mocker, mock_firetext_client): with pytest.raises(SmsClientResponseException) as exc, requests_mock.Mocker() as request_mock: request_mock.post('https://example.com/firetext', json=response_dict, status_code=200) - mock_firetext_client.send_sms(to, content, reference) + mock_firetext_client.send_sms(to, content, reference, False) assert exc.value.status_code == 200 assert '"description": "Some kind of error"' in exc.value.text @@ -103,7 +127,7 @@ def test_send_sms_raises_if_firetext_rejects_with_unexpected_data(mocker, mock_f with pytest.raises(SmsClientResponseException) as exc, requests_mock.Mocker() as request_mock: request_mock.post('https://example.com/firetext', json=response_dict, status_code=400) - mock_firetext_client.send_sms(to, content, reference) + mock_firetext_client.send_sms(to, content, reference, False) assert exc.value.status_code == 400 assert exc.value.text == '{"something": "gone bad"}' @@ -121,7 +145,7 @@ def test_send_sms_override_configured_shortcode_with_sender(mocker, mock_firetex with requests_mock.Mocker() as request_mock: request_mock.post('https://example.com/firetext', json=response_dict, status_code=200) - mock_firetext_client.send_sms(to, content, reference, sender=sender) + mock_firetext_client.send_sms(to, content, reference, False, sender=sender) request_args = parse_qs(request_mock.request_history[0].text) assert request_args['from'][0] == 'fromservice' @@ -132,7 +156,7 @@ def test_send_sms_raises_if_firetext_rejects_with_connect_timeout(rmock, mock_fi with pytest.raises(FiretextClientResponseException) as exc: rmock.register_uri('POST', 'https://example.com/firetext', exc=ConnectTimeout) - mock_firetext_client.send_sms(to, content, reference) + mock_firetext_client.send_sms(to, content, reference, False) assert exc.value.status_code == 504 assert exc.value.text == 'Gateway Time-out' @@ -143,7 +167,7 @@ def test_send_sms_raises_if_firetext_rejects_with_read_timeout(rmock, mock_firet with pytest.raises(FiretextClientResponseException) as exc: rmock.register_uri('POST', 'https://example.com/firetext', exc=ReadTimeout) - mock_firetext_client.send_sms(to, content, reference) + mock_firetext_client.send_sms(to, content, reference, False) assert exc.value.status_code == 504 assert exc.value.text == 'Gateway Time-out' diff --git a/tests/app/clients/test_mmg.py b/tests/app/clients/test_mmg.py index dd1fc771c..096c57207 100644 --- a/tests/app/clients/test_mmg.py +++ b/tests/app/clients/test_mmg.py @@ -44,7 +44,7 @@ def test_send_sms_successful_returns_mmg_response(notify_api, mocker): with requests_mock.Mocker() as request_mock: request_mock.post('https://example.com/mmg', json=response_dict, status_code=200) - response = mmg_client.send_sms(to, content, reference) + response = mmg_client.send_sms(to, content, reference, False) response_json = response.json() assert response.status_code == 200 @@ -59,7 +59,7 @@ def test_send_sms_calls_mmg_correctly(notify_api, mocker): with requests_mock.Mocker() as request_mock: request_mock.post('https://example.com/mmg', json=response_dict, status_code=200) - mmg_client.send_sms(to, content, reference) + mmg_client.send_sms(to, content, reference, False) assert request_mock.call_count == 1 assert request_mock.request_history[0].url == 'https://example.com/mmg' @@ -83,7 +83,7 @@ def test_send_sms_raises_if_mmg_rejects(notify_api, mocker): with pytest.raises(SmsClientResponseException) as exc, requests_mock.Mocker() as request_mock: request_mock.post('https://example.com/mmg', json=response_dict, status_code=400) - mmg_client.send_sms(to, content, reference) + mmg_client.send_sms(to, content, reference, False) assert exc.value.status_code == 400 assert '"Error": 206' in exc.value.text @@ -100,7 +100,7 @@ def test_send_sms_override_configured_shortcode_with_sender(notify_api, mocker): with requests_mock.Mocker() as request_mock: request_mock.post('https://example.com/mmg', json=response_dict, status_code=200) - mmg_client.send_sms(to, content, reference, sender=sender) + mmg_client.send_sms(to, content, reference, False, sender=sender) request_args = request_mock.request_history[0].json() assert request_args['sender'] == 'fromservice' @@ -112,7 +112,7 @@ def test_send_sms_raises_if_mmg_fails_to_return_json(notify_api, mocker): with pytest.raises(SmsClientResponseException) as exc, requests_mock.Mocker() as request_mock: request_mock.post('https://example.com/mmg', text=response_dict, status_code=200) - mmg_client.send_sms(to, content, reference) + mmg_client.send_sms(to, content, reference, False) assert 'Code 200 text NOT AT ALL VALID JSON {"key" : "value"}} exception Expecting value: line 1 column 1 (char 0)' in str(exc.value) # noqa assert exc.value.status_code == 200 @@ -124,7 +124,7 @@ def test_send_sms_raises_if_mmg_rejects_with_connect_timeout(rmock): with pytest.raises(MMGClientResponseException) as exc: rmock.register_uri('POST', 'https://example.com/mmg', exc=ConnectTimeout) - mmg_client.send_sms(to, content, reference) + mmg_client.send_sms(to, content, reference, False) assert exc.value.status_code == 504 assert exc.value.text == 'Gateway Time-out' @@ -135,7 +135,7 @@ def test_send_sms_raises_if_mmg_rejects_with_read_timeout(rmock): with pytest.raises(MMGClientResponseException) as exc: rmock.register_uri('POST', 'https://example.com/mmg', exc=ReadTimeout) - mmg_client.send_sms(to, content, reference) + mmg_client.send_sms(to, content, reference, False) assert exc.value.status_code == 504 assert exc.value.text == 'Gateway Time-out' diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 6fbb88e4b..41e5b8e36 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -594,6 +594,7 @@ def mock_firetext_client(mocker): current_app = mocker.Mock(config={ 'FIRETEXT_URL': 'https://example.com/firetext', 'FIRETEXT_API_KEY': 'foo', + 'FIRETEXT_INTERNATIONAL_API_KEY': 'international', 'FROM_NUMBER': 'bar' }) client.init_app(current_app, statsd_client) diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 6f99b1f94..16e8a817c 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -123,7 +123,8 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist( to="447234123123", content="Sample service: Hello Jo\nHere is some HTML & entities", reference=str(db_notification.id), - sender=current_app.config['FROM_NUMBER'] + sender=current_app.config['FROM_NUMBER'], + international=False ) notification = Notification.query.filter_by(id=db_notification.id).one() @@ -158,7 +159,7 @@ def test_should_send_personalised_template_to_correct_email_provider_and_persist 'Jo some HTML', body='Hello Jo\nThis is an email from GOV.\u200bUK with some HTML\n', html_body=ANY, - reply_to_address=None + reply_to_address=None, ) assert '