Merge pull request #1852 from alphagov/read-job-meta-from-s3

Read job metadata from S3 metadata

app/aws/s3.py

@@ -18,17 +18,25 @@ def get_s3_object(bucket_name, file_location):
     return s3.Object(bucket_name, file_location)
 
 
+def get_job_location(service_id, job_id):
+    return (
+        current_app.config['CSV_UPLOAD_BUCKET_NAME'],
+        FILE_LOCATION_STRUCTURE.format(service_id, job_id),
+    )
+
+
 def get_job_from_s3(service_id, job_id):
-    bucket_name = current_app.config['CSV_UPLOAD_BUCKET_NAME']
+    obj = get_s3_object(*get_job_location(service_id, job_id))
-    file_location = FILE_LOCATION_STRUCTURE.format(service_id, job_id)
-    obj = get_s3_object(bucket_name, file_location)
     return obj.get()['Body'].read().decode('utf-8')
 
 
+def get_job_metadata_from_s3(service_id, job_id):
+    obj = get_s3_object(*get_job_location(service_id, job_id))
+    return obj.get()['Metadata']
+
+
 def remove_job_from_s3(service_id, job_id):
-    bucket_name = current_app.config['CSV_UPLOAD_BUCKET_NAME']
+    return remove_s3_object(*get_job_location(service_id, job_id))
-    file_location = FILE_LOCATION_STRUCTURE.format(service_id, job_id)
-    return remove_s3_object(bucket_name, file_location)
 
 
 def get_s3_bucket_objects(bucket_name, subfolder='', older_than=7, limit_days=2):

app/job/rest.py

@@ -5,6 +5,7 @@ from flask import (
     current_app
 )
 
