diff --git a/README.md b/README.md index 8edd6bd15..ea3636115 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,6 @@ export NOTIFY_ENVIRONMENT='development' export MMG_API_KEY='MMG_API_KEY' export FIRETEXT_API_KEY='FIRETEXT_ACTUAL_KEY' -export REACH_API_KEY='REACH_API_KEY' export NOTIFICATION_QUEUE_PREFIX='YOUR_OWN_PREFIX' export FLASK_APP=application.py @@ -46,7 +45,6 @@ Things to change: ``` notify-pass credentials/firetext notify-pass credentials/mmg -notify-pass credentials/reach ``` ### Postgres diff --git a/app/__init__.py b/app/__init__.py index b7bf85d8a..cfc70d841 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -36,7 +36,6 @@ from app.clients.email.aws_ses import AwsSesClient from app.clients.email.aws_ses_stub import AwsSesStubClient from app.clients.sms.firetext import FiretextClient from app.clients.sms.mmg import MMGClient -from app.clients.sms.reach import ReachClient class SQLAlchemy(_SQLAlchemy): @@ -57,7 +56,6 @@ ma = Marshmallow() notify_celery = NotifyCelery() firetext_client = FiretextClient() mmg_client = MMGClient() -reach_client = ReachClient() aws_ses_client = AwsSesClient() aws_ses_stub_client = AwsSesStubClient() encryption = Encryption() @@ -100,7 +98,6 @@ def create_app(application): logging.init_app(application, statsd_client) firetext_client.init_app(application, statsd_client=statsd_client) mmg_client.init_app(application, statsd_client=statsd_client) - reach_client.init_app(application, statsd_client=statsd_client) aws_ses_client.init_app(application.config['AWS_REGION'], statsd_client=statsd_client) aws_ses_stub_client.init_app( @@ -111,7 +108,7 @@ def create_app(application): # If a stub url is provided for SES, then use the stub client rather than the real SES boto client email_clients = [aws_ses_stub_client] if application.config['SES_STUB_URL'] else [aws_ses_client] notification_provider_clients.init_app( - sms_clients=[firetext_client, mmg_client, reach_client], + sms_clients=[firetext_client, mmg_client], email_clients=email_clients ) diff --git a/app/celery/process_sms_client_response_tasks.py b/app/celery/process_sms_client_response_tasks.py index b8f1f63bb..b414250e6 100644 --- a/app/celery/process_sms_client_response_tasks.py +++ b/app/celery/process_sms_client_response_tasks.py @@ -8,7 +8,6 @@ from app import notify_celery, statsd_client from app.clients import ClientException from app.clients.sms.firetext import get_firetext_responses from app.clients.sms.mmg import get_mmg_responses -from app.clients.sms.reach import get_reach_responses from app.dao import notifications_dao from app.dao.templates_dao import dao_get_template_by_id from app.models import NOTIFICATION_PENDING @@ -19,7 +18,6 @@ from app.notifications.notifications_ses_callback import ( sms_response_mapper = { 'MMG': get_mmg_responses, 'Firetext': get_firetext_responses, - 'Reach': get_reach_responses } diff --git a/app/clients/sms/reach.py b/app/clients/sms/reach.py deleted file mode 100644 index 3541f8b5c..000000000 --- a/app/clients/sms/reach.py +++ /dev/null @@ -1,52 +0,0 @@ -import json - -from requests import RequestException, request - -from app.clients.sms import SmsClient, SmsClientResponseException - - -def get_reach_responses(status, detailed_status_code=None): - if status == 'TODO-d': - return ("delivered", "TODO: Delivered") - elif status == 'TODO-tf': - return ("temporary-failure", "TODO: Temporary failure") - elif status == 'TODO-pf': - return ("permanent-failure", "TODO: Permanent failure") - else: - raise KeyError - - -class ReachClient(SmsClient): - def init_app(self, *args, **kwargs): - super().init_app(*args, **kwargs) - self.url = self.current_app.config.get('REACH_URL') - - @property - def name(self): - return 'reach' - - def try_send_sms(self, to, content, reference, international, sender): - data = { - # TODO - } - - try: - response = request( - "POST", - self.url, - data=json.dumps(data), - headers={ - 'Content-Type': 'application/json', - }, - timeout=60 - ) - - response.raise_for_status() - try: - json.loads(response.text) - except (ValueError, AttributeError): - raise SmsClientResponseException("Invalid response JSON") - except RequestException: - raise SmsClientResponseException("Request failed") - - return response diff --git a/app/config.py b/app/config.py index 2b359724d..f927a3be0 100644 --- a/app/config.py +++ b/app/config.py @@ -381,7 +381,6 @@ class Config(object): # these environment vars aren't defined in the manifest so to set them on paas use `cf set-env` MMG_URL = os.environ.get("MMG_URL", "https://api.mmg.co.uk/jsonv2a/api.php") FIRETEXT_URL = os.environ.get("FIRETEXT_URL", "https://www.firetext.co.uk/api/sendsms/json") - REACH_URL = os.environ.get("REACH_URL", "TODO") SES_STUB_URL = os.environ.get("SES_STUB_URL") AWS_REGION = 'eu-west-1' @@ -485,7 +484,6 @@ class Test(Development): MMG_URL = 'https://example.com/mmg' FIRETEXT_URL = 'https://example.com/firetext' - REACH_URL = 'https://example.com/reach' CBC_PROXY_ENABLED = True DVLA_EMAIL_ADDRESSES = ['success@simulator.amazonses.com', 'success+2@simulator.amazonses.com'] diff --git a/app/notifications/notifications_sms_callback.py b/app/notifications/notifications_sms_callback.py index f7353d45c..f0896d745 100644 --- a/app/notifications/notifications_sms_callback.py +++ b/app/notifications/notifications_sms_callback.py @@ -54,28 +54,6 @@ def process_firetext_response(): return jsonify(result='success'), 200 -@sms_callback_blueprint.route('/reach', methods=['POST']) -def process_reach_response(): - client_name = 'Reach' - - # TODO: validate request - errors = None - - if errors: - raise InvalidRequest(errors, status_code=400) - - status = 'TODO-d' # TODO - detailed_status_code = 'something' # TODO - provider_reference = 'notification_id' # TODO - - process_sms_client_response.apply_async( - [status, provider_reference, client_name, detailed_status_code], - queue=QueueNames.SMS_CALLBACKS, - ) - - return jsonify(result='success'), 200 - - def validate_callback_data(data, fields, client_name): errors = [] for f in fields: diff --git a/manifest.yml.j2 b/manifest.yml.j2 index facd0807e..eeda63212 100644 --- a/manifest.yml.j2 +++ b/manifest.yml.j2 @@ -27,9 +27,9 @@ 'STATSD_HOST': None }, 'routes': { - 'preview': ['api.notify.works/notifications/sms/mmg', 'api.notify.works/notifications/sms/firetext', 'api.notify.works/notifications/sms/reach','notify-api-sms-receipts-preview.apps.internal'], - 'staging': ['api.staging-notify.works/notifications/sms/mmg', 'api.staging-notify.works/notifications/sms/firetext', 'api.staging-notify.works/notifications/sms/reach', 'notify-api-sms-receipts-staging.apps.internal'], - 'production': ['api.notifications.service.gov.uk/notifications/sms/mmg', 'api.notifications.service.gov.uk/notifications/sms/firetext', 'api.notifications.service.gov.uk/notifications/sms/reach', 'notify-api-sms-receipts-production.apps.internal' ], + 'preview': ['api.notify.works/notifications/sms/mmg', 'api.notify.works/notifications/sms/firetext', 'notify-api-sms-receipts-preview.apps.internal'], + 'staging': ['api.staging-notify.works/notifications/sms/mmg', 'api.staging-notify.works/notifications/sms/firetext', 'notify-api-sms-receipts-staging.apps.internal'], + 'production': ['api.notifications.service.gov.uk/notifications/sms/mmg', 'api.notifications.service.gov.uk/notifications/sms/firetext', 'notify-api-sms-receipts-production.apps.internal' ], }, 'health-check-type': 'port', 'health-check-invocation-timeout': 3, diff --git a/migrations/versions/0370_remove_reach.py b/migrations/versions/0370_remove_reach.py new file mode 100644 index 000000000..4572fbddc --- /dev/null +++ b/migrations/versions/0370_remove_reach.py @@ -0,0 +1,54 @@ +""" + +Revision ID: 0370_remove_reach +Revises: 0369_update_sms_rates +Create Date: 2022-04-27 16:00:00 + +""" +import itertools +import uuid +from datetime import datetime + +from alembic import op +from sqlalchemy.sql import text + +from app.models import LetterRate + + +revision = '0370_remove_reach' +down_revision = '0369_update_sms_rates' + + +def upgrade(): + conn = op.get_bind() + conn.execute("DELETE FROM provider_details WHERE identifier = 'reach'") + + +def downgrade(): + conn = op.get_bind() + conn.execute( + """ + INSERT INTO provider_details ( + id, + display_name, + identifier, + priority, + notification_type, + active, + version, + created_by_id + ) + VALUES ( + '{}', + 'Reach', + 'reach', + 0, + 'sms', + false, + 1, + null + ) + """.format( + str(uuid.uuid4()), + ) + ) diff --git a/tests/app/celery/test_process_sms_client_response_tasks.py b/tests/app/celery/test_process_sms_client_response_tasks.py index 046566906..6005fbce3 100644 --- a/tests/app/celery/test_process_sms_client_response_tasks.py +++ b/tests/app/celery/test_process_sms_client_response_tasks.py @@ -18,7 +18,7 @@ def test_process_sms_client_response_raises_error_if_reference_is_not_a_valid_uu status='000', provider_reference='something-bad', client_name='sms-client') -@pytest.mark.parametrize('client_name', ('Firetext', 'MMG', 'Reach')) +@pytest.mark.parametrize('client_name', ('Firetext', 'MMG')) def test_process_sms_response_raises_client_exception_for_unknown_status( sample_notification, mocker, @@ -43,9 +43,6 @@ def test_process_sms_response_raises_client_exception_for_unknown_status( ('3', '2', 'MMG', 'delivered', "Delivered to operator"), ('4', '27', 'MMG', 'temporary-failure', "Absent Subscriber"), ('5', '13', 'MMG', 'permanent-failure', "Sender id blacklisted"), - ('TODO-d', None, 'Reach', 'delivered', "TODO: Delivered"), - ('TODO-tf', None, 'Reach', 'temporary-failure', "TODO: Temporary failure"), - ('TODO-pf', None, 'Reach', 'permanent-failure', "TODO: Permanent failure"), ]) def test_process_sms_client_response_updates_notification_status( sample_notification, diff --git a/tests/app/clients/test_reach.py b/tests/app/clients/test_reach.py deleted file mode 100644 index b62a5fd0b..000000000 --- a/tests/app/clients/test_reach.py +++ /dev/null @@ -1,84 +0,0 @@ -import pytest -import requests_mock -from requests.exceptions import ConnectTimeout, ReadTimeout - -from app import reach_client -from app.clients.sms import SmsClientResponseException - -# TODO: tests for get_reach_responses - - -def test_try_send_sms_successful_returns_reach_response(notify_api, mocker): - to = content = reference = 'foo' - response_dict = {} # TODO - - with requests_mock.Mocker() as request_mock: - request_mock.post('https://example.com/reach', json=response_dict, status_code=200) - response = reach_client.try_send_sms(to, content, reference, False, 'sender') - - # response_json = response.json() - assert response.status_code == 200 - # TODO: assertions - - -def test_try_send_sms_calls_reach_correctly(notify_api, mocker): - to = '+447234567890' - content = 'my message' - reference = 'my reference' - response_dict = {} # TODO - - with requests_mock.Mocker() as request_mock: - request_mock.post('https://example.com/reach', json=response_dict, status_code=200) - reach_client.try_send_sms(to, content, reference, False, 'sender') - - assert request_mock.call_count == 1 - assert request_mock.request_history[0].url == 'https://example.com/reach' - assert request_mock.request_history[0].method == 'POST' - - # request_args = request_mock.request_history[0].json() - # TODO: assertions - - -def test_try_send_sms_raises_if_reach_rejects(notify_api, mocker): - to = content = reference = 'foo' - response_dict = { - 'Error': 206, - 'Description': 'Some kind of error' - } - - with pytest.raises(SmsClientResponseException) as exc, requests_mock.Mocker() as request_mock: - request_mock.post('https://example.com/reach', json=response_dict, status_code=400) - reach_client.try_send_sms(to, content, reference, False, 'sender') - - assert "Request failed" in str(exc) - - -def test_try_send_sms_raises_if_reach_fails_to_return_json(notify_api, mocker): - to = content = reference = 'foo' - response_dict = 'NOT AT ALL VALID JSON {"key" : "value"}}' - - with pytest.raises(SmsClientResponseException) as exc, requests_mock.Mocker() as request_mock: - request_mock.post('https://example.com/reach', text=response_dict, status_code=200) - reach_client.try_send_sms(to, content, reference, False, 'sender') - - assert 'Invalid response JSON' in str(exc.value) - - -def test_try_send_sms_raises_if_reach_rejects_with_connect_timeout(rmock): - to = content = reference = 'foo' - - with pytest.raises(SmsClientResponseException) as exc: - rmock.register_uri('POST', 'https://example.com/reach', exc=ConnectTimeout) - reach_client.try_send_sms(to, content, reference, False, 'sender') - - assert 'Request failed' in str(exc.value) - - -def test_try_send_sms_raises_if_reach_rejects_with_read_timeout(rmock): - to = content = reference = 'foo' - - with pytest.raises(SmsClientResponseException) as exc: - rmock.register_uri('POST', 'https://example.com/reach', exc=ReadTimeout) - reach_client.try_send_sms(to, content, reference, False, 'sender') - - assert 'Request failed' in str(exc.value) diff --git a/tests/app/notifications/test_notifications_sms_callbacks.py b/tests/app/notifications/test_notifications_sms_callbacks.py index 1c1a02f01..b3bb7b46e 100644 --- a/tests/app/notifications/test_notifications_sms_callbacks.py +++ b/tests/app/notifications/test_notifications_sms_callbacks.py @@ -17,13 +17,6 @@ def mmg_post(client, data): headers=[('Content-Type', 'application/json')]) -def reach_post(client, data): - return client.post( - path='/notifications/sms/reach', - data=data, - headers=[('Content-Type', 'application/json')]) - - def test_firetext_callback_should_not_need_auth(client, mocker): mocker.patch('app.notifications.notifications_sms_callback.process_sms_client_response') data = 'mobile=441234123123&status=0&reference=notification_id&time=2016-03-10 14:17:00' @@ -142,24 +135,6 @@ def test_mmg_callback_should_return_200_and_call_task_with_valid_data(client, mo ) -# TODO: more tests about edge cases for this provider -def test_reach_callback_should_return_200_and_call_task_with_valid_data(client, mocker): - mock_celery = mocker.patch( - 'app.notifications.notifications_sms_callback.process_sms_client_response.apply_async') - data = json.dumps({"data": "TODO"}) - - response = reach_post(client, data) - - assert response.status_code == 200 - json_data = json.loads(response.data) - assert json_data['result'] == 'success' - - mock_celery.assert_called_once_with( - ['TODO-d', 'notification_id', 'Reach', 'something'], - queue='sms-callbacks', - ) - - def test_validate_callback_data_returns_none_when_valid(): form = {'status': 'good', 'reference': 'send-sms-code'} diff --git a/tests/app/provider_details/test_rest.py b/tests/app/provider_details/test_rest.py index 4e973568f..6d1e28ffc 100644 --- a/tests/app/provider_details/test_rest.py +++ b/tests/app/provider_details/test_rest.py @@ -11,7 +11,7 @@ def test_get_provider_details_returns_all_providers(admin_request, notify_db_ses json_resp = admin_request.get('provider_details.get_providers')['provider_details'] assert len(json_resp) > 0 - assert {'ses', 'firetext', 'mmg', 'dvla'} < {x['identifier'] for x in json_resp} + assert {'ses', 'firetext', 'mmg', 'dvla'} <= {x['identifier'] for x in json_resp} def test_get_provider_details_by_id(client, notify_db):