From 5e3bb08860c4dd54ccc9c596eedf650e23c0b5d3 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 20 Jan 2017 13:17:53 +0000 Subject: [PATCH 1/2] The notification.to field is being formatted to be +447... This meant that the research mode task would never work. Also the way we mark data as "temporary-failure" with firetext is with first a pending status callback then a declined callback. This PR changes the research mode task to account for that situation. --- app/celery/research_mode_tasks.py | 20 +++++++---- tests/app/celery/test_research_mode_tasks.py | 38 ++++++++++++-------- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/app/celery/research_mode_tasks.py b/app/celery/research_mode_tasks.py index 7ca2b9cc4..d04f8fb7f 100644 --- a/app/celery/research_mode_tasks.py +++ b/app/celery/research_mode_tasks.py @@ -6,9 +6,9 @@ from requests import request, RequestException, HTTPError from app.models import SMS_TYPE -temp_fail = "07700900003" -perm_fail = "07700900002" -delivered = "07700900001" +temp_fail = "7700900003" +perm_fail = "7700900002" +delivered = "7700900001" delivered_email = "delivered@simulator.notify" perm_fail_email = "perm-fail@simulator.notify" @@ -23,6 +23,12 @@ def send_sms_response(provider, reference, to): else: headers = {"Content-type": "application/x-www-form-urlencoded"} body = firetext_callback(reference, to) + # to simulate getting a temporary_failure from firetext + # we need to send a pending status updated then a permanent-failure + if body['status'] == '2': # pending status + make_request(SMS_TYPE, provider, body, headers) + body = firetext_callback(reference, perm_fail) + make_request(SMS_TYPE, provider, body, headers) @@ -71,9 +77,9 @@ def mmg_callback(notification_id, to): status: 5 - rejected (perm failure) """ - if to == temp_fail: + if to.strip().endswith(temp_fail): status = "4" - elif to == perm_fail: + elif to.strip().endswith(perm_fail): status = "5" else: status = "3" @@ -90,8 +96,10 @@ def firetext_callback(notification_id, to): status: 0 - delivered status: 1 - perm failure """ - if to == perm_fail: + if to.strip().endswith(perm_fail): status = "1" + elif to.strip().endswith(temp_fail): + status = "2" else: status = "0" return { diff --git a/tests/app/celery/test_research_mode_tasks.py b/tests/app/celery/test_research_mode_tasks.py index 753fd3819..7810b7ddb 100644 --- a/tests/app/celery/test_research_mode_tasks.py +++ b/tests/app/celery/test_research_mode_tasks.py @@ -1,3 +1,4 @@ +import pytest from flask import json from app.celery.research_mode_tasks import ( send_sms_response, @@ -50,42 +51,49 @@ def test_make_ses_callback(notify_api, rmock): assert rmock.called -def test_delivered_mmg_callback(): - data = json.loads(mmg_callback("1234", "07700900001")) - assert data['MSISDN'] == "07700900001" +@pytest.mark.parametrize("phone_number", ["07700900001", "+447700900001", "7700900001", "+44 7700900001", + "+4407513453456"]) +def test_delivered_mmg_callback(phone_number): + data = json.loads(mmg_callback("1234", phone_number)) + assert data['MSISDN'] == phone_number assert data['status'] == "3" assert data['reference'] == "mmg_reference" assert data['CID'] == "1234" -def test_perm_failure_mmg_callback(): - data = json.loads(mmg_callback("1234", "07700900002")) - assert data['MSISDN'] == "07700900002" +@pytest.mark.parametrize("phone_number", ["07700900002", "+447700900002", "7700900002", "+44 7700900002"]) +def test_perm_failure_mmg_callback(phone_number): + data = json.loads(mmg_callback("1234", phone_number)) + assert data['MSISDN'] == phone_number assert data['status'] == "5" assert data['reference'] == "mmg_reference" assert data['CID'] == "1234" -def test_temp_failure_mmg_callback(): - data = json.loads(mmg_callback("1234", "07700900003")) - assert data['MSISDN'] == "07700900003" +@pytest.mark.parametrize("phone_number", ["07700900003", "+447700900003", "7700900003", "+44 7700900003"]) +def test_temp_failure_mmg_callback(phone_number): + data = json.loads(mmg_callback("1234", phone_number)) + assert data['MSISDN'] == phone_number assert data['status'] == "4" assert data['reference'] == "mmg_reference" assert data['CID'] == "1234" -def test_delivered_firetext_callback(): - assert firetext_callback('1234', '07700900001') == { - 'mobile': '07700900001', +@pytest.mark.parametrize("phone_number", ["07700900001", "+447700900001", "7700900001", "+44 7700900001", + "+4407513453456"]) +def test_delivered_firetext_callback(phone_number): + assert firetext_callback('1234', phone_number) == { + 'mobile': phone_number, 'status': '0', 'time': '2016-03-10 14:17:00', 'reference': '1234' } -def test_failure_firetext_callback(): - assert firetext_callback('1234', '07700900002') == { - 'mobile': '07700900002', +@pytest.mark.parametrize("phone_number", ["07700900002", "+447700900002", "7700900002", "+44 7700900002"]) +def test_failure_firetext_callback(phone_number): + assert firetext_callback('1234', phone_number) == { + 'mobile': phone_number, 'status': '1', 'time': '2016-03-10 14:17:00', 'reference': '1234' From fb27a2d4c94f7e251173f37a3b0f43d6db8890a6 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 23 Jan 2017 10:43:33 +0000 Subject: [PATCH 2/2] Changed the body of the request to use the right phone number, the phone number is not currently used in the callback logic but if it was then this may cause a problem. Changed the test to use a 077009 series phone number. --- app/celery/research_mode_tasks.py | 7 ++++++- tests/app/celery/test_research_mode_tasks.py | 13 ++++++++----- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/app/celery/research_mode_tasks.py b/app/celery/research_mode_tasks.py index d04f8fb7f..dd1f2545d 100644 --- a/app/celery/research_mode_tasks.py +++ b/app/celery/research_mode_tasks.py @@ -27,7 +27,12 @@ def send_sms_response(provider, reference, to): # we need to send a pending status updated then a permanent-failure if body['status'] == '2': # pending status make_request(SMS_TYPE, provider, body, headers) - body = firetext_callback(reference, perm_fail) + # 1 is a declined status for firetext, will result in a temp-failure + body = {'mobile': to, + 'status': "1", + 'time': '2016-03-10 14:17:00', + 'reference': reference + } make_request(SMS_TYPE, provider, body, headers) diff --git a/tests/app/celery/test_research_mode_tasks.py b/tests/app/celery/test_research_mode_tasks.py index 7810b7ddb..b56de3bb4 100644 --- a/tests/app/celery/test_research_mode_tasks.py +++ b/tests/app/celery/test_research_mode_tasks.py @@ -25,18 +25,21 @@ def test_make_mmg_callback(notify_api, rmock): assert json.loads(rmock.request_history[0].text)['MSISDN'] == '07700900001' -def test_make_firetext_callback(notify_api, rmock): +@pytest.mark.parametrize("phone_number", + ["07700900001", "07700900002", "07700900003", + "07700900236"]) +def test_make_firetext_callback(notify_api, rmock, phone_number): endpoint = "http://localhost:6011/notifications/sms/firetext" rmock.request( "POST", endpoint, json="some data", status_code=200) - send_sms_response("firetext", "1234", "07700900001") + send_sms_response("firetext", "1234", phone_number) assert rmock.called assert rmock.request_history[0].url == endpoint - assert 'mobile=07700900001' in rmock.request_history[0].text + assert 'mobile={}'.format(phone_number) in rmock.request_history[0].text def test_make_ses_callback(notify_api, rmock): @@ -52,7 +55,7 @@ def test_make_ses_callback(notify_api, rmock): @pytest.mark.parametrize("phone_number", ["07700900001", "+447700900001", "7700900001", "+44 7700900001", - "+4407513453456"]) + "+447700900236"]) def test_delivered_mmg_callback(phone_number): data = json.loads(mmg_callback("1234", phone_number)) assert data['MSISDN'] == phone_number @@ -80,7 +83,7 @@ def test_temp_failure_mmg_callback(phone_number): @pytest.mark.parametrize("phone_number", ["07700900001", "+447700900001", "7700900001", "+44 7700900001", - "+4407513453456"]) + "+447700900256"]) def test_delivered_firetext_callback(phone_number): assert firetext_callback('1234', phone_number) == { 'mobile': phone_number,