Merge branch 'master' into refactor-letter-rates

tests/app/aws/test_s3.py

@@ -1,5 +1,6 @@
 from unittest.mock import call
 from datetime import datetime, timedelta
+import pytest
 
 from flask import current_app
 
@@ -9,7 +10,8 @@ from app.aws.s3 import (
     get_s3_bucket_objects,
     get_s3_file,
     filter_s3_bucket_objects_within_date_range,
-    remove_transformed_dvla_file
+    remove_transformed_dvla_file,
+    upload_letters_pdf
 )
 from tests.app.conftest import datetime_in_past
 
@@ -139,3 +141,21 @@ def test_get_s3_bucket_objects_does_not_return_outside_of_date_range(notify_api,
     filtered_items = filter_s3_bucket_objects_within_date_range(s3_objects_stub)
 
     assert len(filtered_items) == 0
+
+
+@pytest.mark.parametrize('crown_flag,expected_crown_text', [
+    (True, 'C'),
+    (False, 'N'),
+])
+@freeze_time("2017-12-04 15:00:00")
+def test_upload_letters_pdf_calls_utils_s3upload_with_correct_args(
+        notify_api, mocker, crown_flag, expected_crown_text):
+    s3_upload_mock = mocker.patch('app.aws.s3.utils_s3upload')
+    upload_letters_pdf(reference='foo', crown=crown_flag, filedata='some_data')
+
+    s3_upload_mock.assert_called_with(
+        filedata='some_data',
+        region='eu-west-1',
+        bucket_name='test-letters-pdf',
+        file_location='2017-12-04/NOTIFY.FOO.D.2.C.{}.20171204150000.PDF'.format(expected_crown_text)
+    )

tests/app/billing/test_billing.py

@@ -1,25 +1,25 @@
 from datetime import datetime, timedelta
 import json
 
+import pytest
+
 from app.billing.rest import _transform_billing_for_month
 from app.dao.monthly_billing_dao import (
     create_or_update_monthly_billing,
     get_monthly_billing_by_notification_type,
 )
 from app.models import SMS_TYPE, EMAIL_TYPE
+from app.dao.date_util import get_current_financial_year_start_year
+from app.dao.annual_billing_dao import dao_get_free_sms_fragment_limit_for_year
 from tests.app.db import (
     create_notification,
     create_rate,
-    create_monthly_billing_entry
+    create_monthly_billing_entry,
+    create_annual_billing,
 )
-
-from tests import create_authorization_header
-
-from app.dao.date_util import get_current_financial_year_start_year
-from app.dao.annual_billing_dao import dao_get_free_sms_fragment_limit_for_year
-from tests.app.db import create_annual_billing
 from app.billing.rest import update_free_sms_fragment_limit_data
 
+from tests import create_authorization_header
 
 APR_2016_MONTH_START = datetime(2016, 3, 31, 23, 00, 00)
 APR_2016_MONTH_END = datetime(2016, 4, 30, 22, 59, 59, 99999)
@@ -270,70 +270,62 @@ def test_create_update_free_sms_fragment_limit_invalid_schema(client, sample_ser
     assert 'JSON' in json_resp['message']
 
 
-def test_create_free_sms_fragment_limit_current_year(client, sample_service):
+def test_create_free_sms_fragment_limit_current_year_updates_future_years(admin_request, sample_service):
     current_year = get_current_financial_year_start_year()
-    data = {'free_sms_fragment_limit': 9999}
-    response = client.post('service/{}/billing/free-sms-fragment-limit'.format(sample_service.id),
-                           data=json.dumps(data),
-                           headers=[('Content-Type', 'application/json'), create_authorization_header()])
+    future_billing = create_annual_billing(sample_service.id, 1, current_year + 1)
 
-    response_get = client.get(
-        'service/{}/billing/free-sms-fragment-limit?financial_year_start={}'.format(sample_service.id, current_year),
-        headers=[('Content-Type', 'application/json'), create_authorization_header()])
+    admin_request.post(
+        'billing.create_or_update_free_sms_fragment_limit',
+        service_id=sample_service.id,
+        _data={'free_sms_fragment_limit': 9999},
+        _expected_status=201
+    )
 
-    json_resp = json.loads(response_get.get_data(as_text=True))
-    assert response.status_code == 201
-    assert response_get.status_code == 200
-    assert json_resp['financial_year_start'] == current_year
-    assert json_resp['free_sms_fragment_limit'] == 9999
+    current_billing = dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year)
+    assert future_billing.free_sms_fragment_limit == 9999
+    assert current_billing.financial_year_start == current_year
+    assert current_billing.free_sms_fragment_limit == 9999
 
 
-def test_create_free_sms_fragment_limit_past_year(client, sample_service):
-
-    data = {'financial_year_start': 2016, 'free_sms_fragment_limit': 9999}
-    response = client.post('service/{}/billing/free-sms-fragment-limit'.format(sample_service.id),
-                           data=json.dumps(data),
-                           headers=[('Content-Type', 'application/json'), create_authorization_header()])
-
-    response_get = client.get(
-        'service/{}/billing/free-sms-fragment-limit?financial_year_start=2016'.format(sample_service.id),
-        headers=[('Content-Type', 'application/json'), create_authorization_header()])
-
-    json_resp = json.loads(response_get.get_data(as_text=True))
-    assert response.status_code == 201
-    assert response_get.status_code == 200
-    assert json_resp['financial_year_start'] == 2016
-    assert json_resp['free_sms_fragment_limit'] == 9999
-
-
-def test_update_free_sms_fragment_limit(client, sample_service):
+@pytest.mark.parametrize('update_existing', [True, False])
+def test_create_or_update_free_sms_fragment_limit_past_year_doenst_update_other_years(
+    admin_request,
+    sample_service,
+    update_existing
+):
     current_year = get_current_financial_year_start_year()
+    create_annual_billing(sample_service.id, 1, current_year)
+    if update_existing:
+        create_annual_billing(sample_service.id, 1, current_year - 1)
 
-    annual_billing = dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year)
-    assert annual_billing.free_sms_fragment_limit == 250000
+    data = {'financial_year_start': current_year - 1, 'free_sms_fragment_limit': 9999}
+    admin_request.post(
+        'billing.create_or_update_free_sms_fragment_limit',
+        service_id=sample_service.id,
+        _data=data,
+        _expected_status=201)
 
