Merge pull request #3526 from alphagov/remove-reach

Remove support for Reach provider

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

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
     )
 

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
 }
 
 

app/clients/sms/reach.py

@@ -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

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']

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:

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,

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()),
+        )
+    )

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,

tests/app/clients/test_reach.py

@@ -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)

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'}

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):