Remove support for Reach provider

This provider was never active and support was never completed, so
there's little value in keeping all this potentially confusing code.
This commit is contained in:
Ben Thorner
2022-04-27 18:03:05 +01:00
parent a4fe11a3aa
commit c27107fa74
12 changed files with 60 additions and 201 deletions

View File

@@ -29,7 +29,6 @@ export NOTIFY_ENVIRONMENT='development'
export MMG_API_KEY='MMG_API_KEY' export MMG_API_KEY='MMG_API_KEY'
export FIRETEXT_API_KEY='FIRETEXT_ACTUAL_KEY' export FIRETEXT_API_KEY='FIRETEXT_ACTUAL_KEY'
export REACH_API_KEY='REACH_API_KEY'
export NOTIFICATION_QUEUE_PREFIX='YOUR_OWN_PREFIX' export NOTIFICATION_QUEUE_PREFIX='YOUR_OWN_PREFIX'
export FLASK_APP=application.py export FLASK_APP=application.py
@@ -46,7 +45,6 @@ Things to change:
``` ```
notify-pass credentials/firetext notify-pass credentials/firetext
notify-pass credentials/mmg notify-pass credentials/mmg
notify-pass credentials/reach
``` ```
### Postgres ### Postgres

View File

@@ -36,7 +36,6 @@ from app.clients.email.aws_ses import AwsSesClient
from app.clients.email.aws_ses_stub import AwsSesStubClient from app.clients.email.aws_ses_stub import AwsSesStubClient
from app.clients.sms.firetext import FiretextClient from app.clients.sms.firetext import FiretextClient
from app.clients.sms.mmg import MMGClient from app.clients.sms.mmg import MMGClient
from app.clients.sms.reach import ReachClient
class SQLAlchemy(_SQLAlchemy): class SQLAlchemy(_SQLAlchemy):
@@ -57,7 +56,6 @@ ma = Marshmallow()
notify_celery = NotifyCelery() notify_celery = NotifyCelery()
firetext_client = FiretextClient() firetext_client = FiretextClient()
mmg_client = MMGClient() mmg_client = MMGClient()
reach_client = ReachClient()
aws_ses_client = AwsSesClient() aws_ses_client = AwsSesClient()
aws_ses_stub_client = AwsSesStubClient() aws_ses_stub_client = AwsSesStubClient()
encryption = Encryption() encryption = Encryption()
@@ -100,7 +98,6 @@ def create_app(application):
logging.init_app(application, statsd_client) logging.init_app(application, statsd_client)
firetext_client.init_app(application, statsd_client=statsd_client) firetext_client.init_app(application, statsd_client=statsd_client)
mmg_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_client.init_app(application.config['AWS_REGION'], statsd_client=statsd_client)
aws_ses_stub_client.init_app( 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 # 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] email_clients = [aws_ses_stub_client] if application.config['SES_STUB_URL'] else [aws_ses_client]
notification_provider_clients.init_app( notification_provider_clients.init_app(
sms_clients=[firetext_client, mmg_client, reach_client], sms_clients=[firetext_client, mmg_client],
email_clients=email_clients email_clients=email_clients
) )

View File

@@ -8,7 +8,6 @@ from app import notify_celery, statsd_client
from app.clients import ClientException from app.clients import ClientException
from app.clients.sms.firetext import get_firetext_responses from app.clients.sms.firetext import get_firetext_responses
from app.clients.sms.mmg import get_mmg_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 import notifications_dao
from app.dao.templates_dao import dao_get_template_by_id from app.dao.templates_dao import dao_get_template_by_id
from app.models import NOTIFICATION_PENDING from app.models import NOTIFICATION_PENDING
@@ -19,7 +18,6 @@ from app.notifications.notifications_ses_callback import (
sms_response_mapper = { sms_response_mapper = {
'MMG': get_mmg_responses, 'MMG': get_mmg_responses,
'Firetext': get_firetext_responses, 'Firetext': get_firetext_responses,
'Reach': get_reach_responses
} }

View File

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

View File

@@ -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` # 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") 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") 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") SES_STUB_URL = os.environ.get("SES_STUB_URL")
AWS_REGION = 'eu-west-1' AWS_REGION = 'eu-west-1'
@@ -485,7 +484,6 @@ class Test(Development):
MMG_URL = 'https://example.com/mmg' MMG_URL = 'https://example.com/mmg'
FIRETEXT_URL = 'https://example.com/firetext' FIRETEXT_URL = 'https://example.com/firetext'
REACH_URL = 'https://example.com/reach'
CBC_PROXY_ENABLED = True CBC_PROXY_ENABLED = True
DVLA_EMAIL_ADDRESSES = ['success@simulator.amazonses.com', 'success+2@simulator.amazonses.com'] DVLA_EMAIL_ADDRESSES = ['success@simulator.amazonses.com', 'success+2@simulator.amazonses.com']

View File

@@ -54,28 +54,6 @@ def process_firetext_response():
return jsonify(result='success'), 200 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): def validate_callback_data(data, fields, client_name):
errors = [] errors = []
for f in fields: for f in fields:

View File

@@ -27,9 +27,9 @@
'STATSD_HOST': None 'STATSD_HOST': None
}, },
'routes': { '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'], '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', 'api.staging-notify.works/notifications/sms/reach', 'notify-api-sms-receipts-staging.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', 'api.notifications.service.gov.uk/notifications/sms/reach', 'notify-api-sms-receipts-production.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-type': 'port',
'health-check-invocation-timeout': 3, 'health-check-invocation-timeout': 3,

View File

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

View File

@@ -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') 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( def test_process_sms_response_raises_client_exception_for_unknown_status(
sample_notification, sample_notification,
mocker, mocker,
@@ -43,9 +43,6 @@ def test_process_sms_response_raises_client_exception_for_unknown_status(
('3', '2', 'MMG', 'delivered', "Delivered to operator"), ('3', '2', 'MMG', 'delivered', "Delivered to operator"),
('4', '27', 'MMG', 'temporary-failure', "Absent Subscriber"), ('4', '27', 'MMG', 'temporary-failure', "Absent Subscriber"),
('5', '13', 'MMG', 'permanent-failure', "Sender id blacklisted"), ('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( def test_process_sms_client_response_updates_notification_status(
sample_notification, sample_notification,

View File

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

View File

@@ -17,13 +17,6 @@ def mmg_post(client, data):
headers=[('Content-Type', 'application/json')]) 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): def test_firetext_callback_should_not_need_auth(client, mocker):
mocker.patch('app.notifications.notifications_sms_callback.process_sms_client_response') 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' 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(): def test_validate_callback_data_returns_none_when_valid():
form = {'status': 'good', form = {'status': 'good',
'reference': 'send-sms-code'} 'reference': 'send-sms-code'}

View File

@@ -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'] json_resp = admin_request.get('provider_details.get_providers')['provider_details']
assert len(json_resp) > 0 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): def test_get_provider_details_by_id(client, notify_db):