-    data_new = {'financial_year_start': current_year, 'free_sms_fragment_limit': 9999}
-    response = client.post('service/{}/billing/free-sms-fragment-limit'.format(sample_service.id),
-                           data=json.dumps(data_new),
-                           headers=[('Content-Type', 'application/json'), create_authorization_header()])
-
-    response_get = client.get(
-        'service/{}/billing/free-sms-fragment-limit?financial_year_start={}'
-        .format(sample_service.id, current_year),
-        headers=[('Content-Type', 'application/json'), create_authorization_header()])
-
-    json_resp = json.loads(response_get.get_data(as_text=True))
-
-    assert response.status_code == 201
-    assert response_get.status_code == 200
-    assert json_resp['financial_year_start'] == current_year
-    assert json_resp['free_sms_fragment_limit'] == 9999
+    assert dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year - 1).free_sms_fragment_limit == 9999
+    assert dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year).free_sms_fragment_limit == 1
 
 
-def test_get_free_sms_fragment_limit_current_year(client, sample_service):
+def test_create_free_sms_fragment_limit_updates_existing_year(admin_request, sample_service):
+    current_year = get_current_financial_year_start_year()
+    annual_billing = create_annual_billing(sample_service.id, 1, current_year)
+
+    admin_request.post(
+        'billing.create_or_update_free_sms_fragment_limit',
+        service_id=sample_service.id,
+        _data={'financial_year_start': current_year, 'free_sms_fragment_limit': 2},
+        _expected_status=201)
+
+    assert annual_billing.free_sms_fragment_limit == 2
+
+
+def test_get_free_sms_fragment_limit_current_year_creates_new_row(client, sample_service):
 
     current_year = get_current_financial_year_start_year()
-    create_annual_billing(sample_service.id, free_sms_fragment_limit=9999, financial_year_start=current_year - 1)
+    create_annual_billing(sample_service.id, 9999, current_year - 1)
 
     response_get = client.get(
         'service/{}/billing/free-sms-fragment-limit'.format(sample_service.id),
@@ -342,7 +334,7 @@ def test_get_free_sms_fragment_limit_current_year(client, sample_service):
     json_resp = json.loads(response_get.get_data(as_text=True))
     assert response_get.status_code == 200
     assert json_resp['financial_year_start'] == get_current_financial_year_start_year()
-    assert json_resp['free_sms_fragment_limit'] == 250000
+    assert json_resp['free_sms_fragment_limit'] == 9999
 
 
 def test_get_free_sms_fragment_limit_past_year_not_exist(client, sample_service):
@@ -385,10 +377,9 @@ def test_get_free_sms_fragment_limit_future_year_not_exist(client, sample_servic
 
 def test_update_free_sms_fragment_limit_data(client, sample_service):
     current_year = get_current_financial_year_start_year()
-    annual_billing = dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year)
-    assert annual_billing.free_sms_fragment_limit == 250000
+    create_annual_billing(sample_service.id, free_sms_fragment_limit=250000, financial_year_start=current_year - 1)
 
-    update_free_sms_fragment_limit_data(sample_service.id, 9999)
+    update_free_sms_fragment_limit_data(sample_service.id, 9999, current_year)
 
     annual_billing = dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year)
     assert annual_billing.free_sms_fragment_limit == 9999

tests/app/celery/test_ftp_update_tasks.py

@@ -21,8 +21,9 @@ from app.celery.tasks import (
     update_letter_notifications_to_sent_to_dvla
 )
 
-from tests.app.db import create_notification
+from tests.app.db import create_notification, create_service_callback_api
 from tests.conftest import set_config
+from unittest.mock import call
 
 
 def test_update_job_to_sent_to_dvla(sample_letter_template, sample_letter_job):
@@ -97,11 +98,15 @@ def test_update_letter_notifications_statuses_persisted(notify_api, mocker, samp
                                       billable_units=0)
     failed_letter = create_notification(sample_letter_template, reference='ref-bar', status=NOTIFICATION_SENDING,
                                         billable_units=0)
-
+    create_service_callback_api(service=sample_letter_template.service, url="https://original_url.com")
     valid_file = '{}|Sent|1|Unsorted\n{}|Failed|2|Sorted'.format(
         sent_letter.reference, failed_letter.reference)
     mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=valid_file)
 
+    send_mock = mocker.patch(
+        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+    )
+
     update_letter_notifications_statuses(filename='foo.txt')
 
     assert sent_letter.status == NOTIFICATION_DELIVERED
