First batch of flake8 changes.

Many unused variables, and replacing some old fixtures with
admin_request before I realised just how many there where 😩

tests/app/notifications/test_receive_notification.py

@@ -1,4 +1,3 @@
-import uuid
 import base64
 from datetime import datetime
 from unittest.mock import call
@@ -6,7 +5,6 @@ from unittest.mock import call
 import pytest
 from flask import json
 
-from app.dao.services_dao import dao_fetch_service_by_inbound_number
 from app.notifications.receive_notifications import (
     format_mmg_message,
     format_mmg_datetime,
@@ -80,7 +78,7 @@ def test_receive_notification_from_mmg_without_permissions_does_not_persist(
     permissions
 ):
     mocked = mocker.patch("app.notifications.receive_notifications.tasks.send_inbound_sms_to_service.apply_async")
-    service = create_service_with_inbound_number(inbound_number='07111111111', service_permissions=permissions)
+    create_service_with_inbound_number(inbound_number='07111111111', service_permissions=permissions)
     data = {
         "ID": "1234",
         "MSISDN": "07111111111",
@@ -113,8 +111,7 @@ def test_receive_notification_from_firetext_without_permissions_does_not_persist
                  return_value=service)
     mocked_send_inbound_sms = mocker.patch(
         "app.notifications.receive_notifications.tasks.send_inbound_sms_to_service.apply_async")
-    mocked_has_permissions = mocker.patch(
-        "app.notifications.receive_notifications.has_inbound_sms_permissions", return_value=False)
+    mocker.patch("app.notifications.receive_notifications.has_inbound_sms_permissions", return_value=False)
 
     data = "source=07999999999&destination=07111111111&message=this is a message&time=2017-01-01 12:00:00"
     response = firetext_post(client, data)
@@ -339,7 +336,7 @@ def test_receive_notification_from_firetext_persists_message(notify_db_session,
 
 def test_receive_notification_from_firetext_persists_message_with_normalized_phone(notify_db_session, client, mocker):
     mocker.patch("app.notifications.receive_notifications.tasks.send_inbound_sms_to_service.apply_async")
-    mock = mocker.patch('app.notifications.receive_notifications.statsd_client.incr')
+    mocker.patch('app.notifications.receive_notifications.statsd_client.incr')
 
     create_service_with_inbound_number(
         inbound_number='07111111111', service_name='b', service_permissions=[EMAIL_TYPE, SMS_TYPE, INBOUND_SMS_TYPE])

tests/app/notifications/test_rest.py

@@ -244,7 +244,7 @@ def test_get_all_notifications_only_returns_notifications_of_matching_type(
 
 
 @pytest.mark.parametrize('key_type', [KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST])
-def test_no_api_keys_return_job_notifications_by_default(
+def test_do_not_return_job_notifications_by_default(
     client,
     sample_template,
     sample_job,
@@ -254,7 +254,7 @@ def test_no_api_keys_return_job_notifications_by_default(
     normal_api_key = create_api_key(sample_template.service, KEY_TYPE_NORMAL)
     test_api_key = create_api_key(sample_template.service, KEY_TYPE_TEST)
 
-    job_notification = create_notification(sample_template, job=sample_job)
+    create_notification(sample_template, job=sample_job)
     normal_notification = create_notification(sample_template, api_key=normal_api_key)
     team_notification = create_notification(sample_template, api_key=team_api_key)
     test_notification = create_notification(sample_template, api_key=test_api_key)
@@ -307,7 +307,7 @@ def test_only_normal_api_keys_can_return_job_notifications(
                           key_type=KEY_TYPE_TEST)
     save_model_api_key(test_api_key)
 
-    job_notification = create_sample_notification(
+    create_sample_notification(
         notify_db,
         notify_db_session,
         job=sample_job
@@ -381,8 +381,8 @@ def test_should_reject_invalid_page_param(client, sample_email_template):
 
 def test_valid_page_size_param(notify_api, notify_db, notify_db_session, sample_email_template):
     with notify_api.test_request_context():
-        n1 = create_sample_notification(notify_db, notify_db_session)
-        n2 = create_sample_notification(notify_db, notify_db_session)
+        create_sample_notification(notify_db, notify_db_session)
+        create_sample_notification(notify_db, notify_db_session)
         with notify_api.test_client() as client:
             auth_header = create_authorization_header(service_id=sample_email_template.service_id)
 
@@ -399,8 +399,8 @@ def test_valid_page_size_param(notify_api, notify_db, notify_db_session, sample_
 
 def test_invalid_page_size_param(client, notify_db, notify_db_session, sample_email_template):
 
-    n1 = create_sample_notification(notify_db, notify_db_session)
-    n2 = create_sample_notification(notify_db, notify_db_session)
+    create_sample_notification(notify_db, notify_db_session)
+    create_sample_notification(notify_db, notify_db_session)
     auth_header = create_authorization_header(service_id=sample_email_template.service_id)
 
     response = client.get(
@@ -454,12 +454,12 @@ def test_get_all_notifications_returns_empty_list(client, sample_api_key):
 
 
 def test_filter_by_template_type(client, notify_db, notify_db_session, sample_template, sample_email_template):
-    notification_1 = create_sample_notification(
+    create_sample_notification(
         notify_db,
         notify_db_session,
         service=sample_email_template.service,
         template=sample_template)
-    notification_2 = create_sample_notification(
+    create_sample_notification(
         notify_db,
         notify_db_session,
         service=sample_email_template.service,
@@ -480,8 +480,8 @@ def test_filter_by_template_type(client, notify_db, notify_db_session, sample_te
 def test_filter_by_multiple_template_types(client,
                                            sample_template,
                                            sample_email_template):
-    notification_1 = create_notification(sample_template)
-    notification_2 = create_notification(sample_email_template)
+    create_notification(sample_template)
+    create_notification(sample_email_template)
 
     auth_header = create_authorization_header(service_id=sample_email_template.service_id)
 
@@ -496,8 +496,8 @@ def test_filter_by_multiple_template_types(client,
 
 
 def test_filter_by_status(client, sample_email_template):
-    notification_1 = create_notification(sample_email_template, status="delivered")
-    notification_2 = create_notification(sample_email_template)
+    create_notification(sample_email_template, status="delivered")
+    create_notification(sample_email_template)
 
     auth_header = create_authorization_header(service_id=sample_email_template.service_id)
 
@@ -512,8 +512,8 @@ def test_filter_by_status(client, sample_email_template):
 
 
 def test_filter_by_multiple_statuses(client, sample_email_template):
-    notification_1 = create_notification(sample_email_template, status="delivered")
-    notification_2 = create_notification(sample_email_template, status='sending')
+    create_notification(sample_email_template, status="delivered")
+    create_notification(sample_email_template, status='sending')
 
     auth_header = create_authorization_header(service_id=sample_email_template.service_id)
 
@@ -529,9 +529,9 @@ def test_filter_by_multiple_statuses(client, sample_email_template):
 
 
 def test_filter_by_status_and_template_type(client, sample_template, sample_email_template):
-    notification_1 = create_notification(sample_template)
-    notification_2 = create_notification(sample_email_template)
-    notification_3 = create_notification(sample_email_template, status="delivered")
+    create_notification(sample_template)
+    create_notification(sample_email_template)
+    create_notification(sample_email_template, status="delivered")
 
     auth_header = create_authorization_header(service_id=sample_email_template.service_id)
 

tests/app/service/test_rest.py

@@ -77,7 +77,7 @@ def test_get_service_list_with_only_active_flag(client, service_factory):
 
 
 def test_get_service_list_with_user_id_and_only_active_flag(
-        client,
+        admin_request,
         sample_user,
         service_factory
 ):
@@ -85,72 +85,47 @@ def test_get_service_list_with_user_id_and_only_active_flag(
 
     inactive = service_factory.get('one', user=sample_user)
     active = service_factory.get('two', user=sample_user)
-    from_other_user = service_factory.get('three', user=other_user)
+    # from other user
+    service_factory.get('three', user=other_user)
 
     inactive.active = False
 
-    auth_header = create_authorization_header()
-    response = client.get(
-        '/service?user_id={}&only_active=True'.format(sample_user.id),
-        headers=[auth_header]
+    json_resp = admin_request.get(
+        'service.get_services',
+        user_id=sample_user.id,
+        only_active=True
     )
-    assert response.status_code == 200
-    json_resp = json.loads(response.get_data(as_text=True))
     assert len(json_resp['data']) == 1
     assert json_resp['data'][0]['id'] == str(active.id)
 
 
-def test_get_service_list_by_user(client, sample_user, service_factory):
+def test_get_service_list_by_user(admin_request, sample_user, service_factory):
     other_user = create_user(email='foo@bar.gov.uk')
     service_factory.get('one', sample_user)
     service_factory.get('two', sample_user)
     service_factory.get('three', other_user)
 
-    auth_header = create_authorization_header()
-    response = client.get(
-        '/service?user_id={}'.format(sample_user.id),
-        headers=[auth_header]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 200
+    json_resp = admin_request.get('service.get_services', user_id=sample_user.id)
     assert len(json_resp['data']) == 2
     assert json_resp['data'][0]['name'] == 'one'
     assert json_resp['data'][1]['name'] == 'two'
 
 
-def test_get_service_list_by_user_should_return_empty_list_if_no_services(client, sample_service):
+def test_get_service_list_by_user_should_return_empty_list_if_no_services(admin_request, sample_service):
     # service is already created by sample user
     new_user = create_user(email='foo@bar.gov.uk')
 
-    auth_header = create_authorization_header()
-    response = client.get(
-        '/service?user_id={}'.format(new_user.id),
-        headers=[auth_header]
-    )
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert response.status_code == 200
+    json_resp = admin_request.get('service.get_services', user_id=new_user.id)
+    assert json_resp['data'] == []
+
+
+def test_get_service_list_should_return_empty_list_if_no_services(admin_request):
+    json_resp = admin_request.get('service.get_services')
     assert len(json_resp['data']) == 0
 
 
-def test_get_service_list_should_return_empty_list_if_no_services(client):
-    auth_header = create_authorization_header()
-    response = client.get(
-        '/service',
-        headers=[auth_header]
-    )
-    assert response.status_code == 200
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert len(json_resp['data']) == 0
-
-
-def test_get_service_by_id(client, sample_service):
-    auth_header = create_authorization_header()
-    resp = client.get(
-        '/service/{}'.format(sample_service.id),
-        headers=[auth_header]
-    )
-    assert resp.status_code == 200
-    json_resp = json.loads(resp.get_data(as_text=True))
+def test_get_service_by_id(admin_request, sample_service):
+    json_resp = admin_request.get('service.get_service_by_id', service_id=sample_service.id)
     assert json_resp['data']['name'] == sample_service.name
     assert json_resp['data']['id'] == str(sample_service.id)
     assert not json_resp['data']['research_mode']
@@ -161,55 +136,29 @@ def test_get_service_by_id(client, sample_service):
     assert json_resp['data']['prefix_sms'] is True
 
 
-def test_get_service_by_id_returns_free_sms_limit(client, sample_service):
-
-    auth_header = create_authorization_header()
-    resp = client.get(
-        '/service/{}'.format(sample_service.id),
-        headers=[auth_header]
-    )
-    assert resp.status_code == 200
-    json_resp = json.loads(resp.get_data(as_text=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(client, sample_service):
-
-    auth_header = create_authorization_header()
-    resp = client.get(
-        '/service/{}?detailed=True'.format(sample_service.id),
-        headers=[auth_header]
-    )
-    assert resp.status_code == 200
-    json_resp = json.loads(resp.get_data(as_text=True))
+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('endpoint', ['/service/{}', '/service/{}?detailed=True'])
-def test_get_service_by_id_returns_organisation_type(client, sample_service, endpoint):
-
-    auth_header = create_authorization_header()
-    resp = client.get(
-        endpoint.format(sample_service.id),
-        headers=[auth_header]
-    )
-    assert resp.status_code == 200
-    json_resp = json.loads(resp.get_data(as_text=True))
+@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)
     assert json_resp['data']['organisation_type'] is None
 
 
-def test_get_service_list_has_default_permissions(client, service_factory):
+def test_get_service_list_has_default_permissions(admin_request, service_factory):
     service_factory.get('one')
     service_factory.get('one')
     service_factory.get('two')
     service_factory.get('three')
-    auth_header = create_authorization_header()
-    response = client.get(
-        '/service',
-        headers=[auth_header]
-    )
-    assert response.status_code == 200
-    json_resp = json.loads(response.get_data(as_text=True))
+
+    json_resp = admin_request.get('service.get_services')
     assert len(json_resp['data']) == 3
     assert all(
         set(
@@ -221,13 +170,8 @@ def test_get_service_list_has_default_permissions(client, service_factory):
     )
 
 
-def test_get_service_by_id_has_default_service_permissions(client, sample_service):
-    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))
+def test_get_service_by_id_has_default_service_permissions(admin_request, sample_service):
+    json_resp = admin_request.get('service.get_service_by_id', service_id=sample_service.id)
 
     assert set(
         json_resp['data']['permissions']
@@ -236,19 +180,15 @@ def test_get_service_by_id_has_default_service_permissions(client, sample_servic
     ])
 
 
-def test_get_service_by_id_should_404_if_no_service(notify_api, notify_db):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
-            service_id = str(uuid.uuid4())
-            auth_header = create_authorization_header()
-            resp = client.get(
-                '/service/{}'.format(service_id),
-                headers=[auth_header]
-            )
-            assert resp.status_code == 404
-            json_resp = json.loads(resp.get_data(as_text=True))
-            assert json_resp['result'] == 'error'
-            assert json_resp['message'] == 'No result found'
+def test_get_service_by_id_should_404_if_no_service(admin_request, notify_db_session):
+    json_resp = admin_request.get(
+        'service.get_service_by_id',
+        service_id=uuid.uuid4(),
+        _expected_status=404
+    )
+
+    assert json_resp['result'] == 'error'
+    assert json_resp['message'] == 'No result found'
 
 
 def test_get_service_by_id_and_user(client, sample_service, sample_user):
@@ -1596,7 +1536,7 @@ def test_get_detailed_service(notify_db, notify_db_session, notify_api, sample_s
     ]
 )
 def test_get_monthly_notification_stats(mocker, client, sample_service, url, expected_status, expected_json):
-    mock_dao = mocker.patch(
+    mocker.patch(
         'app.service.rest.dao_fetch_monthly_historical_stats_for_service',
         return_value={'foo': 'bar'},
     )
@@ -2577,7 +2517,7 @@ def test_get_letter_contacts_when_there_are_no_letter_contacts(client, sample_se
 
 def test_get_letter_contacts_with_one_letter_contact(client, notify_db, notify_db_session):
     service = create_service()
-    letter_contact = create_letter_contact(service, 'Aberdeen, AB23 1XH')
+    create_letter_contact(service, 'Aberdeen, AB23 1XH')
 
     response = client.get('/service/{}/letter-contact'.format(service.id),
                           headers=[create_authorization_header()])

tests/app/service/test_statistics.py

@@ -1,8 +1,6 @@
-from datetime import datetime
 import collections
 
 import pytest
-from freezegun import freeze_time
 
 from app.service.statistics import (
     format_statistics,

tests/app/service/test_suspend_resume_service.py

@@ -6,7 +6,7 @@ import uuid
 
 from freezegun import freeze_time
 
-from app.models import Service, ApiKey
+from app.models import Service
 from tests import create_authorization_header
 
 

tests/app/template/test_rest.py

@@ -1,4 +1,3 @@
-import uuid
 import json
 import random
 import string
@@ -17,7 +16,7 @@ from tests.app.conftest import (
     sample_template_without_letter_permission,
     sample_template_without_sms_permission,
 )
-from tests.app.db import create_service, create_letter_contact
+from tests.app.db import create_service, create_letter_contact, create_template
 
 
 @pytest.mark.parametrize('template_type, subject', [
@@ -309,68 +308,18 @@ def test_should_be_able_to_get_all_templates_for_a_service(client, sample_user,
     assert update_json_resp['data'][1]['created_at']
 
 
-def test_should_get_only_templates_for_that_service(client, sample_user, service_factory):
+def test_should_get_only_templates_for_that_service(admin_request, notify_db_sesision):
+    service_1 = create_service(service_name='service_1')
+    service_2 = create_service(service_name='service_2')
+    id_1 = create_template(service_1).id
+    id_2 = create_template(service_1).id
+    id_3 = create_template(service_2).id
 
-    service_1 = service_factory.get('service 1', email_from='service.1')
-    service_2 = service_factory.get('service 2', email_from='service.2')
+    json_resp_1 = admin_request.get('template.get_all_templates_for_service', service_id=service_1.id)
+    json_resp_2 = admin_request.get('template.get_all_templates_for_service', service_id=service_2.id)
 
-    auth_header_1 = create_authorization_header()
-
-    response_1 = client.get(
-        '/service/{}/template'.format(service_1.id),
-        headers=[auth_header_1]
-    )
-
-    auth_header_2 = create_authorization_header()
-
-    response_2 = client.get(
-        '/service/{}/template'.format(service_2.id),
-        headers=[auth_header_2]
-    )
-
-    assert response_1.status_code == 200
-    assert response_2.status_code == 200
-
-    json_resp_1 = json.loads(response_1.get_data(as_text=True))
-    json_resp_2 = json.loads(response_2.get_data(as_text=True))
-
-    assert len(json_resp_1['data']) == 1
-    assert len(json_resp_2['data']) == 1
-
-    data = {
-        'name': 'my template 2',
-        'template_type': EMAIL_TYPE,
-        'subject': 'subject 2',
-        'content': 'template content',
-        'service': str(service_1.id),
-        'created_by': str(sample_user.id)
-    }
-    data = json.dumps(data)
-    create_auth_header = create_authorization_header()
-    resp = client.post(
-        '/service/{}/template'.format(service_1.id),
-        headers=[('Content-Type', 'application/json'), create_auth_header],
-        data=data
-    )
-
-    response_3 = client.get(
-        '/service/{}/template'.format(service_1.id),
-        headers=[auth_header_1]
-    )
-
-    response_4 = client.get(
-        '/service/{}/template'.format(service_2.id),
-        headers=[auth_header_2]
-    )
-
-    assert response_3.status_code == 200
-    assert response_4.status_code == 200
-
-    json_resp_3 = json.loads(response_3.get_data(as_text=True))
-    json_resp_4 = json.loads(response_4.get_data(as_text=True))
-
-    assert len(json_resp_3['data']) == 2
-    assert len(json_resp_4['data']) == 1
+    assert {template['id'] for template in json_resp_1['data']} == {str(id_1), str(id_2)}
+    assert {template['id'] for template in json_resp_2['data']} == {str(id_3)}
 
 
 @pytest.mark.parametrize(

tests/app/user/test_rest_verify.py

@@ -382,7 +382,6 @@ def test_send_user_email_code_with_urlencoded_next_param(admin_request, mocker,
         _expected_status=204
     )
     noti = Notification.query.one()
-    code = VerifyCode.query.one()
     assert noti.personalisation['url'].endswith('?next=%2Fservices')
 
 

tests/app/v2/notifications/test_get_notifications.py

@@ -7,7 +7,7 @@ from tests import create_authorization_header
 from tests.app.db import (
     create_notification,
     create_template,
-    create_service)
+)
 
 from tests.app.conftest import (
     sample_notification,
@@ -30,7 +30,8 @@ def test_get_notification_by_id_returns_200(
         scheduled_for="2017-05-12 15:15"
     )
 
-    another = create_notification(
+    # another
+    create_notification(
         template=sample_template,
         billable_units=billable_units,
         sent_by=provider,

tests/app/v2/notifications/test_post_notifications.py

@@ -634,7 +634,7 @@ def test_post_email_notification_with_valid_reply_to_id_returns_201(client, samp
 
 
 def test_post_email_notification_with_invalid_reply_to_id_returns_400(client, sample_email_template, mocker, fake_uuid):
-    mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
+    mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
     data = {
         "email_address": sample_email_template.service.users[0].email_address,
         "template_id": sample_email_template.id,
@@ -652,10 +652,9 @@ def test_post_email_notification_with_invalid_reply_to_id_returns_400(client, sa
     assert 'BadRequestError' in resp_json['errors'][0]['error']
 
 
-def test_persist_sender_to_notification_mapping_for_email(notify_db_session):
-    service = create_service()
-    template = create_template(service=service, template_type=EMAIL_TYPE)
-    sender = create_reply_to_email(service=service, email_address='reply@test.com', is_default=False)
+def test_persist_sender_to_notification_mapping_for_email(sample_service):
+    template = create_template(service=sample_service, template_type=EMAIL_TYPE)
+    sender = create_reply_to_email(service=sample_service, email_address='reply@test.com', is_default=False)
     form = {
         "email_address": "recipient@test.com",
         "template_id": str(template.id),
@@ -669,10 +668,9 @@ def test_persist_sender_to_notification_mapping_for_email(notify_db_session):
     assert notification_to_email_reply_to[0].service_email_reply_to_id == sender.id
 
 
-def test_persist_sender_to_notification_mapping_for_sms(notify_db_session):
-    service = create_service()
-    template = create_template(service=service, template_type=SMS_TYPE)
-    sender = create_service_sms_sender(service=service, sms_sender='12345', is_default=False)
+def test_persist_sender_to_notification_mapping_for_sms(sample_service):
+    template = create_template(service=sample_service, template_type=SMS_TYPE)
+    sender = create_service_sms_sender(service=sample_service, sms_sender='12345', is_default=False)
     form = {
         'phone_number': '+447700900855',
         'template_id': str(template.id),