diff --git a/.flake8 b/.flake8 new file mode 100644 index 000000000..f4f92e1bc --- /dev/null +++ b/.flake8 @@ -0,0 +1,7 @@ +[flake8] +# Rule definitions: http://flake8.pycqa.org/en/latest/user/error-codes.html +# W503: line break before binary operator +exclude = venv*,__pycache__,node_modules,cache +ignore = W503 +max-complexity = 14 +max-line-length = 120 diff --git a/app/__init__.py b/app/__init__.py index 4d08261e8..34aa8523e 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -1,24 +1,20 @@ import os -import re import urllib -import json from datetime import datetime, timedelta, timezone from time import monotonic -import dateutil import itertools import ago from flask import ( Flask, session, - Markup, - escape, render_template, make_response, current_app, request, g, - url_for) + url_for +) from flask._compat import string_types from flask.globals import _lookup_req_object, _request_ctx_stack from flask_login import LoginManager @@ -38,7 +34,7 @@ from notifications_utils.formatters import formatted_list from werkzeug.exceptions import abort from werkzeug.local import LocalProxy -import app.proxy_fix +from app import proxy_fix from app.asset_fingerprinter import AssetFingerprinter from app.its_dangerous_session import ItsdangerousSessionInterface from app.notify_client.service_api_client import ServiceAPIClient @@ -135,7 +131,6 @@ def create_app(): application.add_template_filter(format_datetime_normal) application.add_template_filter(format_datetime_short) application.add_template_filter(format_time) - application.add_template_filter(syntax_highlight_json) application.add_template_filter(valid_phone_number) application.add_template_filter(linkable_name) application.add_template_filter(format_date) @@ -217,10 +212,6 @@ def linkable_name(value): return urllib.parse.quote_plus(value) -def syntax_highlight_json(code): - return Markup(highlight(code, JavascriptLexer(), HtmlFormatter(noclasses=True))) - - def format_datetime(date): return '{} at {}'.format( format_date(date), diff --git a/app/config.py b/app/config.py index 209fad4cf..bb9932494 100644 --- a/app/config.py +++ b/app/config.py @@ -1,5 +1,4 @@ import os -from datetime import timedelta if os.environ.get('VCAP_SERVICES'): diff --git a/app/event_handlers.py b/app/event_handlers.py index 8fc3a815d..8200e34be 100644 --- a/app/event_handlers.py +++ b/app/event_handlers.py @@ -1,5 +1,4 @@ from app import events_api_client -from flask_login import current_user def on_user_logged_in(sender, user): diff --git a/app/main/__init__.py b/app/main/__init__.py index 1a7d56f17..042aa1e3d 100644 --- a/app/main/__init__.py +++ b/app/main/__init__.py @@ -2,7 +2,7 @@ from flask import Blueprint main = Blueprint('main', __name__) # noqa -from app.main.views import ( +from app.main.views import ( # noqa index, sign_in, sign_out, diff --git a/app/main/forms.py b/app/main/forms.py index ea34f8137..cf899cc4f 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -23,7 +23,7 @@ from wtforms import ( RadioField, FieldList, DateField, - SelectField) +) from wtforms.fields.html5 import EmailField, TelField, SearchField from wtforms.validators import (DataRequired, Email, Length, Regexp, Optional) from flask_wtf.file import FileField as FileField_wtf, FileAllowed diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index c8f8b66f8..151cfc98d 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -34,7 +34,7 @@ def whitelist(service_id): if form.validate_on_submit(): service_api_client.update_whitelist(service_id, { 'email_addresses': list(filter(None, form.email_addresses.data)), - 'phone_numbers': list(filter(None, form.phone_numbers.data)) + 'phone_numbers': list(filter(None, form.phone_numbers.data)) }) flash('Whitelist updated', 'default_with_tick') return redirect(url_for('.api_integration', service_id=service_id)) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 028bc2469..f4d5dad19 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -21,7 +21,6 @@ from app import ( service_api_client, template_statistics_client, inbound_number_client, - format_datetime_short, format_date_numeric, format_datetime_numeric, ) diff --git a/app/main/views/feedback.py b/app/main/views/feedback.py index 6f1631a2f..9d5f135d6 100644 --- a/app/main/views/feedback.py +++ b/app/main/views/feedback.py @@ -1,6 +1,6 @@ import requests import pytz -from flask import render_template, url_for, redirect, flash, current_app, abort, request, session +from flask import render_template, url_for, redirect, current_app, abort, request, session from flask_login import current_user from app import convert_to_boolean, current_service, service_api_client from app.main import main @@ -118,8 +118,9 @@ def feedback(ticket_type): current_app.logger.error( "Deskpro create ticket request failed with {} '{}'".format( resp.status_code, - resp.json()) + resp.json() ) + ) abort(500, "Feedback submission failed") return redirect(url_for('.thanks', urgent=urgent, anonymous=anonymous)) diff --git a/app/main/views/inbound_number.py b/app/main/views/inbound_number.py index 17cb4af15..8f5a5bfaa 100644 --- a/app/main/views/inbound_number.py +++ b/app/main/views/inbound_number.py @@ -1,16 +1,8 @@ -from flask import ( - render_template, - redirect, - session, - url_for, - request -) - +from flask import render_template from flask_login import login_required from app.main import main from app import inbound_number_client from app.utils import user_has_permissions -from flask import jsonify @main.route('/inbound-sms-admin', methods=['GET', 'POST']) diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 6e5a2cc12..511716530 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -34,7 +34,6 @@ from app.utils import ( generate_previous_dict, user_has_permissions, generate_notifications_csv, - get_template, get_time_left, REQUESTED_STATUSES, FAILURE_STATUSES, diff --git a/app/main/views/letter_jobs.py b/app/main/views/letter_jobs.py index 56ef47efe..a63e1f99d 100644 --- a/app/main/views/letter_jobs.py +++ b/app/main/views/letter_jobs.py @@ -1,9 +1,7 @@ -import datetime - from flask import redirect, render_template, request, session, url_for from flask_login import login_required -from app import letter_jobs_client, format_datetime_24h +from app import letter_jobs_client from app.main import main from app.utils import user_has_permissions diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index 8f45b80fc..8eae948b3 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -108,7 +108,7 @@ def remove_user_from_service(service_id, user_id): # Do it through the template or the form class? form = PermissionsForm(**{ role: user.has_permissions(permissions=permissions) for role, permissions in roles.items() - }) + }) if request.method == 'POST': try: diff --git a/app/main/views/notifications.py b/app/main/views/notifications.py index 1ac400eaf..cb52d5b34 100644 --- a/app/main/views/notifications.py +++ b/app/main/views/notifications.py @@ -118,8 +118,6 @@ def view_notification_updates(service_id, notification_id): def get_single_notification_partials(notification): - status_args = get_status_arg(request.args) - return { 'notifications': render_template( 'partials/notifications/notifications.html', diff --git a/app/main/views/organisations.py b/app/main/views/organisations.py index c7d41a38e..735329193 100644 --- a/app/main/views/organisations.py +++ b/app/main/views/organisations.py @@ -1,4 +1,4 @@ -from flask import current_app, redirect, render_template, session, url_for, request +from flask import current_app, redirect, render_template, session, url_for from flask_login import login_required from app import organisations_client diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 19ac0f39f..7356f5c53 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -74,12 +74,12 @@ def service_settings(service_id): reply_to_email_address_count = len(reply_to_email_addresses) default_reply_to_email_address = next( (x['email_address'] for x in reply_to_email_addresses if x['is_default']), "None" - ) + ) letter_contact_details = service_api_client.get_letter_contacts(service_id) letter_contact_details_count = len(letter_contact_details) default_letter_contact_block = next( (Field(x['contact_block'], html='escape') for x in letter_contact_details if x['is_default']), "None" - ) + ) return render_template( 'views/service-settings.html', organisation=organisation, @@ -417,7 +417,7 @@ def service_set_sms_sender(service_id): service_api_client.update_service( current_service['id'], sms_sender=form.sms_sender.data or None - ) + ) return redirect(url_for('.service_settings', service_id=service_id)) if request.method == 'GET': diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index 73108b4b4..69fc36a29 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -11,13 +11,11 @@ from flask import ( from flask_login import ( current_user, - confirm_login ) from app import ( login_manager, user_api_client, - service_api_client, invite_api_client ) @@ -56,9 +54,10 @@ def sign_in(): else: return redirect(url_for('.two_factor')) # Vague error message for login in case of user not known, locked, inactive or password not verified - flash(Markup(( - "The email address or password you entered is incorrect." - " Forgot your password?" + flash(Markup( + ( + "The email address or password you entered is incorrect." + " Forgot your password?" ).format(password_reset=url_for('.forgot_password')) )) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 719d8ae40..86a0aa42e 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -8,19 +8,17 @@ from flask import ( url_for, flash, abort, - json, Response, ) from flask_login import login_required, current_user from dateutil.parser import parse -from notifications_utils.formatters import escape_html from notifications_utils.recipients import first_column_headings from notifications_utils.template import LetterDVLATemplate from notifications_python_client.errors import HTTPError from app.main import main -from app.utils import user_has_permissions, get_template, get_help_argument, email_or_sms_not_enabled +from app.utils import user_has_permissions, get_template, email_or_sms_not_enabled from app.template_previews import TemplatePreview, get_page_count_for_letter from app.main.forms import ( ChooseTemplateType, @@ -61,7 +59,7 @@ def view_template(service_id, template_id): letter_contact_details = service_api_client.get_letter_contacts(service_id) default_letter_contact_block_id = next( (x['id'] for x in letter_contact_details if x['is_default']), "None" - ) + ) else: default_letter_contact_block_id = None return render_template( diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 1a0692bb0..d8c034eaa 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -3,7 +3,6 @@ from __future__ import unicode_literals from flask import url_for from app.utils import BrowsableItem from app.notify_client import _attach_current_user, NotifyAdminAPIClient -from . import notification_api_client class ServiceAPIClient(NotifyAdminAPIClient): diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index b3904c653..887339fab 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -1,6 +1,3 @@ -import uuid - -from flask import session from notifications_python_client.errors import HTTPError from app.notify_client import NotifyAdminAPIClient diff --git a/app/utils.py b/app/utils.py index d21d5e643..d7ad0ce15 100644 --- a/app/utils.py +++ b/app/utils.py @@ -8,7 +8,6 @@ import unicodedata from urllib.parse import urlparse from collections import namedtuple from datetime import datetime, timedelta, timezone -from dateutil import parser import dateutil import ago diff --git a/requirements_for_test.txt b/requirements_for_test.txt index 05320886a..36ed7cfdc 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -1,5 +1,4 @@ -r requirements.txt -pycodestyle==2.3.1 pytest==3.2.3 pytest-mock==1.6.3 pytest-cov==2.5.1 @@ -8,3 +7,4 @@ coveralls==1.2.0 httpretty==0.8.14 beautifulsoup4==4.6.0 freezegun==0.3.9 +flake8==3.4.1 diff --git a/tests/__init__.py b/tests/__init__.py index ed53da305..d39311897 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1,4 +1,3 @@ -import csv import pytest import uuid diff --git a/tests/app/main/test_asset_fingerprinter.py b/tests/app/main/test_asset_fingerprinter.py index c2cbc5df7..2e36cd847 100644 --- a/tests/app/main/test_asset_fingerprinter.py +++ b/tests/app/main/test_asset_fingerprinter.py @@ -1,7 +1,4 @@ # coding=utf-8 -import os - -from unittest import mock from app.asset_fingerprinter import AssetFingerprinter @@ -95,5 +92,5 @@ class TestAssetFingerprint(object): class TestAssetFingerprintWithUnicode(object): def test_can_read_self(self): - string_with_unicode_character = 'Ralph’s apostrophe' + 'Ralph’s apostrophe is a string containing a unicode character' AssetFingerprinter(filesystem_path='tests/app/main/').get_url('test_asset_fingerprinter.py') diff --git a/tests/app/main/test_errorhandlers.py b/tests/app/main/test_errorhandlers.py index ef97c2ad2..f9185a1d1 100644 --- a/tests/app/main/test_errorhandlers.py +++ b/tests/app/main/test_errorhandlers.py @@ -1,5 +1,4 @@ from bs4 import BeautifulSoup -from flask import url_for def test_bad_url_returns_page_not_found(client): diff --git a/tests/app/main/test_permissions.py b/tests/app/main/test_permissions.py index a8d385a53..721e74953 100644 --- a/tests/app/main/test_permissions.py +++ b/tests/app/main/test_permissions.py @@ -20,10 +20,10 @@ def _test_permissions( decorator = user_has_permissions(*permissions, any_=any_, admin_override=admin_override) decorated_index = decorator(index) if will_succeed: - response = decorated_index() + decorated_index() else: try: - response = decorated_index() + decorated_index() pytest.fail("Failed to throw a forbidden or unauthorised exception") except (Forbidden, Unauthorized): pass diff --git a/tests/app/main/test_placeholder_form.py b/tests/app/main/test_placeholder_form.py index 4c6093944..af1a7b57b 100644 --- a/tests/app/main/test_placeholder_form.py +++ b/tests/app/main/test_placeholder_form.py @@ -1,6 +1,5 @@ import pytest from app.main.forms import get_placeholder_form_instance -from wtforms import Label def test_form_class_not_mutated(app_): @@ -8,7 +7,7 @@ def test_form_class_not_mutated(app_): with app_.test_request_context( method='POST', data={'placeholder_value': ''} - ) as req: + ): form1 = get_placeholder_form_instance('name', {}, optional_placeholder=False) form2 = get_placeholder_form_instance('city', {}, optional_placeholder=True) diff --git a/tests/app/main/test_two_factor_form.py b/tests/app/main/test_two_factor_form.py index 7a3a18d0b..b0fd63e38 100644 --- a/tests/app/main/test_two_factor_form.py +++ b/tests/app/main/test_two_factor_form.py @@ -6,8 +6,10 @@ def test_form_is_valid_returns_no_errors( app_, mock_check_verify_code, ): - with app_.test_request_context(method='POST', - data={'sms_code': '12345'}) as req: + with app_.test_request_context( + method='POST', + data={'sms_code': '12345'} + ): def _check_code(code): return user_api_client.check_verify_code('1', code, "sms") form = TwoFactorForm(_check_code) @@ -19,8 +21,10 @@ def test_returns_errors_when_code_is_too_short( app_, mock_check_verify_code, ): - with app_.test_request_context(method='POST', - data={'sms_code': '145'}) as req: + with app_.test_request_context( + method='POST', + data={'sms_code': '145'} + ): def _check_code(code): return user_api_client.check_verify_code('1', code, "sms") form = TwoFactorForm(_check_code) @@ -33,8 +37,10 @@ def test_returns_errors_when_code_is_missing( app_, mock_check_verify_code, ): - with app_.test_request_context(method='POST', - data={}) as req: + with app_.test_request_context( + method='POST', + data={} + ): def _check_code(code): return user_api_client.check_verify_code('1', code, "sms") form = TwoFactorForm(_check_code) @@ -47,8 +53,10 @@ def test_returns_errors_when_code_contains_letters( app_, mock_check_verify_code, ): - with app_.test_request_context(method='POST', - data={'sms_code': 'asdfg'}) as req: + with app_.test_request_context( + method='POST', + data={'sms_code': 'asdfg'} + ): def _check_code(code): return user_api_client.check_verify_code('1', code, "sms") form = TwoFactorForm(_check_code) @@ -61,8 +69,10 @@ def test_should_return_errors_when_code_is_expired( app_, mock_check_verify_code_code_expired, ): - with app_.test_request_context(method='POST', - data={'sms_code': '23456'}) as req: + with app_.test_request_context( + method='POST', + data={'sms_code': '23456'} + ): def _check_code(code): return user_api_client.check_verify_code('1', code, "sms") form = TwoFactorForm(_check_code) diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index 6a268ccb3..aa7b14da7 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -353,4 +353,4 @@ def test_new_invited_user_verifies_and_added_to_service( raw_html = response.data.decode('utf-8') page = BeautifulSoup(raw_html, 'html.parser') - element = page.find('h2').text == 'Trial mode' + assert page.find('h1').text == 'Dashboard' diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index 91d08e61d..a9b5957b9 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -7,15 +7,12 @@ from flask import url_for from bs4 import BeautifulSoup from app.main.views.jobs import get_time_left, get_status_filters -from tests import notification_json from tests.conftest import ( SERVICE_ONE_ID, mock_get_notifications, normalize_spaces, - mock_get_notifications ) from freezegun import freeze_time -from datetime import datetime @pytest.mark.parametrize( @@ -99,9 +96,6 @@ def test_can_show_notifications( )) assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - content = response.get_data(as_text=True) - notifications = notification_json(service_one['id']) - notification = notifications['notifications'][0] text_of_first_row = page.select('tbody tr')[0].text assert '07123456789' in text_of_first_row assert ( @@ -334,7 +328,7 @@ def test_redacts_templates_that_should_be_redacted( active_user_with_permissions, mock_get_detailed_service, ): - _notifications_mock = mock_get_notifications( + mock_get_notifications( mocker, active_user_with_permissions, template_content="hello ((name))", @@ -370,12 +364,12 @@ def test_big_numbers_and_search_dont_show_for_letters( search_bar_visible ): page = client_request.get( - 'main.view_notifications', - service_id=service_one['id'], - message_type=message_type, - status='', - page=1, - ) + 'main.view_notifications', + service_id=service_one['id'], + message_type=message_type, + status='', + page=1, + ) assert (len(page.select("[role=tablist]")) > 0) == tablist_visible assert (len(page.select("[type=search]")) > 0) == search_bar_visible @@ -401,10 +395,10 @@ def test_sending_status_hint_does_not_include_status_for_letters( mock_get_notifications(mocker, True, diff_template_type=message_type) page = client_request.get( - 'main.view_notifications', - service_id=service_one['id'], - message_type=message_type - ) + 'main.view_notifications', + service_id=service_one['id'], + message_type=message_type + ) if message_type == 'letter': assert normalize_spaces(page.select(".align-with-message-body")[0].text) == "27 September at 5:30pm" diff --git a/tests/app/main/views/test_api_keys.py b/tests/app/main/views/test_api_keys.py index d8beeae36..3dc08d962 100644 --- a/tests/app/main/views/test_api_keys.py +++ b/tests/app/main/views/test_api_keys.py @@ -351,7 +351,7 @@ def test_should_update_whitelist( ('phone_numbers-2', '+1800-555-555'), ]) - response = logged_in_client.post( + logged_in_client.post( url_for('main.whitelist', service_id=service_id), data=data ) diff --git a/tests/app/main/views/test_conversation.py b/tests/app/main/views/test_conversation.py index 1da8caf3b..bc5e2eabd 100644 --- a/tests/app/main/views/test_conversation.py +++ b/tests/app/main/views/test_conversation.py @@ -2,7 +2,6 @@ from datetime import datetime import json import pytest -from bs4 import BeautifulSoup from flask import ( url_for, ) @@ -214,6 +213,7 @@ def test_view_conversation_with_empty_inbound( messages = page.select('.sms-message-wrapper') assert len(messages) == 1 + assert mock_get_inbound_sms.called is True def test_conversation_links_to_reply( diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 15f2cfd52..35a68dea4 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -51,8 +51,10 @@ def test_get_started( mock_get_usage, mock_get_inbound_sms_summary ): - mock_template_stats = mocker.patch('app.template_statistics_client.get_template_statistics_for_service', - return_value=copy.deepcopy(stub_template_stats)) + mocker.patch( + 'app.template_statistics_client.get_template_statistics_for_service', + return_value=copy.deepcopy(stub_template_stats) + ) response = logged_in_client.get(url_for('main.service_dashboard', service_id=SERVICE_ONE_ID)) @@ -70,8 +72,10 @@ def test_get_started_is_hidden_once_templates_exist( mock_get_usage, mock_get_inbound_sms_summary ): - mock_template_stats = mocker.patch('app.template_statistics_client.get_template_statistics_for_service', - return_value=copy.deepcopy(stub_template_stats)) + mocker.patch( + 'app.template_statistics_client.get_template_statistics_for_service', + return_value=copy.deepcopy(stub_template_stats) + ) response = logged_in_client.get(url_for('main.service_dashboard', service_id=SERVICE_ONE_ID)) # mock_get_service_templates.assert_called_once_with(SERVICE_ONE_ID) diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 7595c61f9..43a9c784e 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -1,13 +1,10 @@ import json -import uuid -from urllib.parse import urlparse, quote, parse_qs import pytest from flask import url_for from bs4 import BeautifulSoup -from app.main.views.jobs import get_time_left, get_status_filters -from tests import notification_json +from app.main.views.jobs import get_time_left from tests.conftest import SERVICE_ONE_ID, normalize_spaces, mock_get_notifications from freezegun import freeze_time diff --git a/tests/app/main/views/test_letters.py b/tests/app/main/views/test_letters.py index 3ade48787..1e2fc50e2 100644 --- a/tests/app/main/views/test_letters.py +++ b/tests/app/main/views/test_letters.py @@ -3,8 +3,6 @@ from bs4 import BeautifulSoup from flask import url_for from functools import partial -from tests import service_json - letters_urls = [ partial(url_for, 'main.add_service_template', template_type='letter'), ] diff --git a/tests/app/main/views/test_notifications.py b/tests/app/main/views/test_notifications.py index 0849a3f19..d86507872 100644 --- a/tests/app/main/views/test_notifications.py +++ b/tests/app/main/views/test_notifications.py @@ -2,13 +2,6 @@ from freezegun import freeze_time from flask import url_for import pytest -from app.utils import ( - REQUESTED_STATUSES, - FAILURE_STATUSES, - SENDING_STATUSES, - DELIVERED_STATUSES, -) - from notifications_utils.template import LetterImageTemplate from tests.conftest import mock_get_notification, SERVICE_ONE_ID, normalize_spaces diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index 2fd7c4356..a9e278bd7 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -239,14 +239,14 @@ def test_create_global_stats_sets_failure_rates(fake_uuid): service_json(fake_uuid, 'b', []) ] services[0]['statistics'] = create_stats( - emails_requested=1, - emails_delivered=1, - emails_failed=0, + emails_requested=1, + emails_delivered=1, + emails_failed=0, ) services[1]['statistics'] = create_stats( - emails_requested=2, - emails_delivered=1, - emails_failed=1, + emails_requested=2, + emails_delivered=1, + emails_failed=1, ) stats = create_global_stats(services) diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 2dfe09243..82c1a0ef5 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -1,6 +1,5 @@ # -*- coding: utf-8 -*- import uuid -from unittest.mock import Mock from io import BytesIO from os import path from glob import glob @@ -25,14 +24,9 @@ from tests.conftest import ( mock_get_service_email_template, normalize_spaces, SERVICE_ONE_ID, - mock_get_service, mock_get_live_service, multiple_reply_to_email_addresses, - multiple_letter_contact_blocks, - multiple_sms_senders, no_reply_to_email_addresses, - no_letter_contact_blocks, - no_sms_senders ) template_types = ['email', 'sms'] @@ -99,7 +93,7 @@ def test_sender_session_is_present_after_selected( mock_get_service_email_template, multiple_reply_to_email_addresses ): - response = logged_in_client.post( + logged_in_client.post( url_for('.set_sender', service_id=service_one['id'], template_id=fake_uuid), data={'sender': '1234'} ) @@ -867,7 +861,7 @@ def test_send_test_caches_page_count( mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=99) - response = logged_in_client.get( + logged_in_client.get( url_for( 'main.send_test', service_id=service_one['id'], @@ -1364,8 +1358,6 @@ def test_check_messages_should_revalidate_file_when_uploading_file( fake_uuid ): - service_id = service_one['id'] - mocker.patch( 'app.main.views.send.s3download', return_value=""" @@ -1374,14 +1366,14 @@ def test_check_messages_should_revalidate_file_when_uploading_file( +447700900986,,,, """ ) - data = mock_get_job(service_one['id'], fake_uuid)['data'] + data = mock_get_job(SERVICE_ONE_ID, fake_uuid)['data'] with logged_in_client.session_transaction() as session: session['upload_data'] = {'original_file_name': 'invalid.csv', 'template_id': data['template'], 'notification_count': data['notification_count'], 'valid': True} response = logged_in_client.post( - url_for('main.start_job', service_id=service_one['id'], upload_id=data['id']), + url_for('main.start_job', service_id=SERVICE_ONE_ID, upload_id=data['id']), data={'file': (BytesIO(''.encode('utf-8')), 'invalid.csv')}, content_type='multipart/form-data', follow_redirects=True @@ -1448,18 +1440,19 @@ def test_route_permissions_send_check_notifications( session['recipient'] = '07700900001' session['placeholders'] = {'name': 'a'} validate_route_permission_with_client( - mocker, - client, - method, - response_code, - url_for( - route, - service_id=service_one['id'], - template_id=fake_uuid - ), - ['send_texts', 'send_emails', 'send_letters'], - api_user_active, - service_one) + mocker, + client, + method, + response_code, + url_for( + route, + service_id=service_one['id'], + template_id=fake_uuid + ), + ['send_texts', 'send_emails', 'send_letters'], + api_user_active, + service_one + ) @pytest.mark.parametrize('route', [ @@ -1617,7 +1610,7 @@ def test_check_messages_shows_too_many_messages_errors( ): # csv with 100 phone numbers mocker.patch('app.main.views.send.s3download', return_value=',\n'.join( - ['phone number'] + ([mock_get_users_by_service(None)[0]._mobile_number]*100) + ['phone number'] + ([mock_get_users_by_service(None)[0]._mobile_number] * 100) )) mocker.patch('app.service_api_client.get_detailed_service_for_today', return_value={ 'data': { @@ -1906,10 +1899,10 @@ def test_non_ascii_characters_in_letter_recipients_file_shows_error( assert ' '.join( page.find('div', class_='banner-dangerous').text.split() ) == ( - 'There is a problem with unicode.csv ' - 'You need to fix 1 address ' - 'Skip to file contents' - ) + 'There is a problem with unicode.csv ' + 'You need to fix 1 address ' + 'Skip to file contents' + ) assert page.find('span', class_='table-field-error-label').text == u'Can’t include П, е, т or я' diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 436c310c7..b8cf44e9c 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -15,17 +15,15 @@ from tests.conftest import ( platform_admin_user, normalize_spaces, no_reply_to_email_addresses, - single_reply_to_email_address, multiple_reply_to_email_addresses, multiple_letter_contact_blocks, - no_reply_to_email_addresses, no_letter_contact_blocks, get_default_reply_to_email_address, get_non_default_reply_to_email_address, get_default_letter_contact_block, get_non_default_letter_contact_block, SERVICE_ONE_ID - ) +) @pytest.mark.parametrize('user, expected_rows', [ @@ -169,23 +167,24 @@ def test_should_show_overview_for_service_with_more_things_set( ('https://test.url.com', 'https://test.url.com'), ]) def test_service_settings_show_elided_api_url_if_needed( - logged_in_platform_admin_client, - service_one, - mock_get_letter_organisations, - single_reply_to_email_address, - single_letter_contact_block, - mocker, - fake_uuid, - url, - elided_url, - mock_get_inbound_number_for_service + logged_in_platform_admin_client, + service_one, + mock_get_letter_organisations, + single_reply_to_email_address, + single_letter_contact_block, + mocker, + fake_uuid, + url, + elided_url, + mock_get_inbound_number_for_service, ): service_one['permissions'] = ['sms', 'email', 'inbound_sms'] service_one['inbound_api'] = [fake_uuid] mocked_get_fn = mocker.patch( 'app.service_api_client.get', - return_value={'data': {'id': fake_uuid, 'url': url}}) + return_value={'data': {'id': fake_uuid, 'url': url}} + ) response = logged_in_platform_admin_client.get( url_for( @@ -200,6 +199,7 @@ def test_service_settings_show_elided_api_url_if_needed( api_url = [api_setting[1].text.strip() for api_setting in non_empty_trs if api_setting[0].text.strip() == 'API endpoint for received text messages'][0] assert api_url == elided_url + assert mocked_get_fn.called is True def test_if_cant_send_letters_then_cant_see_letter_contact_block( @@ -871,7 +871,7 @@ def test_add_reply_to_email_address( ): fixture(mocker) data['email_address'] = "test@example.gov.uk" - page = client_request.post( + client_request.post( 'main.service_add_email_reply_to', service_id=SERVICE_ONE_ID, _data=data @@ -899,7 +899,7 @@ def test_add_letter_contact( ): fixture(mocker) data['letter_contact_block'] = "1 Example Street" - page = client_request.post( + client_request.post( 'main.service_add_letter_contact', service_id=SERVICE_ONE_ID, _data=data @@ -951,7 +951,7 @@ def test_edit_reply_to_email_address( ): fixture(mocker) data['email_address'] = "test@example.gov.uk" - page = client_request.post( + client_request.post( 'main.service_edit_email_reply_to', service_id=SERVICE_ONE_ID, reply_to_email_id=fake_uuid, @@ -983,7 +983,7 @@ def test_edit_letter_contact_block( ): fixture(mocker) data['letter_contact_block'] = "1 Example Street" - page = client_request.post( + client_request.post( 'main.service_edit_letter_contact', service_id=SERVICE_ONE_ID, letter_contact_id=fake_uuid, @@ -1058,10 +1058,10 @@ def test_default_box_shows_on_non_default_sender_details_while_editing( def test_switch_service_to_research_mode( - logged_in_platform_admin_client, - platform_admin_user, - service_one, - mocker, + logged_in_platform_admin_client, + platform_admin_user, + service_one, + mocker, ): mocker.patch('app.service_api_client.post', return_value=service_one) response = logged_in_platform_admin_client.get( @@ -1079,8 +1079,8 @@ def test_switch_service_to_research_mode( def test_switch_service_from_research_mode_to_normal( - logged_in_platform_admin_client, - mocker, + logged_in_platform_admin_client, + mocker, ): service = service_json( research_mode=True @@ -1099,13 +1099,13 @@ def test_switch_service_from_research_mode_to_normal( def test_shows_research_mode_indicator( - logged_in_client, - service_one, - mocker, - mock_get_letter_organisations, - single_reply_to_email_address, - single_letter_contact_block, - mock_get_inbound_number_for_service + logged_in_client, + service_one, + mocker, + mock_get_letter_organisations, + single_reply_to_email_address, + single_letter_contact_block, + mock_get_inbound_number_for_service, ): service_one['research_mode'] = True mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one) @@ -1119,12 +1119,12 @@ def test_shows_research_mode_indicator( def test_does_not_show_research_mode_indicator( - logged_in_client, - service_one, - mock_get_letter_organisations, - single_reply_to_email_address, - single_letter_contact_block, - mock_get_inbound_number_for_service + logged_in_client, + service_one, + mock_get_letter_organisations, + single_reply_to_email_address, + single_letter_contact_block, + mock_get_inbound_number_for_service, ): response = logged_in_client.get(url_for('main.service_settings', service_id=service_one['id'])) assert response.status_code == 200 @@ -1140,13 +1140,13 @@ def test_does_not_show_research_mode_indicator( ("https://test.com", "123456789", "Must be at least 10 characters"), ]) def test_set_inbound_api_validation( - logged_in_client, - mock_update_service, - service_one, - mock_get_letter_organisations, - url, - bearer_token, - expected_errors, + logged_in_client, + mock_update_service, + service_one, + mock_get_letter_organisations, + url, + bearer_token, + expected_errors, ): service_one['permissions'] = ['inbound_sms'] response = logged_in_client.post(url_for( @@ -1164,9 +1164,9 @@ def test_set_inbound_api_validation( @pytest.mark.parametrize('method', ['get', 'post']) def test_cant_set_letter_contact_block_if_service_cant_send_letters( - logged_in_client, - service_one, - method + logged_in_client, + service_one, + method, ): assert 'letter' not in service_one['permissions'] response = getattr(logged_in_client, method)( @@ -1176,8 +1176,8 @@ def test_cant_set_letter_contact_block_if_service_cant_send_letters( def test_set_letter_contact_block_prepopulates( - logged_in_client, - service_one + logged_in_client, + service_one, ): service_one['permissions'] = ['letter'] service_one['letter_contact_block'] = 'foo bar baz waz' @@ -1187,9 +1187,9 @@ def test_set_letter_contact_block_prepopulates( def test_set_letter_contact_block_saves( - logged_in_client, - service_one, - mock_update_service, + logged_in_client, + service_one, + mock_update_service, ): service_one['permissions'] = ['letter'] response = logged_in_client.post( @@ -1226,9 +1226,9 @@ def test_set_letter_contact_block_redirects_to_template( def test_set_letter_contact_block_has_max_10_lines( - logged_in_client, - service_one, - mock_update_service, + logged_in_client, + service_one, + mock_update_service, ): service_one['permissions'] = ['letter'] response = logged_in_client.post( @@ -1242,8 +1242,8 @@ def test_set_letter_contact_block_has_max_10_lines( def test_set_letter_branding_platform_admin_only( - logged_in_client, - service_one, + logged_in_client, + service_one, ): response = logged_in_client.get(url_for('main.set_letter_branding', service_id=service_one['id'])) assert response.status_code == 403 @@ -1254,11 +1254,11 @@ def test_set_letter_branding_platform_admin_only( ('500', '500'), ]) def test_set_letter_branding_prepopulates( - logged_in_platform_admin_client, - service_one, - mock_get_letter_organisations, - current_dvla_org_id, - expected_selected, + logged_in_platform_admin_client, + service_one, + mock_get_letter_organisations, + current_dvla_org_id, + expected_selected, ): if current_dvla_org_id: service_one['dvla_organisation'] = current_dvla_org_id @@ -1268,11 +1268,11 @@ def test_set_letter_branding_prepopulates( assert page.select('input[checked]')[0]['value'] == expected_selected -def test_set_letter_contact_block_saves( - logged_in_platform_admin_client, - service_one, - mock_update_service, - mock_get_letter_organisations, +def test_set_letter_branding_saves( + logged_in_platform_admin_client, + service_one, + mock_update_service, + mock_get_letter_organisations, ): response = logged_in_platform_admin_client.post( url_for('main.set_letter_branding', service_id=service_one['id']), @@ -1284,10 +1284,10 @@ def test_set_letter_contact_block_saves( def test_should_show_branding( - logged_in_platform_admin_client, - service_one, - mock_get_organisations, - mock_get_letter_organisations, + logged_in_platform_admin_client, + service_one, + mock_get_organisations, + mock_get_letter_organisations, ): response = logged_in_platform_admin_client.get(url_for( 'main.service_set_branding_and_org', service_id=service_one['id'] @@ -1310,9 +1310,9 @@ def test_should_show_branding( def test_should_show_organisations( - logged_in_platform_admin_client, - service_one, - mock_get_organisations + logged_in_platform_admin_client, + service_one, + mock_get_organisations, ): response = logged_in_platform_admin_client.get(url_for( 'main.service_set_branding_and_org', service_id=service_one['id'] @@ -1335,10 +1335,10 @@ def test_should_show_organisations( def test_should_set_branding_and_organisations( - logged_in_platform_admin_client, - service_one, - mock_get_organisations, - mock_update_service, + logged_in_platform_admin_client, + service_one, + mock_get_organisations, + mock_update_service, ): response = logged_in_platform_admin_client.post( url_for( @@ -1361,9 +1361,9 @@ def test_should_set_branding_and_organisations( def test_switch_service_enable_letters( - logged_in_platform_admin_client, - service_one, - mocker, + logged_in_platform_admin_client, + service_one, + mocker, ): mocked_fn = mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one) @@ -1378,9 +1378,9 @@ def test_switch_service_enable_letters( def test_switch_service_disable_letters( - logged_in_platform_admin_client, - service_one, - mocker, + logged_in_platform_admin_client, + service_one, + mocker, ): service_one['permissions'] = ['letter'] mocked_fn = mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one) @@ -1430,7 +1430,7 @@ def test_switch_service_enable_international_sms( international_sms_permission_expected_in_api_call, ): mocked_fn = mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one) - page = client_request.post( + client_request.post( 'main.service_set_international_sms', service_id=service_one['id'], _data={ @@ -1448,9 +1448,9 @@ def test_switch_service_enable_international_sms( def test_set_new_inbound_api_and_valid_bearer_token_calls_create_inbound_api_endpoint( - logged_in_platform_admin_client, - service_one, - mocker, + logged_in_platform_admin_client, + service_one, + mocker, ): service_one['permissions'] = ['inbound_sms'] service_one['inbound_api'] = [] @@ -1481,11 +1481,11 @@ def test_set_new_inbound_api_and_valid_bearer_token_calls_create_inbound_api_end ] ) def test_update_inbound_api_and_valid_bearer_token_calls_update_inbound_api_endpoint( - logged_in_platform_admin_client, - service_one, - mocker, - fake_uuid, - inbound_api_data + logged_in_platform_admin_client, + service_one, + mocker, + fake_uuid, + inbound_api_data, ): service_one['permissions'] = ['inbound_sms'] service_one['inbound_api'] = [fake_uuid] @@ -1515,7 +1515,8 @@ def test_update_inbound_api_and_valid_bearer_token_calls_update_inbound_api_endp ) assert response.status_code == 302 assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True) - assert mocked_post_fn.called + assert mocked_get_fn.called is True + assert mocked_post_fn.called is True if inbound_api_data['bearer_token'] == dummy_bearer_token: del inbound_api_data['bearer_token'] @@ -1526,10 +1527,10 @@ def test_update_inbound_api_and_valid_bearer_token_calls_update_inbound_api_endp def test_save_inbound_api_without_changes_does_not_update_inbound_api( - logged_in_platform_admin_client, - service_one, - mocker, - fake_uuid + logged_in_platform_admin_client, + service_one, + mocker, + fake_uuid, ): service_one['permissions'] = ['inbound_sms'] service_one['inbound_api'] = [fake_uuid] @@ -1549,14 +1550,15 @@ def test_save_inbound_api_without_changes_does_not_update_inbound_api( ) assert response.status_code == 302 assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True) + assert mocked_get_fn.called is True assert mocked_post_fn.called is False def test_archive_service_after_confirm( - logged_in_platform_admin_client, - service_one, - mocker, - mock_get_inbound_number_for_service + logged_in_platform_admin_client, + service_one, + mocker, + mock_get_inbound_number_for_service, ): mocked_fn = mocker.patch('app.service_api_client.post', return_value=service_one) @@ -1568,13 +1570,13 @@ def test_archive_service_after_confirm( def test_archive_service_prompts_user( - logged_in_platform_admin_client, - service_one, - mocker, - single_reply_to_email_address, - single_letter_contact_block, - mock_get_letter_organisations, - mock_get_inbound_number_for_service + logged_in_platform_admin_client, + service_one, + mocker, + single_reply_to_email_address, + single_letter_contact_block, + mock_get_letter_organisations, + mock_get_inbound_number_for_service, ): mocked_fn = mocker.patch('app.service_api_client.post') @@ -1587,12 +1589,12 @@ def test_archive_service_prompts_user( def test_cant_archive_inactive_service( - logged_in_platform_admin_client, - service_one, - single_reply_to_email_address, - single_letter_contact_block, - mock_get_letter_organisations, - mock_get_inbound_number_for_service + logged_in_platform_admin_client, + service_one, + single_reply_to_email_address, + single_letter_contact_block, + mock_get_letter_organisations, + mock_get_inbound_number_for_service, ): service_one['active'] = False @@ -1604,10 +1606,10 @@ def test_cant_archive_inactive_service( def test_suspend_service_after_confirm( - logged_in_platform_admin_client, - service_one, - mocker, - mock_get_inbound_number_for_service + logged_in_platform_admin_client, + service_one, + mocker, + mock_get_inbound_number_for_service, ): mocked_fn = mocker.patch('app.service_api_client.post', return_value=service_one) @@ -1619,13 +1621,13 @@ def test_suspend_service_after_confirm( def test_suspend_service_prompts_user( - logged_in_platform_admin_client, - service_one, - mocker, - single_reply_to_email_address, - single_letter_contact_block, - mock_get_letter_organisations, - mock_get_inbound_number_for_service + logged_in_platform_admin_client, + service_one, + mocker, + single_reply_to_email_address, + single_letter_contact_block, + mock_get_letter_organisations, + mock_get_inbound_number_for_service, ): mocked_fn = mocker.patch('app.service_api_client.post') @@ -1639,12 +1641,12 @@ def test_suspend_service_prompts_user( def test_cant_suspend_inactive_service( - logged_in_platform_admin_client, - service_one, - single_reply_to_email_address, - single_letter_contact_block, - mock_get_letter_organisations, - mock_get_inbound_number_for_service + logged_in_platform_admin_client, + service_one, + single_reply_to_email_address, + single_letter_contact_block, + mock_get_letter_organisations, + mock_get_inbound_number_for_service, ): service_one['active'] = False @@ -1656,12 +1658,12 @@ def test_cant_suspend_inactive_service( def test_resume_service_after_confirm( - logged_in_platform_admin_client, - service_one, - single_reply_to_email_address, - single_letter_contact_block, - mocker, - mock_get_inbound_number_for_service + logged_in_platform_admin_client, + service_one, + single_reply_to_email_address, + single_letter_contact_block, + mocker, + mock_get_inbound_number_for_service, ): service_one['active'] = False mocked_fn = mocker.patch('app.service_api_client.post', return_value=service_one) @@ -1674,13 +1676,13 @@ def test_resume_service_after_confirm( def test_resume_service_prompts_user( - logged_in_platform_admin_client, - service_one, - single_reply_to_email_address, - single_letter_contact_block, - mocker, - mock_get_letter_organisations, - mock_get_inbound_number_for_service + logged_in_platform_admin_client, + service_one, + single_reply_to_email_address, + single_letter_contact_block, + mocker, + mock_get_letter_organisations, + mock_get_inbound_number_for_service, ): service_one['active'] = False mocked_fn = mocker.patch('app.service_api_client.post') @@ -1695,12 +1697,12 @@ def test_resume_service_prompts_user( def test_cant_resume_active_service( - logged_in_platform_admin_client, - service_one, - single_reply_to_email_address, - single_letter_contact_block, - mock_get_letter_organisations, - mock_get_inbound_number_for_service + logged_in_platform_admin_client, + service_one, + single_reply_to_email_address, + single_letter_contact_block, + mock_get_letter_organisations, + mock_get_inbound_number_for_service, ): response = logged_in_platform_admin_client.get(url_for('main.service_settings', service_id=service_one['id'])) diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index d6c258bdf..8a11e6283 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -1,6 +1,5 @@ from datetime import datetime from unittest.mock import Mock, ANY -import uuid import pytest from bs4 import BeautifulSoup @@ -16,7 +15,6 @@ from tests.conftest import ( SERVICE_ONE_ID, active_user_with_permissions, platform_admin_user, - mock_get_user, ) from tests import validate_route_permission, template_json, single_notification_json diff --git a/tests/app/main/views/test_two_factor.py b/tests/app/main/views/test_two_factor.py index 88f7798e1..f325fa8fc 100644 --- a/tests/app/main/views/test_two_factor.py +++ b/tests/app/main/views/test_two_factor.py @@ -64,7 +64,7 @@ def test_should_login_user_and_should_redirect_to_next_url( 'main.service_dashboard', service_id=SERVICE_ONE_ID, _external=True - ) + ) def test_should_login_user_and_not_redirect_to_external_url( diff --git a/tests/app/notify_client/test_notification_client.py b/tests/app/notify_client/test_notification_client.py index 3cf09346c..963b2686e 100644 --- a/tests/app/notify_client/test_notification_client.py +++ b/tests/app/notify_client/test_notification_client.py @@ -76,7 +76,7 @@ def test_get_api_notifications_changes_letter_statuses(mocker): notis = notification_json(service_id=service_id, rows=0) notis['notifications'] = [sms_notification, email_notification, letter_notification] - mock_post = mocker.patch('app.notify_client.notification_api_client.NotificationApiClient.get', return_value=notis) + mocker.patch('app.notify_client.notification_api_client.NotificationApiClient.get', return_value=notis) ret = NotificationApiClient().get_api_notifications_for_service(service_id)