@@ -111,6 +116,25 @@ def test_update_letter_notifications_statuses_persisted(notify_api, mocker, samp
     assert failed_letter.billable_units == 2
     assert failed_letter.updated_at
 
+    calls = [call([str(failed_letter.id)], queue="notify-internal-tasks"),
+             call([str(sent_letter.id)], queue="notify-internal-tasks")]
+    send_mock.assert_has_calls(calls, any_order=True)
+
+
+def test_update_letter_notifications_does_not_call_send_callback_if_no_db_entry(notify_api, mocker,
+                                                                                sample_letter_template):
+    sent_letter = create_notification(sample_letter_template, reference='ref-foo', status=NOTIFICATION_SENDING,
+                                      billable_units=0)
+    valid_file = '{}|Sent|1|Unsorted\n'.format(sent_letter.reference)
+    mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=valid_file)
+
+    send_mock = mocker.patch(
+        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+    )
+
+    update_letter_notifications_statuses(filename='foo.txt')
+    send_mock.assert_not_called()
+
 
 def test_update_letter_notifications_to_sent_to_dvla_updates_based_on_notification_references(
     client,
@@ -132,11 +156,15 @@ def test_update_letter_notifications_to_sent_to_dvla_updates_based_on_notificati
 
 def test_update_letter_notifications_to_error_updates_based_on_notification_references(
     client,
-    sample_letter_template
+    sample_letter_template,
+    mocker
 ):
+    send_mock = mocker.patch(
+        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+    )
     first = create_notification(sample_letter_template, reference='first ref')
     second = create_notification(sample_letter_template, reference='second ref')
-
+    create_service_callback_api(service=sample_letter_template.service, url="https://original_url.com")
     dt = datetime.utcnow()
     with freeze_time(dt):
         update_letter_notifications_to_error([first.reference])
@@ -146,3 +174,4 @@ def test_update_letter_notifications_to_error_updates_based_on_notification_refe
     assert first.sent_at is None
     assert first.updated_at == dt
     assert second.status == NOTIFICATION_CREATED
+    assert send_mock.called

tests/app/celery/test_process_ses_receipts_tasks.py

@@ -1,7 +1,7 @@
 import json
 from datetime import datetime
 
-from app.celery.callback_tasks import process_ses_results
+from app.celery.process_ses_receipts_tasks import process_ses_results
 
 from tests.app.db import create_notification
 
@@ -18,7 +18,7 @@ def test_process_ses_results(sample_email_template):
 
 
 def test_process_ses_results_does_not_retry_if_errors(notify_db, mocker):
-    mocked = mocker.patch('app.celery.callback_tasks.process_ses_results.retry')
+    mocked = mocker.patch('app.celery.process_ses_receipts_tasks.process_ses_results.retry')
     response = json.loads(ses_notification_callback())
     process_ses_results(response=response)
     assert mocked.call_count == 0
@@ -26,7 +26,7 @@ def test_process_ses_results_does_not_retry_if_errors(notify_db, mocker):
 
 def test_process_ses_results_retry_called(notify_db, mocker):
     mocker.patch("app.dao.notifications_dao.update_notification_status_by_reference", side_effect=Exception("EXPECTED"))
-    mocked = mocker.patch('app.celery.callback_tasks.process_ses_results.retry')
+    mocked = mocker.patch('app.celery.process_ses_receipts_tasks.process_ses_results.retry')
     response = json.loads(ses_notification_callback())
     process_ses_results(response=response)
     assert mocked.call_count != 0

tests/app/celery/test_service_callback_tasks.py

@@ -0,0 +1,185 @@
+import json
+from datetime import datetime
+
+import pytest
+import requests_mock
+
+from requests import RequestException
+
+from app import (DATETIME_FORMAT)
+
+from tests.app.conftest import (
+    sample_service as create_sample_service,
+    sample_template as create_sample_template,
+)
+from tests.app.db import (
+    create_notification,
+    create_user,
+    create_service_callback_api
+)
+from app.celery.service_callback_tasks import send_delivery_status_to_service
+
+
+@pytest.mark.parametrize("notification_type",
+                         ["email", "letter", "sms"])
+def test_send_delivery_status_to_service_post_https_request_to_service(notify_db,
+                                                                       notify_db_session,
+                                                                       notification_type):
+    user = create_user()
+    service = create_sample_service(notify_db, notify_db_session, user=user, restricted=True)
+
+    callback_api = create_service_callback_api(service=service, url="https://some.service.gov.uk/",
+                                               bearer_token="something_unique")
+    template = create_sample_template(
+        notify_db, notify_db_session, service=service, template_type=notification_type, subject_line='Hello'
+    )
+
+    datestr = datetime(2017, 6, 20)
+
+    notification = create_notification(template=template,
+                                       created_at=datestr,
+                                       updated_at=datestr,
+                                       sent_at=datestr,
+                                       status='sent'
+                                       )
+
+    with requests_mock.Mocker() as request_mock:
+        request_mock.post(callback_api.url,
+                          json={},
+                          status_code=200)
+        send_delivery_status_to_service(notification.id)
+
+    mock_data = {
+        "id": str(notification.id),
+        "reference": str(notification.client_reference),
+        "to": notification.to,
+        "status": notification.status,
+        "created_at": datestr.strftime(DATETIME_FORMAT),  # the time GOV.UK email sent the request
+        "updated_at": datestr.strftime(DATETIME_FORMAT),  # the last time the status was updated
+        "sent_at": datestr.strftime(DATETIME_FORMAT),  # the time the email was sent
+        "notification_type": notification_type
+    }
+
+    assert request_mock.call_count == 1
+    assert request_mock.request_history[0].url == callback_api.url
+    assert request_mock.request_history[0].method == 'POST'
+    assert request_mock.request_history[0].text == json.dumps(mock_data)
+    assert request_mock.request_history[0].headers["Content-type"] == "application/json"
+    assert request_mock.request_history[0].headers["Authorization"] == "Bearer {}".format(callback_api.bearer_token)
+
+
+@pytest.mark.parametrize("notification_type",
+                         ["email", "letter", "sms"])
+def test_send_delivery_status_to_service_does_not_sent_request_when_service_callback_api_does_not_exist(
+        notify_db, notify_db_session, mocker, notification_type):
+    service = create_sample_service(notify_db, notify_db_session, restricted=True)
+
+    template = create_sample_template(
+        notify_db, notify_db_session, service=service, template_type=notification_type, subject_line='Hello'
+    )
+    datestr = datetime(2017, 6, 20)
+
+    notification = create_notification(template=template,
+                                       created_at=datestr,
+                                       updated_at=datestr,
+                                       sent_at=datestr,
+                                       status='sent'
+                                       )
+    mocked = mocker.patch("requests.request")
+    send_delivery_status_to_service(notification.id)
+
+    mocked.call_count == 0
+
+
+@pytest.mark.parametrize("notification_type",
+                         ["email", "letter", "sms"])
+def test_send_delivery_status_to_service_retries_if_request_returns_500(notify_db,
+                                                                        notify_db_session,
+                                                                        mocker,
+                                                                        notification_type):
+    user = create_user()
+    service = create_sample_service(notify_db, notify_db_session, user=user, restricted=True)
+
+    template = create_sample_template(
+        notify_db, notify_db_session, service=service, template_type=notification_type, subject_line='Hello'
+    )
+    callback_api = create_service_callback_api(service=service, url="https://some.service.gov.uk/",
+                                               bearer_token="something_unique")
+    datestr = datetime(2017, 6, 20)
+    notification = create_notification(template=template,
+                                       created_at=datestr,
+                                       updated_at=datestr,
+                                       sent_at=datestr,
+                                       status='sent'
+                                       )
+    mocked = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.retry')
+    with requests_mock.Mocker() as request_mock:
+        request_mock.post(callback_api.url,
+                          json={},
+                          status_code=500)
+        send_delivery_status_to_service(notification.id)
+
+    assert mocked.call_count == 1
+    assert mocked.call_args[1]['queue'] == 'retry-tasks'
+
+
+@pytest.mark.parametrize("notification_type",
+                         ["email", "letter", "sms"])
+def test_send_delivery_status_to_service_retries_if_request_throws_unknown(notify_db,
+                                                                           notify_db_session,
+                                                                           mocker,
+                                                                           notification_type):
+    user = create_user()
+    service = create_sample_service(notify_db, notify_db_session, user=user, restricted=True)
+
+    template = create_sample_template(
+        notify_db, notify_db_session, service=service, template_type=notification_type, subject_line='Hello'
+    )
+    create_service_callback_api(service=service, url="https://some.service.gov.uk/",
+                                bearer_token="something_unique")
+    datestr = datetime(2017, 6, 20)
+    notification = create_notification(template=template,
+                                       created_at=datestr,
+                                       updated_at=datestr,
+                                       sent_at=datestr,
+                                       status='sent'
+                                       )
+
+    mocked = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.retry')
+    mocker.patch("app.celery.tasks.request", side_effect=RequestException())
+
+    send_delivery_status_to_service(notification.id)
+
+    assert mocked.call_count == 1
+    assert mocked.call_args[1]['queue'] == 'retry-tasks'
+
+
+@pytest.mark.parametrize("notification_type",
+                         ["email", "letter", "sms"])
+def test_send_delivery_status_to_service_does_not_retries_if_request_returns_404(notify_db,
+                                                                                 notify_db_session,
+                                                                                 mocker,
+                                                                                 notification_type):
+    user = create_user()
+    service = create_sample_service(notify_db, notify_db_session, user=user, restricted=True)
+
+    template = create_sample_template(
+        notify_db, notify_db_session, service=service, template_type=notification_type, subject_line='Hello'
+    )
+    callback_api = create_service_callback_api(service=service, url="https://some.service.gov.uk/",
+                                               bearer_token="something_unique")
+    datestr = datetime(2017, 6, 20)
+    notification = create_notification(template=template,
+                                       created_at=datestr,
+                                       updated_at=datestr,
+                                       sent_at=datestr,
+                                       status='sent'
+                                       )
+    mocked = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.retry')
+    with requests_mock.Mocker() as request_mock:
+        request_mock.post(callback_api.url,
+                          json={},
+                          status_code=404)
+        send_delivery_status_to_service(notification.id)
+
+    mocked.call_count == 0

tests/app/celery/test_tasks.py

@@ -29,7 +29,8 @@ from app.celery.tasks import (
     process_incomplete_jobs,
     get_template_class,
     s3,
-    send_inbound_sms_to_service)
+    send_inbound_sms_to_service,
+)
 from app.config import QueueNames
 from app.dao import jobs_dao, services_dao
 from app.models import (
@@ -64,7 +65,7 @@ from tests.app.db import (
     create_template,
     create_user,
     create_reply_to_email,
-    create_service_with_defined_sms_sender
+    create_service_with_defined_sms_sender,
 )
 
 

tests/app/conftest.py

@@ -150,7 +150,6 @@ def sample_service(
     email_from=None,
     permissions=None,
     research_mode=None,
-    free_sms_fragment_limit=250000
 ):
     if user is None:
         user = create_user()
@@ -162,8 +161,7 @@ def sample_service(
         'message_limit': limit,
         'restricted': restricted,
         'email_from': email_from,
-        'created_by': user,
-        'free_sms_fragment_limit': free_sms_fragment_limit
+        'created_by': user
     }
     service = Service.query.filter_by(name=service_name).first()
     if not service:
@@ -1002,7 +1000,11 @@ def notify_service(notify_db, notify_db_session):
             created_by=user,
             prefix_sms=False,
         )