+from app.aws.s3 import get_job_metadata_from_s3
 from app.dao.jobs_dao import (
     dao_create_job,
     dao_update_job,
@@ -119,11 +120,22 @@ def create_job(service_id):
     data.update({
         "service": service_id
     })
+    try:
+        data.update(
+            **get_job_metadata_from_s3(service_id, data['id'])
+        )
+    except KeyError:
+        raise InvalidRequest({'id': ['Missing data for required field.']}, status_code=400)
+
+    data['template'] = data.pop('template_id')
     template = dao_get_template_by_id(data['template'])
 
     if template.template_type == LETTER_TYPE and service.restricted:
         raise InvalidRequest("Create letter job is not allowed for service in trial mode ", 403)
 
+    if data.get('valid') != 'True':
+        raise InvalidRequest("File is not valid, can't create job", 400)
+
     errors = unarchived_template_schema.validate({'archived': template.archived})
 
     if errors:

tests/app/job/test_rest.py

@@ -1,6 +1,7 @@
 import json
 import uuid
 from datetime import datetime, timedelta
+from functools import partial
 
 from freezegun import freeze_time
 import pytest
@@ -17,9 +18,7 @@ from app.dao.templates_dao import dao_update_template
 from app.models import NOTIFICATION_STATUS_TYPES, JOB_STATUS_TYPES, JOB_STATUS_PENDING
 
 
-def test_get_job_with_invalid_service_id_returns404(notify_api, sample_service):
+def test_get_job_with_invalid_service_id_returns404(client, sample_service):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
     path = '/service/{}/job'.format(sample_service.id)
     auth_header = create_authorization_header()
     response = client.get(path, headers=[auth_header])
@@ -28,10 +27,8 @@ def test_get_job_with_invalid_service_id_returns404(notify_api, sample_service):
     assert len(resp_json['data']) == 0
 
 
-def test_get_job_with_invalid_job_id_returns404(notify_api, sample_template):
+def test_get_job_with_invalid_job_id_returns404(client, sample_template):
     service_id = sample_template.service.id
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
     path = '/service/{}/job/{}'.format(service_id, "bad-id")
     auth_header = create_authorization_header()
     response = client.get(path, headers=[auth_header])
@@ -41,10 +38,8 @@ def test_get_job_with_invalid_job_id_returns404(notify_api, sample_template):
     assert resp_json['message'] == 'No result found'
 
 
-def test_get_job_with_unknown_id_returns404(notify_api, sample_template, fake_uuid):
+def test_get_job_with_unknown_id_returns404(client, sample_template, fake_uuid):
     service_id = sample_template.service.id
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
     path = '/service/{}/job/{}'.format(service_id, fake_uuid)
     auth_header = create_authorization_header()
     response = client.get(path, headers=[auth_header])
@@ -56,10 +51,9 @@ def test_get_job_with_unknown_id_returns404(notify_api, sample_template, fake_uu
     }
 
 
-def test_cancel_job(notify_api, sample_scheduled_job):
+def test_cancel_job(client, sample_scheduled_job):
     job_id = str(sample_scheduled_job.id)
     service_id = sample_scheduled_job.service.id
-    with notify_api.test_request_context(), notify_api.test_client() as client:
     path = '/service/{}/job/{}/cancel'.format(service_id, job_id)
     auth_header = create_authorization_header()
     response = client.post(path, headers=[auth_header])
@@ -69,10 +63,9 @@ def test_cancel_job(notify_api, sample_scheduled_job):
     assert resp_json['data']['job_status'] == 'cancelled'
 
 
-def test_cant_cancel_normal_job(notify_api, sample_job, mocker):
+def test_cant_cancel_normal_job(client, sample_job, mocker):
     job_id = str(sample_job.id)
     service_id = sample_job.service.id
-    with notify_api.test_request_context(), notify_api.test_client() as client:
     mock_update = mocker.patch('app.dao.jobs_dao.dao_update_job')
     path = '/service/{}/job/{}/cancel'.format(service_id, job_id)
     auth_header = create_authorization_header()
@@ -81,17 +74,17 @@ def test_cant_cancel_normal_job(notify_api, sample_job, mocker):
     assert mock_update.call_count == 0
 
 
-def test_create_unscheduled_job(notify_api, sample_template, mocker, fake_uuid):
+def test_create_unscheduled_job(client, sample_template, mocker, fake_uuid):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
     mocker.patch('app.celery.tasks.process_job.apply_async')
+    mocker.patch('app.job.rest.get_job_metadata_from_s3', return_value={
+        'template_id': str(sample_template.id),
+        'original_file_name': 'thisisatest.csv',
+        'notification_count': '1',
+        'valid': 'True',
+    })
     data = {
         'id': fake_uuid,
-                'service': str(sample_template.service.id),
+        'created_by': str(sample_template.created_by.id),
-                'template': str(sample_template.id),
-                'original_file_name': 'thisisatest.csv',
-                'notification_count': 1,
-                'created_by': str(sample_template.created_by.id)
     }
     path = '/service/{}/job'.format(sample_template.service.id)
     auth_header = create_authorization_header()
@@ -117,22 +110,23 @@ def test_create_unscheduled_job(notify_api, sample_template, mocker, fake_uuid):
     assert resp_json['data']['job_status'] == 'pending'
     assert resp_json['data']['template'] == str(sample_template.id)
     assert resp_json['data']['original_file_name'] == 'thisisatest.csv'
+    assert resp_json['data']['notification_count'] == 1
 
 
-def test_create_scheduled_job(notify_api, sample_template, mocker, fake_uuid):
+@freeze_time("2016-01-01 12:00:00.000000")
-    with notify_api.test_request_context():
+def test_create_scheduled_job(client, sample_template, mocker, fake_uuid):
-        with notify_api.test_client() as client:
-            with freeze_time("2016-01-01 12:00:00.000000"):
     scheduled_date = (datetime.utcnow() + timedelta(hours=95, minutes=59)).isoformat()
     mocker.patch('app.celery.tasks.process_job.apply_async')
+    mocker.patch('app.job.rest.get_job_metadata_from_s3', return_value={
+        'template_id': str(sample_template.id),
+        'original_file_name': 'thisisatest.csv',
+        'notification_count': '1',
+        'valid': 'True',
+    })
     data = {
         'id': fake_uuid,
-                    'service': str(sample_template.service.id),
-                    'template': str(sample_template.id),
-                    'original_file_name': 'thisisatest.csv',
-                    'notification_count': 1,
         'created_by': str(sample_template.created_by.id),
-                    'scheduled_for': scheduled_date
+        'scheduled_for': scheduled_date,
     }
     path = '/service/{}/job'.format(sample_template.service.id)
     auth_header = create_authorization_header()
@@ -154,6 +148,7 @@ def test_create_scheduled_job(notify_api, sample_template, mocker, fake_uuid):
     assert resp_json['data']['job_status'] == 'scheduled'
     assert resp_json['data']['template'] == str(sample_template.id)
     assert resp_json['data']['original_file_name'] == 'thisisatest.csv'
+    assert resp_json['data']['notification_count'] == 1
 
 
 def test_create_job_returns_403_if_service_is_not_active(client, fake_uuid, sample_service, mocker):
@@ -171,15 +166,53 @@ def test_create_job_returns_403_if_service_is_not_active(client, fake_uuid, samp
     mock_job_dao.assert_not_called()
 
 
+@pytest.mark.parametrize('extra_metadata', (
+    {},
+    {'valid': 'anything not the string True'},
+))
+def test_create_job_returns_400_if_file_is_invalid(
+    client,
+    fake_uuid,
+    sample_template,
+    mocker,
+    extra_metadata,
+):
+    mock_job_dao = mocker.patch("app.dao.jobs_dao.dao_create_job")
+    auth_header = create_authorization_header()
+    metadata = dict(
+        template_id=str(sample_template.id),
+        original_file_name='thisisatest.csv',
+        notification_count=1,
+        **extra_metadata
+    )
+    mocker.patch('app.job.rest.get_job_metadata_from_s3', return_value=metadata)
+    data = {'id': fake_uuid}
+    response = client.post(
+        '/service/{}/job'.format(sample_template.service.id),
+        data=json.dumps(data),
+        headers=[('Content-Type', 'application/json'), auth_header]
+    )
+
+    assert response.status_code == 400
+    print(response.get_data(as_text=True))
+    resp_json = json.loads(response.get_data(as_text=True))
+    print(resp_json)
+    assert resp_json['result'] == 'error'
+    assert resp_json['message'] == 'File is not valid, can\'t create job'
+    mock_job_dao.assert_not_called()
+
+
 def test_create_job_returns_403_if_letter_template_type_and_service_in_trial(
-        client, fake_uuid, sample_trial_letter_template, mocker):
+    client, fake_uuid, sample_trial_letter_template, mocker
+):
+    mocker.patch('app.job.rest.get_job_metadata_from_s3', return_value={
+        'template_id': str(sample_trial_letter_template.id),
+        'original_file_name': 'thisisatest.csv',
+        'notification_count': '1',
+    })
     data = {
         'id': fake_uuid,
-        'service': str(sample_trial_letter_template.service.id),
+        'created_by': str(sample_trial_letter_template.created_by.id),
-        'template': str(sample_trial_letter_template.id),
-        'original_file_name': 'thisisatest.csv',
-        'notification_count': 1,
-        'created_by': str(sample_trial_letter_template.created_by.id)
     }
     mock_job_dao = mocker.patch("app.dao.jobs_dao.dao_create_job")
     auth_header = create_authorization_header()
@@ -194,21 +227,20 @@ def test_create_job_returns_403_if_letter_template_type_and_service_in_trial(
     mock_job_dao.assert_not_called()
 
 
-def test_should_not_create_scheduled_job_more_then_24_hours_hence(notify_api, sample_template, mocker, fake_uuid):
+@freeze_time("2016-01-01 11:09:00.061258")
-    with notify_api.test_request_context():
+def test_should_not_create_scheduled_job_more_then_24_hours_hence(client, sample_template, mocker, fake_uuid):
-        with notify_api.test_client() as client:
-            with freeze_time("2016-01-01 11:09:00.061258"):
     scheduled_date = (datetime.utcnow() + timedelta(hours=96, minutes=1)).isoformat()
-
     mocker.patch('app.celery.tasks.process_job.apply_async')
+    mocker.patch('app.job.rest.get_job_metadata_from_s3', return_value={
+        'template_id': str(sample_template.id),
+        'original_file_name': 'thisisatest.csv',
+        'notification_count': '1',
+        'valid': 'True',
+    })
     data = {
         'id': fake_uuid,
-                    'service': str(sample_template.service.id),
-                    'template': str(sample_template.id),
-                    'original_file_name': 'thisisatest.csv',
-                    'notification_count': 1,
         'created_by': str(sample_template.created_by.id),
-                    'scheduled_for': scheduled_date
+        'scheduled_for': scheduled_date,
     }
     path = '/service/{}/job'.format(sample_template.service.id)
     auth_header = create_authorization_header()
@@ -228,19 +260,18 @@ def test_should_not_create_scheduled_job_more_then_24_hours_hence(notify_api, sa
     assert resp_json['message']['scheduled_for'] == ['Date cannot be more than 96hrs in the future']
 
 
-def test_should_not_create_scheduled_job_in_the_past(notify_api, sample_template, mocker, fake_uuid):
+@freeze_time("2016-01-01 11:09:00.061258")
-    with notify_api.test_request_context():
+def test_should_not_create_scheduled_job_in_the_past(client, sample_template, mocker, fake_uuid):
-        with notify_api.test_client() as client:
-            with freeze_time("2016-01-01 11:09:00.061258"):
     scheduled_date = (datetime.utcnow() - timedelta(minutes=1)).isoformat()
-
     mocker.patch('app.celery.tasks.process_job.apply_async')
+    mocker.patch('app.job.rest.get_job_metadata_from_s3', return_value={
+        'template_id': str(sample_template.id),
+        'original_file_name': 'thisisatest.csv',
+        'notification_count': '1',
+        'valid': 'True',
+    })
     data = {
         'id': fake_uuid,
-                    'service': str(sample_template.service.id),
-                    'template': str(sample_template.id),
-                    'original_file_name': 'thisisatest.csv',
-                    'notification_count': 1,
         'created_by': str(sample_template.created_by.id),
         'scheduled_for': scheduled_date
     }
@@ -262,12 +293,36 @@ def test_should_not_create_scheduled_job_in_the_past(notify_api, sample_template
     assert resp_json['message']['scheduled_for'] == ['Date cannot be in the past']
 
 
-def test_create_job_returns_400_if_missing_data(notify_api, sample_template, mocker):
+def test_create_job_returns_400_if_missing_id(client, sample_template, mocker):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
     mocker.patch('app.celery.tasks.process_job.apply_async')
+    mocker.patch('app.job.rest.get_job_metadata_from_s3', return_value={
+        'template_id': str(sample_template.id),
+    })
+    data = {}
+    path = '/service/{}/job'.format(sample_template.service.id)
+    auth_header = create_authorization_header()
+    headers = [('Content-Type', 'application/json'), auth_header]
+    response = client.post(
+        path,
+        data=json.dumps(data),
+        headers=headers)
+
+    resp_json = json.loads(response.get_data(as_text=True))
+    assert response.status_code == 400
+
+    app.celery.tasks.process_job.apply_async.assert_not_called()
+    assert resp_json['result'] == 'error'
+    assert 'Missing data for required field.' in resp_json['message']['id']
+
+
+def test_create_job_returns_400_if_missing_data(client, sample_template, mocker, fake_uuid):
+    mocker.patch('app.celery.tasks.process_job.apply_async')
+    mocker.patch('app.job.rest.get_job_metadata_from_s3', return_value={
+        'template_id': str(sample_template.id),
+    })
     data = {
-                'template': str(sample_template.id)
+        'id': fake_uuid,
+        'valid': 'True',
     }
     path = '/service/{}/job'.format(sample_template.service.id)
     auth_header = create_authorization_header()
@@ -286,12 +341,13 @@ def test_create_job_returns_400_if_missing_data(notify_api, sample_template, moc
     assert 'Missing data for required field.' in resp_json['message']['notification_count']
 
 
-def test_create_job_returns_404_if_template_does_not_exist(notify_api, sample_service, mocker):
+def test_create_job_returns_404_if_template_does_not_exist(client, sample_service, mocker, fake_uuid):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
     mocker.patch('app.celery.tasks.process_job.apply_async')
+    mocker.patch('app.job.rest.get_job_metadata_from_s3', return_value={
+        'template_id': str(sample_service.id),
+    })
     data = {
-                'template': str(sample_service.id)
+        'id': fake_uuid,
     }
     path = '/service/{}/job'.format(sample_service.id)
     auth_header = create_authorization_header()
@@ -309,12 +365,13 @@ def test_create_job_returns_404_if_template_does_not_exist(notify_api, sample_se
     assert resp_json['message'] == 'No result found'
 
 
-def test_create_job_returns_404_if_missing_service(notify_api, sample_template, mocker):
+def test_create_job_returns_404_if_missing_service(client, sample_template, mocker):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
     mocker.patch('app.celery.tasks.process_job.apply_async')
+    mocker.patch('app.job.rest.get_job_metadata_from_s3', return_value={
+        'template_id': str(sample_template.id),
+    })
     random_id = str(uuid.uuid4())
-            data = {'template': str(sample_template.id)}
+    data = {}
     path = '/service/{}/job'.format(random_id)
     auth_header = create_authorization_header()
     headers = [('Content-Type', 'application/json'), auth_header]
@@ -331,14 +388,16 @@ def test_create_job_returns_404_if_missing_service(notify_api, sample_template,
     assert resp_json['message'] == 'No result found'
 
 
-def test_create_job_returns_400_if_archived_template(notify_api, sample_template, mocker):
+def test_create_job_returns_400_if_archived_template(client, sample_template, mocker, fake_uuid):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
     mocker.patch('app.celery.tasks.process_job.apply_async')
     sample_template.archived = True
     dao_update_template(sample_template)
+    mocker.patch('app.job.rest.get_job_metadata_from_s3', return_value={
+        'template_id': str(sample_template.id),
+    })
     data = {
-                'template': str(sample_template.id)
+        'id': fake_uuid,
+        'valid': 'True',
     }
     path = '/service/{}/job'.format(sample_template.service.id)
     auth_header = create_authorization_header()
@@ -365,11 +424,9 @@ def _setup_jobs(notify_db, notify_db_session, template, number_of_jobs=5):
             template=template)
 
 
-def test_get_all_notifications_for_job_in_order_of_job_number(notify_api,
+def test_get_all_notifications_for_job_in_order_of_job_number(
-                                                              notify_db,
+    client, notify_db, notify_db_session, sample_service
-                                                              notify_db_session,
+):
-                                                              sample_service):
-    with notify_api.test_request_context(), notify_api.test_client() as client:
     main_job = create_job(notify_db, notify_db_session, service=sample_service)
     another_job = create_job(notify_db, notify_db_session, service=sample_service)
 
@@ -426,14 +483,13 @@ def test_get_all_notifications_for_job_in_order_of_job_number(notify_api,
     ]
 )
 def test_get_all_notifications_for_job_filtered_by_status(
-        notify_api,
+        client,
         notify_db,
         notify_db_session,
         sample_service,
         expected_notification_count,
         status_args
 ):
-    with notify_api.test_request_context(), notify_api.test_client() as client:
     job = create_job(notify_db, notify_db_session, service=sample_service)
 
     create_notification(
@@ -487,21 +543,21 @@ def test_get_job_by_id(notify_api, sample_job):
             assert resp_json['data']['created_by']['name'] == 'Test User'
 
 
-def test_get_job_by_id_should_return_statistics(notify_db, notify_db_session, notify_api, sample_job):
+def test_get_job_by_id_should_return_statistics(client, notify_db, notify_db_session, notify_api, sample_job):
     job_id = str(sample_job.id)
     service_id = sample_job.service.id
+    partial_notification = partial(
+        create_notification, notify_db, notify_db_session, service=sample_job.service, job=sample_job
+    )
+    partial_notification(status='created')
+    partial_notification(status='sending')
+    partial_notification(status='delivered')
+    partial_notification(status='pending')
+    partial_notification(status='failed')
+    partial_notification(status='technical-failure')  # noqa
+    partial_notification(status='temporary-failure')  # noqa
+    partial_notification(status='permanent-failure')  # noqa
 
-    create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='created')
-    create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='sending')
-    create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='delivered')
-    create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='pending')
-    create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='failed')
-    create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='technical-failure')  # noqa
-    create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='temporary-failure')  # noqa
-    create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='permanent-failure')  # noqa
-
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
     path = '/service/{}/job/{}'.format(service_id, job_id)
     auth_header = create_authorization_header()
     response = client.get(path, headers=[auth_header])
