darkhelm/notifications-api
1
0
Fork 0
mirror of https://github.com/GSA/notifications-api.git synced 2025-12-18 05:31:48 -05:00
Code Issues Packages Projects Releases Wiki Activity

Change function and variable names for readability and consistency

Browse Source
This commit is contained in:
Pea Tyczynska
2020-06-01 11:45:35 +01:00
parent a4b942cf6c
commit c96142ba5e
8 changed files with 57 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
app/celery/process_sms_client_response_tasks.py
View File

@@ -24,7 +24,7 @@ sms_response_mapper = {
@notify_celery.task(bind=True, name="process-sms-client-response", max_retries=5, default_retry_delay=300)
@statsd(namespace="tasks")
def process_sms_client_response(self, status, provider_reference, client_name, code=None):
def process_sms_client_response(self, status, provider_reference, client_name, detailed_status_code=None):
# validate reference
try:
uuid.UUID(provider_reference, version=4)
@@ -36,7 +36,7 @@ def process_sms_client_response(self, status, provider_reference, client_name, c
# validate status
try:
notification_status, detailed_status = response_parser(status, code)
notification_status, detailed_status = response_parser(status, detailed_status_code)
current_app.logger.info(
f'{client_name} callback returned '
f'status of {notification_status}: {detailed_status} for reference: {provider_reference}'
@@ -53,17 +53,17 @@ def process_sms_client_response(self, status, provider_reference, client_name, c
notification_status=notification_status,
client_name=client_name,
provider_reference=provider_reference,
code=code
detailed_status_code=detailed_status_code
)
def _process_for_status(notification_status, client_name, provider_reference, code=None):
def _process_for_status(notification_status, client_name, provider_reference, detailed_status_code=None):
# record stats
notification = notifications_dao.update_notification_status_by_id(
notification_id=provider_reference,
status=notification_status,
sent_by=client_name.lower(),
code=code
detailed_status_code=detailed_status_code
)
if not notification:
return

12
app/clients/sms/firetext.py
View File

@@ -33,13 +33,15 @@ firetext_codes = {
}
def get_firetext_responses(status, code=None):
substatus = firetext_codes[code]['reason'] if firetext_codes.get(code, None) else None
return firetext_responses[status], substatus
def get_firetext_responses(status, detailed_status_code=None):
detailed_status = firetext_codes[detailed_status_code]['reason'] if firetext_codes.get(
detailed_status_code, None
) else None
return (firetext_responses[status], detailed_status)
def get_message_status_and_reason_from_firetext_code(code):
return firetext_codes[code]['status'], firetext_codes[code]['reason']
def get_message_status_and_reason_from_firetext_code(detailed_status_code):
return firetext_codes[detailed_status_code]['status'], firetext_codes[detailed_status_code]['reason']
class FiretextClientResponseException(SmsClientResponseException):

4
app/clients/sms/mmg.py
View File

@@ -45,8 +45,8 @@ mmg_response_map = {
}
def get_mmg_responses(status, substatus=None):
return (mmg_response_map[status]["status"], mmg_response_map[status]["substatus"].get(substatus, None))
def get_mmg_responses(status, detailed_status_code=None):
return (mmg_response_map[status]["status"], mmg_response_map[status]["substatus"].get(detailed_status_code, None))
class MMGClientResponseException(SmsClientResponseException):

18
app/dao/notifications_dao.py
View File

@@ -89,19 +89,19 @@ def dao_create_notification(notification):
db.session.add(notification)
def _decide_permanent_temporary_failure(status, notification, code=None):
def _decide_permanent_temporary_failure(status, notification, detailed_status_code=None):
# If we get failure status from Firetext, we want to know if this is temporary or permanent failure.
# So we check the failure code to learn that.
# If there is no failure code, or we do not recognise the failure code, we do the following:
# if notifitcation goes form status pending to status failure, we mark it as temporary failure;
# if notification goes straight to status failure, we mark it as permanent failure.
if status == NOTIFICATION_PERMANENT_FAILURE and code not in [None, '000']:
if status == NOTIFICATION_PERMANENT_FAILURE and detailed_status_code not in [None, '000']:
try:
status, reason = get_message_status_and_reason_from_firetext_code(code)
status, reason = get_message_status_and_reason_from_firetext_code(detailed_status_code)
current_app.logger.info(f'Updating notification id {notification.id} to status {status}, reason: {reason}')
return status
except KeyError:
current_app.logger.warning(f'Failure code {code} from Firetext not recognised')
current_app.logger.warning(f'Failure code {detailed_status_code} from Firetext not recognised')
# fallback option:
if notification.status == NOTIFICATION_PENDING and status == NOTIFICATION_PERMANENT_FAILURE:
status = NOTIFICATION_TEMPORARY_FAILURE
@@ -113,8 +113,10 @@ def country_records_delivery(phone_prefix):
return dlr and dlr.lower() == 'yes'
def _update_notification_status(notification, status, code=None):
status = _decide_permanent_temporary_failure(status=status, notification=notification, code=code)
def _update_notification_status(notification, status, detailed_status_code=None):
status = _decide_permanent_temporary_failure(
status=status, notification=notification, detailed_status_code=detailed_status_code
)
notification.status = status
dao_update_notification(notification)
return notification
@@ -122,7 +124,7 @@ def _update_notification_status(notification, status, code=None):
@statsd(namespace="dao")
@transactional
def update_notification_status_by_id(notification_id, status, sent_by=None, code=None):
def update_notification_status_by_id(notification_id, status, sent_by=None, detailed_status_code=None):
notification = Notification.query.with_for_update().filter(Notification.id == notification_id).first()
if not notification:
@@ -149,7 +151,7 @@ def update_notification_status_by_id(notification_id, status, sent_by=None, code
return _update_notification_status(
notification=notification,
status=status,
code=code
detailed_status_code=detailed_status_code
)

8
app/notifications/notifications_sms_callback.py
View File

@@ -22,12 +22,12 @@ def process_mmg_response():
raise InvalidRequest(errors, status_code=400)
status = str(data.get('status'))
substatus = str(data.get('substatus'))
detailed_status_code = str(data.get('substatus'))
provider_reference = data.get('CID')
process_sms_client_response.apply_async(
[status, provider_reference, client_name, substatus],
[status, provider_reference, client_name, detailed_status_code],
queue=QueueNames.SMS_CALLBACKS,
)
@@ -50,7 +50,7 @@ def process_firetext_response():
raise InvalidRequest(errors, status_code=400)
status = request.form.get('status')
code = request.form.get('code')
detailed_status_code = request.form.get('code')
provider_reference = request.form.get('reference')
safe_to_log = dict(request.form).copy()
@@ -60,7 +60,7 @@ def process_firetext_response():
)
process_sms_client_response.apply_async(
[status, provider_reference, client_name, code],
[status, provider_reference, client_name, detailed_status_code],
queue=QueueNames.SMS_CALLBACKS,
)

14
tests/app/clients/test_firetext.py
View File

@@ -8,25 +8,25 @@ import requests_mock
from app.clients.sms.firetext import get_firetext_responses, SmsClientResponseException, FiretextClientResponseException
@pytest.mark.parametrize('code, result', [
@pytest.mark.parametrize('detailed_status_code, result', [
(None, ('delivered', None)), ('000', ('delivered', None))
])
def test_get_firetext_responses_should_return_correct_details_for_delivery(code, result):
assert get_firetext_responses('0', code) == result
def test_get_firetext_responses_should_return_correct_details_for_delivery(detailed_status_code, result):
assert get_firetext_responses('0', detailed_status_code) == result
@pytest.mark.parametrize('code, result', [
@pytest.mark.parametrize('detailed_status_code, result', [
(None, ('permanent-failure', None)), ('401', ('permanent-failure', 'Message Rejected'))
])
def test_get_firetext_responses_should_return_correct_details_for_bounced(code, result):
assert get_firetext_responses('1', code) == result
def test_get_firetext_responses_should_return_correct_details_for_bounced(detailed_status_code, result):
assert get_firetext_responses('1', detailed_status_code) == result
def test_get_firetext_responses_should_return_correct_details_for_complaint():
assert get_firetext_responses('2') == ('pending', None)
def test_get_firetext_responses_should_be_none_if_unrecognised_status_code():
def test_get_firetext_responses_raises_KeyError_if_unrecognised_status_code():
with pytest.raises(KeyError) as e:
get_firetext_responses('99')
assert '99' in str(e.value)

20
tests/app/clients/test_mmg.py
View File

@@ -9,31 +9,31 @@ from app.clients.sms import SmsClientResponseException
from app.clients.sms.mmg import get_mmg_responses, MMGClientResponseException
@pytest.mark.parametrize('substatus, result', [
@pytest.mark.parametrize('detailed_status_code, result', [
(None, ('delivered', None)), ('5', ('delivered', 'Delivered to handset'))
])
def test_get_mmg_responses_should_return_correct_details_for_delivery(substatus, result):
assert get_mmg_responses('3', substatus) == result
def test_get_mmg_responses_should_return_correct_details_for_delivery(detailed_status_code, result):
assert get_mmg_responses('3', detailed_status_code) == result
@pytest.mark.parametrize('substatus, result', [
@pytest.mark.parametrize('detailed_status_code, result', [
(None, ('temporary-failure', None)), ('15', ('temporary-failure', 'Expired'))
])
def test_get_mmg_responses_should_return_correct_details_for_temporary_failure(substatus, result):
assert get_mmg_responses('4', substatus) == result
def test_get_mmg_responses_should_return_correct_details_for_temporary_failure(detailed_status_code, result):
assert get_mmg_responses('4', detailed_status_code) == result
@pytest.mark.parametrize('status, substatus, result', [
@pytest.mark.parametrize('status, detailed_status_code, result', [
('2', None, ('permanent-failure', None)),
('2', '12', ('permanent-failure', "Illegal equipment")),
('5', None, ('permanent-failure', None)),
('5', '20', ('permanent-failure', 'Rejected by anti-flooding mechanism'))
])
def test_get_mmg_responses_should_return_correct_details_for_bounced(status, substatus, result):
assert get_mmg_responses(status, substatus) == result
def test_get_mmg_responses_should_return_correct_details_for_bounced(status, detailed_status_code, result):
assert get_mmg_responses(status, detailed_status_code) == result
def test_get_mmg_responses_should_be_raise_if_unrecognised_status_code():
def test_get_mmg_responses_raises_KeyError_if_unrecognised_status_code():
with pytest.raises(KeyError) as e:
get_mmg_responses('99')
assert '99' in str(e.value)

24
tests/app/notifications/test_process_client_response.py
View File

@@ -35,7 +35,7 @@ def test_process_sms_response_raises_client_exception_for_unknown_status(
assert sample_notification.status == NOTIFICATION_TECHNICAL_FAILURE
@pytest.mark.parametrize('status, substatus, sms_provider, expected_notification_status, reason', [
@pytest.mark.parametrize('status, detailed_status_code, sms_provider, expected_notification_status, reason', [
('0', None, 'Firetext', 'delivered', None),
('1', '101', 'Firetext', 'permanent-failure', 'Unknown Subscriber'),
('2', '102', 'Firetext', 'pending', 'Absent Subscriber'),
@@ -48,7 +48,7 @@ def test_process_sms_client_response_updates_notification_status(
sample_notification,
mocker,
status,
substatus,
detailed_status_code,
sms_provider,
expected_notification_status,
reason
@@ -56,14 +56,14 @@ def test_process_sms_client_response_updates_notification_status(
mock_logger = mocker.patch('app.celery.tasks.current_app.logger.info')
sample_notification.status = 'sending'
process_sms_client_response(status, str(sample_notification.id), sms_provider, substatus)
process_sms_client_response(status, str(sample_notification.id), sms_provider, detailed_status_code)
message = f"{sms_provider} callback returned status of {expected_notification_status}: {reason} for reference: {sample_notification.id}" # noqa
mock_logger.assert_any_call(message)
assert sample_notification.status == expected_notification_status
@pytest.mark.parametrize('code, expected_notification_status, reason', [
@pytest.mark.parametrize('detailed_status_code, expected_notification_status, reason', [
('101', 'permanent-failure', 'Unknown Subscriber'),
('102', 'temporary-failure', 'Absent Subscriber'),
(None, 'temporary-failure', None),
@@ -72,7 +72,7 @@ def test_process_sms_client_response_updates_notification_status(
def test_process_sms_client_response_updates_notification_status_when_called_second_time(
sample_notification,
mocker,
code,
detailed_status_code,
expected_notification_status,
reason
):
@@ -80,29 +80,29 @@ def test_process_sms_client_response_updates_notification_status_when_called_sec
sample_notification.status = 'sending'
process_sms_client_response('2', str(sample_notification.id), 'Firetext')
process_sms_client_response('1', str(sample_notification.id), 'Firetext', code)
process_sms_client_response('1', str(sample_notification.id), 'Firetext', detailed_status_code)
if code and code != '000':
if detailed_status_code and detailed_status_code != '000':
message = f'Updating notification id {sample_notification.id} to status {expected_notification_status}, reason: {reason}' # noqa
mock_logger.assert_called_with(message)
assert sample_notification.status == expected_notification_status
@pytest.mark.parametrize('code', ['102', None, '000'])
def test_process_sms_client_response_updates_notification_status_to_pending_with_and_without_failire_code_present(
@pytest.mark.parametrize('detailed_status_code', ['102', None, '000'])
def test_process_sms_client_response_updates_notification_status_to_pending_with_and_without_failure_code_present(
sample_notification,
mocker,
code
detailed_status_code
):
sample_notification.status = 'sending'
process_sms_client_response('2', str(sample_notification.id), 'Firetext', code)
process_sms_client_response('2', str(sample_notification.id), 'Firetext', detailed_status_code)
assert sample_notification.status == 'pending'
def test_process_sms_client_response_updates_notification_status_when_code_unknown(
def test_process_sms_client_response_updates_notification_status_when_detailed_status_code_not_recognised(
sample_notification,
mocker,
):