-        dao_create_service(service=service, service_id=current_app.config['NOTIFY_SERVICE_ID'], user=user)
+        dao_create_service(
+            service=service,
+            service_id=current_app.config['NOTIFY_SERVICE_ID'],
+            user=user
+        )
 
         data = {
             'service': service,

tests/app/dao/notification_dao/test_notification_dao.py

@@ -29,7 +29,8 @@ from app.dao.notifications_dao import (
     is_delivery_slow_for_provider,
     set_scheduled_notification_to_processed,
     update_notification_status_by_id,
-    update_notification_status_by_reference
+    update_notification_status_by_reference,
+    dao_get_notifications_by_references
 )
 from app.dao.services_dao import dao_update_service
 from app.models import (
@@ -1989,3 +1990,14 @@ def test_dao_update_notifications_by_reference_returns_zero_when_no_notification
                                                                        "billable_units": 2}
                                                           )
     assert updated_count == 0
+
+
+def test_dao_get_notifications_by_reference(sample_template):
+    create_notification(template=sample_template, reference='noref')
+    notification_1 = create_notification(template=sample_template, reference='ref')
+    notification_2 = create_notification(template=sample_template, reference='ref')
+
+    notifications = dao_get_notifications_by_references(['ref'])
+    assert len(notifications) == 2
+    assert notifications[0].id in [notification_1.id, notification_2.id]
+    assert notifications[1].id in [notification_1.id, notification_2.id]

tests/app/dao/test_annual_billing_dao.py

@@ -1,23 +1,14 @@
+
 from app.dao.date_util import get_current_financial_year_start_year
 
 from app.dao.annual_billing_dao import (
     dao_create_or_update_annual_billing_for_year,
     dao_get_free_sms_fragment_limit_for_year,
-    dao_update_annual_billing_for_current_and_future_years,
+    dao_update_annual_billing_for_future_years,
 )
 from tests.app.db import create_annual_billing
 
 