@@ -519,23 +575,23 @@ def test_get_job_by_id_should_return_statistics(notify_db, notify_db_session, no
     assert resp_json['data']['created_by']['name'] == 'Test User'
 
 
-def test_get_job_by_id_should_return_summed_statistics(notify_db, notify_db_session, notify_api, sample_job):
+def test_get_job_by_id_should_return_summed_statistics(client, notify_db, notify_db_session, notify_api, sample_job):
     job_id = str(sample_job.id)
     service_id = sample_job.service.id
+    partial_notification = partial(
+        create_notification, notify_db, notify_db_session, service=sample_job.service, job=sample_job
+    )
+    partial_notification(status='created')
+    partial_notification(status='created')
+    partial_notification(status='created')
+    partial_notification(status='sending')
+    partial_notification(status='failed')
+    partial_notification(status='failed')
+    partial_notification(status='failed')
+    partial_notification(status='technical-failure')
+    partial_notification(status='temporary-failure')
+    partial_notification(status='temporary-failure')
 
-    create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='created')
-    create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='created')
-    create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='created')
-    create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='sending')
-    create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='failed')
-    create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='failed')
-    create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='failed')
-    create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='technical-failure')  # noqa
-    create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='temporary-failure')  # noqa
-    create_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='temporary-failure')  # noqa
-
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
     path = '/service/{}/job/{}'.format(service_id, job_id)
     auth_header = create_authorization_header()
     response = client.get(path, headers=[auth_header])