-def test_get_sample_service_has_default_free_sms_fragment_limit(notify_db_session, sample_service):
-
-    # when sample_service was created, it automatically create an entry in the annual_billing table
-    free_limit = dao_get_free_sms_fragment_limit_for_year(sample_service.id, get_current_financial_year_start_year())
-
-    assert free_limit.free_sms_fragment_limit == 250000
-    assert free_limit.financial_year_start == get_current_financial_year_start_year()
-    assert free_limit.service_id == sample_service.id
-
-
 def test_dao_update_free_sms_fragment_limit(notify_db_session, sample_service):
     new_limit = 9999
     year = get_current_financial_year_start_year()
@@ -27,16 +18,7 @@ def test_dao_update_free_sms_fragment_limit(notify_db_session, sample_service):
     assert new_free_limit.free_sms_fragment_limit == new_limit
 
 
-def test_create_annual_billing_not_specify_year(notify_db_session, sample_service):
-
-    dao_create_or_update_annual_billing_for_year(sample_service.id, 9999)
-
-    free_limit = dao_get_free_sms_fragment_limit_for_year(sample_service.id)
-
-    assert free_limit.free_sms_fragment_limit == 9999
-
-
-def test_create_annual_billing_specify_year(notify_db_session, sample_service):
+def test_create_annual_billing(sample_service):
 
     dao_create_or_update_annual_billing_for_year(sample_service.id, 9999, 2016)
 
@@ -45,18 +27,17 @@ def test_create_annual_billing_specify_year(notify_db_session, sample_service):
     assert free_limit.free_sms_fragment_limit == 9999
 
 
-def test_dao_update_annual_billing_for_current_and_future_years(notify_db_session, sample_service):
+def test_dao_update_annual_billing_for_future_years(notify_db_session, sample_service):
     current_year = get_current_financial_year_start_year()
-    limits = [240000, 250000, 260000, 270000]
+    limits = [1, 2, 3, 4]
     create_annual_billing(sample_service.id, limits[0], current_year - 1)
     create_annual_billing(sample_service.id, limits[2], current_year + 1)
     create_annual_billing(sample_service.id, limits[3], current_year + 2)
 
-    dao_update_annual_billing_for_current_and_future_years(sample_service.id, 9999, current_year)
+    dao_update_annual_billing_for_future_years(sample_service.id, 9999, current_year)
 
-    free_limit = dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year - 1)
-    assert free_limit.free_sms_fragment_limit == 240000
-
-    for year in range(current_year, current_year + 3):
-        free_limit = dao_get_free_sms_fragment_limit_for_year(sample_service.id, year)
-        assert free_limit.free_sms_fragment_limit == 9999
+    assert dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year - 1).free_sms_fragment_limit == 1
+    # current year is not created
+    assert dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year) is None
+    assert dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year + 1).free_sms_fragment_limit == 9999
+    assert dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year + 2).free_sms_fragment_limit == 9999

tests/app/dao/test_services_dao.py

@@ -92,7 +92,7 @@ def test_create_service(sample_user):
     dao_create_service(service, sample_user)
     assert Service.query.count() == 1
 
-    service_db = Service.query.first()
+    service_db = Service.query.one()
     assert service_db.name == "service_name"
     assert service_db.id == service.id
     assert service_db.branding == BRANDING_GOVUK
@@ -102,7 +102,6 @@ def test_create_service(sample_user):
     assert service_db.prefix_sms is True
     assert service.active is True
     assert sample_user in service_db.users
-    assert service_db.free_sms_fragment_limit == 250000
     assert service_db.organisation_type == 'central'
     assert service_db.crown is True
 

tests/app/notifications/rest/test_callbacks.py

@@ -10,6 +10,7 @@ from app.dao.notifications_dao import (
     get_notification_by_id
 )
 from tests.app.conftest import sample_notification as create_sample_notification
+from tests.app.db import create_service_callback_api
 
 
 def firetext_post(client, data):
@@ -18,7 +19,7 @@ def firetext_post(client, data):
         data=data,
         headers=[
             ('Content-Type', 'application/x-www-form-urlencoded'),
-            ('X-Forwarded-For', '203.0.113.195, 70.41.3.18, 150.172.238.178')
+            ('X-Forwarded-For', '203.0.113.195, 70.41.3.18, 150.172.238.178')  # fake IPs
         ])
 
 
@@ -28,7 +29,7 @@ def mmg_post(client, data):
         data=data,
         headers=[
             ('Content-Type', 'application/json'),
-            ('X-Forwarded-For', '203.0.113.195, 70.41.3.18, 150.172.238.178')
+            ('X-Forwarded-For', '203.0.113.195, 70.41.3.18, 150.172.238.178')  # fake IPs
         ])
 
 
@@ -178,7 +179,9 @@ def test_firetext_callback_should_update_notification_status(
         notify_db, notify_db_session, client, sample_email_template, mocker
 ):
     mocker.patch('app.statsd_client.incr')