@@ -550,13 +606,11 @@ def test_get_job_by_id_should_return_summed_statistics(notify_db, notify_db_sess
     assert resp_json['data']['created_by']['name'] == 'Test User'
 
 
-def test_get_jobs(notify_api, notify_db, notify_db_session, sample_template):
+def test_get_jobs(client, notify_db, notify_db_session, sample_template):
     _setup_jobs(notify_db, notify_db_session, sample_template)
 
     service_id = sample_template.service.id
 
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
     path = '/service/{}/job'.format(service_id)
     auth_header = create_authorization_header()
     response = client.get(path, headers=[auth_header])
@@ -565,7 +619,7 @@ def test_get_jobs(notify_api, notify_db, notify_db_session, sample_template):
     assert len(resp_json['data']) == 5
 
 
-def test_get_jobs_with_limit_days(notify_api, notify_db, notify_db_session, sample_template):
+def test_get_jobs_with_limit_days(client, notify_db, notify_db_session, sample_template):
     create_job(
         notify_db,
         notify_db_session,
@@ -581,8 +635,6 @@ def test_get_jobs_with_limit_days(notify_api, notify_db, notify_db_session, samp
 
     service_id = sample_template.service.id
 
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
     path = '/service/{}/job'.format(service_id)
     auth_header = create_authorization_header()
     response = client.get(path, headers=[auth_header], query_string={'limit_days': 5})
@@ -591,18 +643,18 @@ def test_get_jobs_with_limit_days(notify_api, notify_db, notify_db_session, samp
     assert len(resp_json['data']) == 1
 
 
-def test_get_jobs_should_return_statistics(notify_db, notify_db_session, notify_api, sample_service):
+def test_get_jobs_should_return_statistics(client, notify_db, notify_db_session, notify_api, sample_service):
     now = datetime.utcnow()
     earlier = datetime.utcnow() - timedelta(days=1)
     job_1 = create_job(notify_db, notify_db_session, service=sample_service, created_at=earlier)
     job_2 = create_job(notify_db, notify_db_session, service=sample_service, created_at=now)
-
+    partial_notification = partial(create_notification, notify_db, notify_db_session, service=sample_service)
-    create_notification(notify_db, notify_db_session, service=sample_service, job=job_1, status='created')
+    partial_notification(job=job_1, status='created')
-    create_notification(notify_db, notify_db_session, service=sample_service, job=job_1, status='created')
+    partial_notification(job=job_1, status='created')
-    create_notification(notify_db, notify_db_session, service=sample_service, job=job_1, status='created')
+    partial_notification(job=job_1, status='created')
-    create_notification(notify_db, notify_db_session, service=sample_service, job=job_2, status='sending')
+    partial_notification(job=job_2, status='sending')
-    create_notification(notify_db, notify_db_session, service=sample_service, job=job_2, status='sending')
+    partial_notification(job=job_2, status='sending')
-    create_notification(notify_db, notify_db_session, service=sample_service, job=job_2, status='sending')
+    partial_notification(job=job_2, status='sending')
 
     with notify_api.test_request_context():
         with notify_api.test_client() as client:
@@ -619,18 +671,18 @@ def test_get_jobs_should_return_statistics(notify_db, notify_db_session, notify_
 
 
 def test_get_jobs_should_return_no_stats_if_no_rows_in_notifications(
+    client,
     notify_db,
     notify_db_session,
     notify_api,
-        sample_service):
+    sample_service,
+):
 
     now = datetime.utcnow()
     earlier = datetime.utcnow() - timedelta(days=1)
     job_1 = create_job(notify_db, notify_db_session, service=sample_service, created_at=earlier)
     job_2 = create_job(notify_db, notify_db_session, service=sample_service, created_at=now)
 
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
     path = '/service/{}/job'.format(sample_service.id)
     auth_header = create_authorization_header()
     response = client.get(path, headers=[auth_header])