-
+    send_mock = mocker.patch(
+        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+    )
     notification = create_sample_notification(
         notify_db,
         notify_db_session,
@@ -202,13 +205,16 @@ def test_firetext_callback_should_update_notification_status(
     updated = get_notification_by_id(notification.id)
     assert updated.status == 'delivered'
     assert get_notification_by_id(notification.id).status == 'delivered'
+    assert send_mock.called_once_with([notification.id], queue="notify-internal-tasks")
 
 
 def test_firetext_callback_should_update_notification_status_failed(
         notify_db, notify_db_session, client, sample_template, mocker
 ):
     mocker.patch('app.statsd_client.incr')
-
+    mocker.patch(
+        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+    )
     notification = create_sample_notification(
         notify_db,
         notify_db_session,
@@ -235,6 +241,9 @@ def test_firetext_callback_should_update_notification_status_failed(
 
 def test_firetext_callback_should_update_notification_status_pending(client, notify_db, notify_db_session, mocker):
     mocker.patch('app.statsd_client.incr')
+    mocker.patch(
+        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+    )
     notification = create_sample_notification(
         notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow()
     )
@@ -265,8 +274,11 @@ def test_process_mmg_response_return_200_when_cid_is_send_sms_code(client):
 
 
 def test_process_mmg_response_returns_200_when_cid_is_valid_notification_id(
-        notify_db, notify_db_session, client
+        notify_db, notify_db_session, client, mocker
 ):
+    mocker.patch(
+        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+    )
     notification = create_sample_notification(
         notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow()
     )
@@ -286,8 +298,11 @@ def test_process_mmg_response_returns_200_when_cid_is_valid_notification_id(
 
 
 def test_process_mmg_response_status_5_updates_notification_with_permanently_failed(
-    notify_db, notify_db_session, client
+    notify_db, notify_db_session, client, mocker
 ):
+    mocker.patch(
+        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+    )
     notification = create_sample_notification(
         notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow()
     )
@@ -306,8 +321,11 @@ def test_process_mmg_response_status_5_updates_notification_with_permanently_fai
 
 
 def test_process_mmg_response_status_2_updates_notification_with_permanently_failed(
-    notify_db, notify_db_session, client
+    notify_db, notify_db_session, client, mocker
 ):
+    mocker.patch(
+        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+    )
     notification = create_sample_notification(
         notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow()
     )
@@ -325,8 +343,11 @@ def test_process_mmg_response_status_2_updates_notification_with_permanently_fai
 
 
 def test_process_mmg_response_status_4_updates_notification_with_temporary_failed(
-        notify_db, notify_db_session, client
+        notify_db, notify_db_session, client, mocker
 ):
+    mocker.patch(
+        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+    )
     notification = create_sample_notification(
         notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow()
     )
@@ -345,8 +366,11 @@ def test_process_mmg_response_status_4_updates_notification_with_temporary_faile
 
 
 def test_process_mmg_response_unknown_status_updates_notification_with_failed(
-        notify_db, notify_db_session, client
+        notify_db, notify_db_session, client, mocker
 ):
+    send_mock = mocker.patch(
+        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+    )
     notification = create_sample_notification(
         notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow()
     )
@@ -354,13 +378,14 @@ def test_process_mmg_response_unknown_status_updates_notification_with_failed(
                        "CID": str(notification.id),
                        "MSISDN": "447777349060",
                        "status": 10})
-
+    create_service_callback_api(service=notification.service, url="https://original_url.com")
     response = mmg_post(client, data)
     assert response.status_code == 200
     json_data = json.loads(response.data)
     assert json_data['result'] == 'success'
     assert json_data['message'] == 'MMG callback succeeded. reference {} updated'.format(notification.id)
     assert get_notification_by_id(notification.id).status == 'failed'
+    assert send_mock.called
 
 
 def test_process_mmg_response_returns_400_for_malformed_data(client):
@@ -392,6 +417,9 @@ def test_process_mmg_response_records_statsd(notify_db, notify_db_session, clien
 
         mocker.patch('app.statsd_client.incr')
         mocker.patch('app.statsd_client.timing_with_dates')
+        mocker.patch(
+            'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+        )
         notification = create_sample_notification(
             notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow()
         )
@@ -415,6 +443,9 @@ def test_firetext_callback_should_record_statsd(client, notify_db, notify_db_ses
 
         mocker.patch('app.statsd_client.incr')
         mocker.patch('app.statsd_client.timing_with_dates')
+        mocker.patch(
+            'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+        )
         notification = create_sample_notification(
             notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow()
         )

tests/app/notifications/test_notifications_ses_callback.py

@@ -10,6 +10,7 @@ from app.notifications.notifications_ses_callback import process_ses_response, r
 from app.celery.research_mode_tasks import ses_hard_bounce_callback, ses_soft_bounce_callback, ses_notification_callback
 
 from tests.app.conftest import sample_notification as create_sample_notification
+from tests.app.db import create_service_callback_api
 
 
 def test_ses_callback_should_update_notification_status(
@@ -24,7 +25,42 @@ def test_ses_callback_should_update_notification_status(
         stats_mock = mocker.patch(
             'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
         )
+        send_mock = mocker.patch(
+            'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+        )
+        notification = create_sample_notification(
+            notify_db,
+            notify_db_session,
+            template=sample_email_template,
+            reference='ref',
+            status='sending',
+            sent_at=datetime.utcnow()
+        )
+        create_service_callback_api(service=sample_email_template.service, url="https://original_url.com")
+        assert get_notification_by_id(notification.id).status == 'sending'
 
+        errors = process_ses_response(ses_notification_callback(reference='ref'))
+        assert errors is None
+        assert get_notification_by_id(notification.id).status == 'delivered'
+        statsd_client.timing_with_dates.assert_any_call(
+            "callback.ses.elapsed-time", datetime.utcnow(), notification.sent_at
+        )
+        statsd_client.incr.assert_any_call("callback.ses.delivered")
+        stats_mock.assert_called_once_with(notification)
+        send_mock.assert_called_once_with([str(notification.id)], queue="notify-internal-tasks")
+
+
+def test_ses_callback_does_not_call_send_delivery_status_if_no_db_entry(
+        client,
+        notify_db,
+        notify_db_session,
+        sample_email_template,
+        mocker):
+    with freeze_time('2001-01-01T12:00:00'):
+
+        send_mock = mocker.patch(
+            'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+        )
         notification = create_sample_notification(
             notify_db,
             notify_db_session,
@@ -39,11 +75,8 @@ def test_ses_callback_should_update_notification_status(
         errors = process_ses_response(ses_notification_callback(reference='ref'))
         assert errors is None
         assert get_notification_by_id(notification.id).status == 'delivered'
-        statsd_client.timing_with_dates.assert_any_call(
-            "callback.ses.elapsed-time", datetime.utcnow(), notification.sent_at
-        )
-        statsd_client.incr.assert_any_call("callback.ses.delivered")
-        stats_mock.assert_called_once_with(notification)
+
+        send_mock.assert_not_called()
 
 
 def test_ses_callback_should_update_multiple_notification_status_sent(
@@ -56,7 +89,9 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
     stats_mock = mocker.patch(
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
-
+    send_mock = mocker.patch(
+        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+    )
     notification1 = create_sample_notification(
         notify_db,
         notify_db_session,
@@ -80,7 +115,7 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
         reference='ref3',
         sent_at=datetime.utcnow(),
         status='sending')
-
+    create_service_callback_api(service=sample_email_template.service, url="https://original_url.com")
     assert process_ses_response(ses_notification_callback(reference='ref1')) is None
     assert process_ses_response(ses_notification_callback(reference='ref2')) is None
     assert process_ses_response(ses_notification_callback(reference='ref3')) is None
@@ -90,6 +125,7 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
         call(notification2),
         call(notification3)
     ])
+    assert send_mock.called
 
 
 def test_ses_callback_should_set_status_to_temporary_failure(client,
@@ -101,7 +137,9 @@ def test_ses_callback_should_set_status_to_temporary_failure(client,
     stats_mock = mocker.patch(
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
-
+    send_mock = mocker.patch(
+        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+    )
     notification = create_sample_notification(
         notify_db,
         notify_db_session,
@@ -110,9 +148,11 @@ def test_ses_callback_should_set_status_to_temporary_failure(client,
         status='sending',
         sent_at=datetime.utcnow()
     )
+    create_service_callback_api(service=notification.service, url="https://original_url.com")
     assert get_notification_by_id(notification.id).status == 'sending'
     assert process_ses_response(ses_soft_bounce_callback(reference='ref')) is None
     assert get_notification_by_id(notification.id).status == 'temporary-failure'
+    assert send_mock.called
     stats_mock.assert_called_once_with(notification)
 
 
@@ -146,7 +186,9 @@ def test_ses_callback_should_set_status_to_permanent_failure(client,
     stats_mock = mocker.patch(
         'app.notifications.notifications_ses_callback.create_outcome_notification_statistic_tasks'
     )
-
+    send_mock = mocker.patch(
+        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+    )
     notification = create_sample_notification(
         notify_db,
         notify_db_session,
@@ -155,10 +197,12 @@ def test_ses_callback_should_set_status_to_permanent_failure(client,
         status='sending',
         sent_at=datetime.utcnow()
     )
+    create_service_callback_api(service=sample_email_template.service, url="https://original_url.com")
 
     assert get_notification_by_id(notification.id).status == 'sending'
     assert process_ses_response(ses_hard_bounce_callback(reference='ref')) is None
     assert get_notification_by_id(notification.id).status == 'permanent-failure'
+    assert send_mock.called
     stats_mock.assert_called_once_with(notification)
 
 

tests/app/notifications/test_process_client_response.py

@@ -4,6 +4,7 @@ from app.notifications.process_client_response import (
     validate_callback_data,
     process_sms_client_response
 )
+from tests.app.db import create_service_callback_api
 
 
 def test_validate_callback_data_returns_none_when_valid():
@@ -51,15 +52,32 @@ def test_outcome_statistics_called_for_successful_callback(sample_notification,
         'app.notifications.process_client_response.notifications_dao.update_notification_status_by_id',
         return_value=sample_notification
     )
-
+    send_mock = mocker.patch(
+        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+    )
+    create_service_callback_api(service=sample_notification.service, url="https://original_url.com")
     reference = str(uuid.uuid4())
 
     success, error = process_sms_client_response(status='3', reference=reference, client_name='MMG')
     assert success == "MMG callback succeeded. reference {} updated".format(str(reference))
     assert error is None
+    send_mock.assert_called_once_with([str(sample_notification.id)], queue="notify-internal-tasks")
     stats_mock.assert_called_once_with(sample_notification)
 
 
+def test_sms_resonse_does_not_call_send_callback_if_no_db_entry(sample_notification, mocker):
+    mocker.patch(
+        'app.notifications.process_client_response.notifications_dao.update_notification_status_by_id',
+        return_value=sample_notification
+    )
+    send_mock = mocker.patch(
+        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+    )
+    reference = str(uuid.uuid4())
+    process_sms_client_response(status='3', reference=reference, client_name='MMG')
+    send_mock.assert_not_called()
+
+
 def test_process_sms_response_return_success_for_send_sms_code_reference(mocker):
     stats_mock = mocker.patch('app.notifications.process_client_response.create_outcome_notification_statistic_tasks')
 

tests/app/service/test_rest.py

@@ -38,7 +38,6 @@ from tests.app.db import (
     create_service_with_defined_sms_sender
 )
 from tests.app.db import create_user
-from app.dao.date_util import get_current_financial_year_start_year
 
 
 def test_get_service_list(client, service_factory):
@@ -135,16 +134,6 @@ def test_get_service_by_id(admin_request, sample_service):
     assert json_resp['data']['prefix_sms'] is True
 
 
-def test_get_service_by_id_returns_free_sms_limit(admin_request, sample_service):
-    json_resp = admin_request.get('service.get_service_by_id', service_id=sample_service.id)
-    assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT']
-
-
-def test_get_detailed_service_by_id_returns_free_sms_limit(admin_request, sample_service):
-    json_resp = admin_request.get('service.get_service_by_id', service_id=sample_service.id, detailed=True)
-    assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT']
-
-
 @pytest.mark.parametrize('detailed', [True, False])
 def test_get_service_by_id_returns_organisation_type(admin_request, sample_service, detailed):
     json_resp = admin_request.get('service.get_service_by_id', service_id=sample_service.id, detailed=detailed)
@@ -228,7 +217,8 @@ def test_create_service(client, sample_user):
         'restricted': False,
         'active': False,
         'email_from': 'created.service',
-        'created_by': str(sample_user.id)}
+        'created_by': str(sample_user.id)
+    }
     auth_header = create_authorization_header()
     headers = [('Content-Type', 'application/json'), auth_header]
     resp = client.post(
@@ -243,8 +233,6 @@ def test_create_service(client, sample_user):
     assert not json_resp['data']['research_mode']
     assert json_resp['data']['dvla_organisation'] == '001'
     assert json_resp['data']['sms_sender'] == current_app.config['FROM_NUMBER']
-    # TODO: Remove this after the new data is used
-    assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT']
 
     service_db = Service.query.get(json_resp['data']['id'])
     assert service_db.name == 'created service'
@@ -288,66 +276,6 @@ def test_should_not_create_service_with_missing_user_id_field(notify_api, fake_u
             assert 'Missing data for required field.' in json_resp['message']['user_id']
 
 
-def test_create_service_free_sms_fragment_limit_is_optional(client, sample_user):
-    data1 = {
-        'name': 'service 1',
-        'user_id': str(sample_user.id),
-        'message_limit': 1000,
-        'restricted': False,
-        'active': False,
-        'email_from': 'sample_user.email1',
-        'created_by': str(sample_user.id),
-        'free_sms_fragment_limit': 9999
-    }
-
-    auth_header = create_authorization_header()
-    headers = [('Content-Type', 'application/json'), auth_header]
-    resp = client.post(
-        '/service',
-        data=json.dumps(data1),
-        headers=headers)
-    json_resp = json.loads(resp.get_data(as_text=True))
-    assert resp.status_code == 201
-
-    # Test data from the new annual billing table
-    service_id = json_resp['data']['id']
-    annual_billing = client.get('service/{}/billing/free-sms-fragment-limit?financial_year_start={}'
-                                .format(service_id, get_current_financial_year_start_year()),
-                                headers=[('Content-Type', 'application/json'), create_authorization_header()])
-    json_resp = json.loads(annual_billing.get_data(as_text=True))
-    assert json_resp['free_sms_fragment_limit'] == 9999
-    # TODO: Remove this after the new data is used
-    assert json_resp['free_sms_fragment_limit'] == 9999
-
-    data2 = {
-        'name': 'service 2',
-        'user_id': str(sample_user.id),
-        'message_limit': 1000,
-        'restricted': False,
-        'active': False,
-        'email_from': 'sample_user.email2',
-        'created_by': str(sample_user.id),
-    }
-
-    auth_header = create_authorization_header()
-    headers = [('Content-Type', 'application/json'), auth_header]
-    resp = client.post(
-        '/service',
-        data=json.dumps(data2),
-        headers=headers)
-    json_resp = json.loads(resp.get_data(as_text=True))
-    assert resp.status_code == 201
-    # Test data from the new annual billing table
-    service_id = json_resp['data']['id']
-    annual_billing = client.get('service/{}/billing/free-sms-fragment-limit?financial_year_start={}'
-                                .format(service_id, get_current_financial_year_start_year()),
-                                headers=[('Content-Type', 'application/json'), create_authorization_header()])
-    json_resp = json.loads(annual_billing.get_data(as_text=True))
-    assert json_resp['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT']
-    # TODO: Remove this after the new data is used
-    assert json_resp['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT']
-
-
 def test_should_error_if_created_by_missing(notify_api, sample_user):
     with notify_api.test_request_context():
         with notify_api.test_client() as client:
@@ -430,7 +358,8 @@ def test_should_not_create_service_with_duplicate_name(notify_api,
                 'restricted': False,
                 'active': False,
                 'email_from': 'sample.service2',
-                'created_by': str(sample_user.id)}
+                'created_by': str(sample_user.id)
+            }
             auth_header = create_authorization_header()
             headers = [('Content-Type', 'application/json'), auth_header]
             resp = client.post(
@@ -456,7 +385,8 @@ def test_create_service_should_throw_duplicate_key_constraint_for_existing_email
                 'restricted': False,
                 'active': False,
                 'email_from': 'first.service',
-                'created_by': str(sample_user.id)}
+                'created_by': str(sample_user.id)
+            }
             auth_header = create_authorization_header()
             headers = [('Content-Type', 'application/json'), auth_header]
             resp = client.post(
@@ -603,37 +533,6 @@ def test_update_service_flags_will_remove_service_permissions(client, notify_db,
     assert set([p.permission for p in permissions]) == set([SMS_TYPE, EMAIL_TYPE])
 
 
-# TODO: Remove after new table is created and verified
-def test_update_service_free_sms_fragment_limit(client, notify_db, sample_service):
-    org = Organisation(colour='#000000', logo='justice-league.png', name='Justice League')
-    notify_db.session.add(org)
-    notify_db.session.commit()
-
-    auth_header = create_authorization_header()
-    resp = client.get(
-        '/service/{}'.format(sample_service.id),
-        headers=[auth_header]
-    )
-    json_resp = json.loads(resp.get_data(as_text=True))
-    assert resp.status_code == 200
-    assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT']
-
-    data = {
-        'free_sms_fragment_limit': 9999
-    }
-
-    auth_header = create_authorization_header()
-
-    resp = client.post(
-        '/service/{}'.format(sample_service.id),
-        data=json.dumps(data),
-        headers=[('Content-Type', 'application/json'), auth_header]
-    )
-    result = json.loads(resp.get_data(as_text=True))
-    assert resp.status_code == 200
-    assert result['data']['free_sms_fragment_limit'] == 9999
-
-
 def test_update_permissions_will_override_permission_flags(client, service_with_no_permissions):
     auth_header = create_authorization_header()
 
@@ -916,7 +815,8 @@ def test_default_permissions_are_added_for_user_service(notify_api,
                 'restricted': False,
                 'active': False,
                 'email_from': 'created.service',
-                'created_by': str(sample_user.id)}
+                'created_by': str(sample_user.id)
+            }
             auth_header = create_authorization_header()
             headers = [('Content-Type', 'application/json'), auth_header]
             resp = client.post(
@@ -1503,12 +1403,13 @@ def test_set_sms_prefixing_for_service_cant_be_none(
     admin_request,
     sample_service,
 ):
-    admin_request.post(
+    resp = admin_request.post(
         'service.update_service',
         service_id=sample_service.id,
         _data={'prefix_sms': None},
-        _expected_status=500,
+        _expected_status=400,
     )
+    assert resp['message'] == {'prefix_sms': ['Field may not be null.']}
 
 
 @pytest.mark.parametrize('today_only,stats', [