From edb0b8214d9fce3ab4da5970f116bea52bac0f5f Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Mon, 14 Jun 2021 11:00:05 +0100 Subject: [PATCH 1/6] Move pagination utils into own module This continues the pattern established in [1], just to chip away a bit more at the random collection of stuff in utils/__init__.py. [1]: https://github.com/alphagov/notifications-admin/pull/3923 --- app/main/views/dashboard.py | 3 +-- app/main/views/jobs.py | 13 ++++++------- app/main/views/platform_admin.py | 4 ++-- app/main/views/uploads.py | 12 ++++++------ app/utils/__init__.py | 26 -------------------------- app/utils/pagination.py | 27 +++++++++++++++++++++++++++ tests/app/test_utils.py | 21 --------------------- tests/app/utils/test_pagination.py | 20 ++++++++++++++++++++ 8 files changed, 62 insertions(+), 64 deletions(-) create mode 100644 app/utils/pagination.py create mode 100644 tests/app/utils/test_pagination.py diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index fb8b56ae8..f5a64d213 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -30,12 +30,11 @@ from app.utils import ( DELIVERED_STATUSES, FAILURE_STATUSES, REQUESTED_STATUSES, - generate_next_dict, - generate_previous_dict, get_current_financial_year, service_has_permission, ) from app.utils.csv import Spreadsheet +from app.utils.pagination import generate_next_dict, generate_previous_dict from app.utils.user import user_has_permissions diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 66952030f..fe86ad1c4 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -33,18 +33,17 @@ from app.formatters import get_time_left, message_count_noun from app.main import main from app.main.forms import SearchNotificationsForm from app.models.job import Job -from app.utils import ( - generate_next_dict, - generate_previous_dict, - get_page_from_request, - parse_filter_args, - set_status_filters, -) +from app.utils import parse_filter_args, set_status_filters from app.utils.csv import generate_notifications_csv from app.utils.letters import ( get_letter_printing_statement, printing_today_or_tomorrow, ) +from app.utils.pagination import ( + generate_next_dict, + generate_previous_dict, + get_page_from_request, +) from app.utils.user import user_has_permissions diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index 5ce4dd937..229bd168b 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -27,12 +27,12 @@ from app.statistics_utils import ( get_formatted_percentage, get_formatted_percentage_two_dp, ) -from app.utils import ( +from app.utils.csv import Spreadsheet +from app.utils.pagination import ( generate_next_dict, generate_previous_dict, get_page_from_request, ) -from app.utils.csv import Spreadsheet from app.utils.user import user_is_platform_admin COMPLAINT_THRESHOLD = 0.02 diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index 628009e6c..3ef0666b1 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -45,17 +45,17 @@ from app.s3_client.s3_letter_upload_client import ( upload_letter_to_s3, ) from app.template_previews import TemplatePreview, sanitise_letter -from app.utils import ( - generate_next_dict, - generate_previous_dict, - get_page_from_request, - unicode_truncate, -) +from app.utils import unicode_truncate from app.utils.csv import Spreadsheet, get_errors_for_csv from app.utils.letters import ( get_letter_printing_statement, get_letter_validation_error, ) +from app.utils.pagination import ( + generate_next_dict, + generate_previous_dict, + get_page_from_request, +) from app.utils.templates import get_sample_template, get_template from app.utils.user import user_has_permissions diff --git a/app/utils/__init__.py b/app/utils/__init__.py index 3a2e27298..02359c820 100644 --- a/app/utils/__init__.py +++ b/app/utils/__init__.py @@ -55,32 +55,6 @@ def redirect_to_sign_in(f): return wrapped -def get_page_from_request(): - if 'page' in request.args: - try: - return int(request.args['page']) - except ValueError: - return None - else: - return 1 - - -def generate_previous_dict(view, service_id, page, url_args=None): - return generate_previous_next_dict(view, service_id, page - 1, 'Previous page', url_args or {}) - - -def generate_next_dict(view, service_id, page, url_args=None): - return generate_previous_next_dict(view, service_id, page + 1, 'Next page', url_args or {}) - - -def generate_previous_next_dict(view, service_id, page, title, url_args): - return { - 'url': url_for(view, service_id=service_id, page=page, **url_args), - 'title': title, - 'label': 'page {}'.format(page) - } - - def get_help_argument(): return request.args.get('help') if request.args.get('help') in ('1', '2', '3') else None diff --git a/app/utils/pagination.py b/app/utils/pagination.py new file mode 100644 index 000000000..b6c8bf53b --- /dev/null +++ b/app/utils/pagination.py @@ -0,0 +1,27 @@ +from flask import request, url_for + + +def get_page_from_request(): + if 'page' in request.args: + try: + return int(request.args['page']) + except ValueError: + return None + else: + return 1 + + +def generate_previous_dict(view, service_id, page, url_args=None): + return generate_previous_next_dict(view, service_id, page - 1, 'Previous page', url_args or {}) + + +def generate_next_dict(view, service_id, page, url_args=None): + return generate_previous_next_dict(view, service_id, page + 1, 'Next page', url_args or {}) + + +def generate_previous_next_dict(view, service_id, page, title, url_args): + return { + 'url': url_for(view, service_id=service_id, page=page, **url_args), + 'title': title, + 'label': 'page {}'.format(page) + } diff --git a/tests/app/test_utils.py b/tests/app/test_utils.py index dd0b8da74..68536b22e 100644 --- a/tests/app/test_utils.py +++ b/tests/app/test_utils.py @@ -4,8 +4,6 @@ from freezegun import freeze_time from app import format_datetime_relative from app.formatters import email_safe, round_to_significant_figures from app.utils import ( - generate_next_dict, - generate_previous_dict, get_current_financial_year, get_logo_cdn_domain, is_less_than_days_ago, @@ -29,25 +27,6 @@ def test_email_safe_return_dot_separated_email_domain(service_name, safe_email): assert email_safe(service_name) == safe_email -def test_generate_previous_dict(client): - ret = generate_previous_dict('main.view_jobs', 'foo', 2, {}) - assert 'page=1' in ret['url'] - assert ret['title'] == 'Previous page' - assert ret['label'] == 'page 1' - - -def test_generate_next_dict(client): - ret = generate_next_dict('main.view_jobs', 'foo', 2, {}) - assert 'page=3' in ret['url'] - assert ret['title'] == 'Next page' - assert ret['label'] == 'page 3' - - -def test_generate_previous_next_dict_adds_other_url_args(client): - ret = generate_next_dict('main.view_notifications', 'foo', 2, {'message_type': 'blah'}) - assert 'notifications/blah' in ret['url'] - - def test_get_cdn_domain_on_localhost(client, mocker): mocker.patch.dict('app.current_app.config', values={'ADMIN_BASE_URL': 'http://localhost:6012'}) domain = get_logo_cdn_domain() diff --git a/tests/app/utils/test_pagination.py b/tests/app/utils/test_pagination.py new file mode 100644 index 000000000..444942f5e --- /dev/null +++ b/tests/app/utils/test_pagination.py @@ -0,0 +1,20 @@ +from app.utils.pagination import generate_next_dict, generate_previous_dict + + +def test_generate_previous_dict(client): + ret = generate_previous_dict('main.view_jobs', 'foo', 2, {}) + assert 'page=1' in ret['url'] + assert ret['title'] == 'Previous page' + assert ret['label'] == 'page 1' + + +def test_generate_next_dict(client): + ret = generate_next_dict('main.view_jobs', 'foo', 2, {}) + assert 'page=3' in ret['url'] + assert ret['title'] == 'Next page' + assert ret['label'] == 'page 3' + + +def test_generate_previous_next_dict_adds_other_url_args(client): + ret = generate_next_dict('main.view_notifications', 'foo', 2, {'message_type': 'blah'}) + assert 'notifications/blah' in ret['url'] From 3250f2b3edb5708f2634c3a6ff84815b71b6ad43 Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Mon, 14 Jun 2021 11:09:42 +0100 Subject: [PATCH 2/6] Move redirect_to_signin helper to new util module We'll expand this module in later commits. --- app/main/views/code_not_received.py | 2 +- app/main/views/two_factor.py | 3 ++- app/main/views/verify.py | 2 +- app/main/views/webauthn_credentials.py | 3 ++- app/utils/__init__.py | 21 +-------------------- app/utils/login.py | 13 +++++++++++++ 6 files changed, 20 insertions(+), 24 deletions(-) create mode 100644 app/utils/login.py diff --git a/app/main/views/code_not_received.py b/app/main/views/code_not_received.py index 37269a7d2..b5401a933 100644 --- a/app/main/views/code_not_received.py +++ b/app/main/views/code_not_received.py @@ -4,7 +4,7 @@ from app import user_api_client from app.main import main from app.main.forms import TextNotReceivedForm from app.models.user import User -from app.utils import redirect_to_sign_in +from app.utils.login import redirect_to_sign_in @main.route('/resend-email-verification') diff --git a/app/main/views/two_factor.py b/app/main/views/two_factor.py index 12cdb7103..8a2e2036e 100644 --- a/app/main/views/two_factor.py +++ b/app/main/views/two_factor.py @@ -16,7 +16,8 @@ from app import user_api_client from app.main import main from app.main.forms import TwoFactorForm from app.models.user import User -from app.utils import is_less_than_days_ago, redirect_to_sign_in +from app.utils import is_less_than_days_ago +from app.utils.login import redirect_to_sign_in @main.route('/two-factor-email-sent', methods=['GET']) diff --git a/app/main/views/verify.py b/app/main/views/verify.py index 87c8fa95d..fe81b2b89 100644 --- a/app/main/views/verify.py +++ b/app/main/views/verify.py @@ -17,7 +17,7 @@ from app.main import main from app.main.forms import TwoFactorForm from app.models.service import Service from app.models.user import InvitedOrgUser, InvitedUser, User -from app.utils import redirect_to_sign_in +from app.utils.login import redirect_to_sign_in @main.route('/verify', methods=['GET', 'POST']) diff --git a/app/main/views/webauthn_credentials.py b/app/main/views/webauthn_credentials.py index d1b18a6d2..953242147 100644 --- a/app/main/views/webauthn_credentials.py +++ b/app/main/views/webauthn_credentials.py @@ -10,7 +10,8 @@ from app.main.views.two_factor import log_in_user from app.models.user import User from app.models.webauthn_credential import RegistrationError, WebAuthnCredential from app.notify_client.user_api_client import user_api_client -from app.utils import is_less_than_days_ago, redirect_to_sign_in +from app.utils import is_less_than_days_ago +from app.utils.login import redirect_to_sign_in from app.utils.user import user_is_platform_admin diff --git a/app/utils/__init__.py b/app/utils/__init__.py index 02359c820..582f3403e 100644 --- a/app/utils/__init__.py +++ b/app/utils/__init__.py @@ -5,16 +5,7 @@ from urllib.parse import urlparse import pytz from dateutil import parser -from flask import ( - abort, - current_app, - g, - make_response, - redirect, - request, - session, - url_for, -) +from flask import abort, current_app, g, make_response, request from flask_login import current_user from notifications_utils.field import Field from notifications_utils.timezones import utc_string_to_aware_gmt_datetime @@ -45,16 +36,6 @@ def service_has_permission(permission): return wrap -def redirect_to_sign_in(f): - @wraps(f) - def wrapped(*args, **kwargs): - if 'user_details' not in session: - return redirect(url_for('main.sign_in')) - else: - return f(*args, **kwargs) - return wrapped - - def get_help_argument(): return request.args.get('help') if request.args.get('help') in ('1', '2', '3') else None diff --git a/app/utils/login.py b/app/utils/login.py new file mode 100644 index 000000000..c96fba71f --- /dev/null +++ b/app/utils/login.py @@ -0,0 +1,13 @@ +from functools import wraps + +from flask import redirect, session, url_for + + +def redirect_to_sign_in(f): + @wraps(f) + def wrapped(*args, **kwargs): + if 'user_details' not in session: + return redirect(url_for('main.sign_in')) + else: + return f(*args, **kwargs) + return wrapped From bf2e6802bf5f5488a4179c160ed7ce84048f6f14 Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Mon, 14 Jun 2021 11:15:57 +0100 Subject: [PATCH 3/6] Extract login utils out of two_factor view This better reflects how the code is reused in other views and is not specific to two factor actions. We have a pattern of testing utility functionality for each view (as opposed to testing the util + the view calls the util), so I'm leaving the tests as-is. --- app/main/views/new_password.py | 2 +- app/main/views/two_factor.py | 41 ++++---------------------- app/main/views/webauthn_credentials.py | 3 +- app/utils/login.py | 39 +++++++++++++++++++++++- 4 files changed, 45 insertions(+), 40 deletions(-) diff --git a/app/main/views/new_password.py b/app/main/views/new_password.py index 5167d7ae2..210496447 100644 --- a/app/main/views/new_password.py +++ b/app/main/views/new_password.py @@ -14,8 +14,8 @@ from notifications_utils.url_safe_token import check_token from app.main import main from app.main.forms import NewPasswordForm -from app.main.views.two_factor import log_in_user from app.models.user import User +from app.utils.login import log_in_user @main.route('/new-password/', methods=['GET', 'POST']) diff --git a/app/main/views/two_factor.py b/app/main/views/two_factor.py index 8a2e2036e..75cdd0d78 100644 --- a/app/main/views/two_factor.py +++ b/app/main/views/two_factor.py @@ -17,7 +17,11 @@ from app.main import main from app.main.forms import TwoFactorForm from app.models.user import User from app.utils import is_less_than_days_ago -from app.utils.login import redirect_to_sign_in +from app.utils.login import ( + log_in_user, + redirect_to_sign_in, + redirect_when_logged_in, +) @main.route('/two-factor-email-sent', methods=['GET']) @@ -97,38 +101,3 @@ def revalidate_email_sent(): title = 'Email resent' if request.args.get('email_resent') else 'Check your email' redirect_url = request.args.get('next') return render_template('views/re-validate-email-sent.html', title=title, redirect_url=redirect_url) - - -# see http://flask.pocoo.org/snippets/62/ -def _is_safe_redirect_url(target): - from urllib.parse import urljoin, urlparse - host_url = urlparse(request.host_url) - redirect_url = urlparse(urljoin(request.host_url, target)) - return redirect_url.scheme in ('http', 'https') and \ - host_url.netloc == redirect_url.netloc - - -def log_in_user(user_id): - try: - user = User.from_id(user_id) - # the user will have a new current_session_id set by the API - store it in the cookie for future requests - session['current_session_id'] = user.current_session_id - # Check if coming from new password page - if 'password' in session.get('user_details', {}): - user.update_password(session['user_details']['password'], validated_email_access=True) - user.activate() - user.login() - finally: - # get rid of anything in the session that we don't expect to have been set during register/sign in flow - session.pop("user_details", None) - session.pop("file_uploads", None) - - return redirect_when_logged_in(platform_admin=user.platform_admin) - - -def redirect_when_logged_in(platform_admin): - next_url = request.args.get('next') - if next_url and _is_safe_redirect_url(next_url): - return redirect(next_url) - - return redirect(url_for('main.show_accounts_or_dashboard')) diff --git a/app/main/views/webauthn_credentials.py b/app/main/views/webauthn_credentials.py index 953242147..008e24ada 100644 --- a/app/main/views/webauthn_credentials.py +++ b/app/main/views/webauthn_credentials.py @@ -6,12 +6,11 @@ from flask_login import current_user from werkzeug.exceptions import Forbidden from app.main import main -from app.main.views.two_factor import log_in_user from app.models.user import User from app.models.webauthn_credential import RegistrationError, WebAuthnCredential from app.notify_client.user_api_client import user_api_client from app.utils import is_less_than_days_ago -from app.utils.login import redirect_to_sign_in +from app.utils.login import log_in_user, redirect_to_sign_in from app.utils.user import user_is_platform_admin diff --git a/app/utils/login.py b/app/utils/login.py index c96fba71f..0c7be1f48 100644 --- a/app/utils/login.py +++ b/app/utils/login.py @@ -1,6 +1,8 @@ from functools import wraps -from flask import redirect, session, url_for +from flask import redirect, request, session, url_for + +from app.models.user import User def redirect_to_sign_in(f): @@ -11,3 +13,38 @@ def redirect_to_sign_in(f): else: return f(*args, **kwargs) return wrapped + + +def log_in_user(user_id): + try: + user = User.from_id(user_id) + # the user will have a new current_session_id set by the API - store it in the cookie for future requests + session['current_session_id'] = user.current_session_id + # Check if coming from new password page + if 'password' in session.get('user_details', {}): + user.update_password(session['user_details']['password'], validated_email_access=True) + user.activate() + user.login() + finally: + # get rid of anything in the session that we don't expect to have been set during register/sign in flow + session.pop("user_details", None) + session.pop("file_uploads", None) + + return redirect_when_logged_in(platform_admin=user.platform_admin) + + +def redirect_when_logged_in(platform_admin): + next_url = request.args.get('next') + if next_url and _is_safe_redirect_url(next_url): + return redirect(next_url) + + return redirect(url_for('main.show_accounts_or_dashboard')) + + +# see http://flask.pocoo.org/snippets/62/ +def _is_safe_redirect_url(target): + from urllib.parse import urljoin, urlparse + host_url = urlparse(request.host_url) + redirect_url = urlparse(urljoin(request.host_url, target)) + return redirect_url.scheme in ('http', 'https') and \ + host_url.netloc == redirect_url.netloc From c17d438de8f0da4f158b4630ff317b71749b718d Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Mon, 14 Jun 2021 12:40:12 +0100 Subject: [PATCH 4/6] DRY-up email revalidation check Previously this was duplicated between the "two_factor" and the "webauthn" views, and required more test setup. This DRYs up the check and tests it once, using mocks to simplify the view tests. As part of DRYing up the check into a util module, I've also moved the "is_less_than_days_ago" function it uses. --- app/main/views/dashboard.py | 2 +- app/main/views/two_factor.py | 8 +++--- app/main/views/webauthn_credentials.py | 9 ++++-- app/models/job.py | 3 +- app/utils/__init__.py | 19 ------------- app/utils/login.py | 5 ++++ app/utils/time.py | 20 +++++++++++++ tests/app/main/views/test_two_factor.py | 27 ++++++++---------- .../main/views/test_webauthn_credentials.py | 7 ++--- tests/app/test_utils.py | 28 +------------------ tests/app/utils/test_login.py | 19 +++++++++++++ tests/app/utils/test_time.py | 25 +++++++++++++++++ 12 files changed, 97 insertions(+), 75 deletions(-) create mode 100644 app/utils/time.py create mode 100644 tests/app/utils/test_login.py create mode 100644 tests/app/utils/test_time.py diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index f5a64d213..a398ef81c 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -30,11 +30,11 @@ from app.utils import ( DELIVERED_STATUSES, FAILURE_STATUSES, REQUESTED_STATUSES, - get_current_financial_year, service_has_permission, ) from app.utils.csv import Spreadsheet from app.utils.pagination import generate_next_dict, generate_previous_dict +from app.utils.time import get_current_financial_year from app.utils.user import user_has_permissions diff --git a/app/main/views/two_factor.py b/app/main/views/two_factor.py index 75cdd0d78..96b9b68c7 100644 --- a/app/main/views/two_factor.py +++ b/app/main/views/two_factor.py @@ -16,8 +16,8 @@ from app import user_api_client from app.main import main from app.main.forms import TwoFactorForm from app.models.user import User -from app.utils import is_less_than_days_ago from app.utils.login import ( + email_needs_revalidating, log_in_user, redirect_to_sign_in, redirect_when_logged_in, @@ -79,11 +79,11 @@ def two_factor_sms(): redirect_url = request.args.get('next') if form.validate_on_submit(): - if is_less_than_days_ago(user.email_access_validated_at, 90): - return log_in_user(user_id) - else: + if email_needs_revalidating(user): user_api_client.send_verify_code(user.id, 'email', None, redirect_url) return redirect(url_for('.revalidate_email_sent', next=redirect_url)) + else: + return log_in_user(user_id) return render_template('views/two-factor-sms.html', form=form, redirect_url=redirect_url) diff --git a/app/main/views/webauthn_credentials.py b/app/main/views/webauthn_credentials.py index 008e24ada..2161edf01 100644 --- a/app/main/views/webauthn_credentials.py +++ b/app/main/views/webauthn_credentials.py @@ -9,8 +9,11 @@ from app.main import main from app.models.user import User from app.models.webauthn_credential import RegistrationError, WebAuthnCredential from app.notify_client.user_api_client import user_api_client -from app.utils import is_less_than_days_ago -from app.utils.login import log_in_user, redirect_to_sign_in +from app.utils.login import ( + email_needs_revalidating, + log_in_user, + redirect_to_sign_in, +) from app.utils.user import user_is_platform_admin @@ -168,7 +171,7 @@ def _complete_webauthn_login_attempt(user): # user account is locked as too many failed logins abort(403) - if not is_less_than_days_ago(user.email_access_validated_at, 90): + if email_needs_revalidating(user): user_api_client.send_verify_code(user.id, 'email', None, redirect_url) return redirect(url_for('.revalidate_email_sent', next=redirect_url)) diff --git a/app/models/job.py b/app/models/job.py index 89597a5e5..7561ef777 100644 --- a/app/models/job.py +++ b/app/models/job.py @@ -13,8 +13,9 @@ from app.models import JSONModel, ModelList, PaginatedModelList from app.notify_client.job_api_client import job_api_client from app.notify_client.notification_api_client import notification_api_client from app.notify_client.service_api_client import service_api_client -from app.utils import is_less_than_days_ago, set_status_filters +from app.utils import set_status_filters from app.utils.letters import get_letter_printing_statement +from app.utils.time import is_less_than_days_ago class Job(JSONModel): diff --git a/app/utils/__init__.py b/app/utils/__init__.py index 582f3403e..c80aa9b8f 100644 --- a/app/utils/__init__.py +++ b/app/utils/__init__.py @@ -1,14 +1,10 @@ -from datetime import datetime from functools import wraps from itertools import chain from urllib.parse import urlparse -import pytz -from dateutil import parser from flask import abort, current_app, g, make_response, request from flask_login import current_user from notifications_utils.field import Field -from notifications_utils.timezones import utc_string_to_aware_gmt_datetime from orderedset._orderedset import OrderedSet from werkzeug.datastructures import MultiDict from werkzeug.routing import RequestRedirect @@ -40,15 +36,6 @@ def get_help_argument(): return request.args.get('help') if request.args.get('help') in ('1', '2', '3') else None -def get_current_financial_year(): - now = utc_string_to_aware_gmt_datetime( - datetime.utcnow() - ) - current_month = int(now.strftime('%-m')) - current_year = int(now.strftime('%Y')) - return current_year if current_month > 3 else current_year - 1 - - def get_logo_cdn_domain(): parsed_uri = urlparse(current_app.config['ADMIN_BASE_URL']) @@ -114,12 +101,6 @@ class PermanentRedirect(RequestRedirect): code = 301 -def is_less_than_days_ago(date_from_db, number_of_days): - return ( - datetime.utcnow().astimezone(pytz.utc) - parser.parse(date_from_db) - ).days < number_of_days - - def hide_from_search_engines(f): @wraps(f) def decorated_function(*args, **kwargs): diff --git a/app/utils/login.py b/app/utils/login.py index 0c7be1f48..99627c6eb 100644 --- a/app/utils/login.py +++ b/app/utils/login.py @@ -3,6 +3,7 @@ from functools import wraps from flask import redirect, request, session, url_for from app.models.user import User +from app.utils.time import is_less_than_days_ago def redirect_to_sign_in(f): @@ -41,6 +42,10 @@ def redirect_when_logged_in(platform_admin): return redirect(url_for('main.show_accounts_or_dashboard')) +def email_needs_revalidating(user): + return not is_less_than_days_ago(user.email_access_validated_at, 90) + + # see http://flask.pocoo.org/snippets/62/ def _is_safe_redirect_url(target): from urllib.parse import urljoin, urlparse diff --git a/app/utils/time.py b/app/utils/time.py new file mode 100644 index 000000000..efe251814 --- /dev/null +++ b/app/utils/time.py @@ -0,0 +1,20 @@ +from datetime import datetime + +import pytz +from dateutil import parser +from notifications_utils.timezones import utc_string_to_aware_gmt_datetime + + +def get_current_financial_year(): + now = utc_string_to_aware_gmt_datetime( + datetime.utcnow() + ) + current_month = int(now.strftime('%-m')) + current_year = int(now.strftime('%Y')) + return current_year if current_month > 3 else current_year - 1 + + +def is_less_than_days_ago(date_from_db, number_of_days): + return ( + datetime.utcnow().astimezone(pytz.utc) - parser.parse(date_from_db) + ).days < number_of_days diff --git a/tests/app/main/views/test_two_factor.py b/tests/app/main/views/test_two_factor.py index f431a75fd..50c63e4d0 100644 --- a/tests/app/main/views/test_two_factor.py +++ b/tests/app/main/views/test_two_factor.py @@ -1,7 +1,6 @@ import pytest from bs4 import BeautifulSoup from flask import url_for -from freezegun import freeze_time from tests.conftest import ( SERVICE_ONE_ID, @@ -11,6 +10,11 @@ from tests.conftest import ( ) +@pytest.fixture +def mock_email_validated_recently(mocker): + return mocker.patch('app.main.views.two_factor.email_needs_revalidating', return_value=False) + + @pytest.mark.parametrize('request_url', ['two_factor_email_sent', 'revalidate_email_sent']) @pytest.mark.parametrize('redirect_url', [None, f'/services/{SERVICE_ONE_ID}/templates']) @pytest.mark.parametrize('email_resent, page_title', [ @@ -71,7 +75,6 @@ def test_should_render_two_factor_page( )['href'] == url_for('main.check_and_resend_text_code', next=redirect_url) -@freeze_time('2020-01-27T12:00:00') def test_should_login_user_and_should_redirect_to_next_url( client, api_user_active, @@ -79,12 +82,12 @@ def test_should_login_user_and_should_redirect_to_next_url( mock_get_user_by_email, mock_check_verify_code, mock_create_event, + mock_email_validated_recently, ): with client.session_transaction() as session: session['user_details'] = { 'id': api_user_active['id'], 'email': api_user_active['email_address']} - api_user_active['email_access_validated_at'] = '2020-01-23T11:35:21.726132Z' response = client.post(url_for('main.two_factor_sms', next='/services/{}'.format(SERVICE_ONE_ID)), data={'sms_code': '12345'}) @@ -96,7 +99,6 @@ def test_should_login_user_and_should_redirect_to_next_url( ) -@freeze_time('2020-01-27T12:00:00') def test_should_send_email_and_redirect_to_info_page_if_user_needs_to_revalidate_email( client, api_user_active, @@ -107,7 +109,7 @@ def test_should_send_email_and_redirect_to_info_page_if_user_needs_to_revalidate mocker ): mocker.patch('app.user_api_client.get_user', return_value=api_user_active) - api_user_active['email_access_validated_at'] = '2019-03-23T11:35:21.726132Z' + mocker.patch('app.main.views.two_factor.email_needs_revalidating', return_value=True) with client.session_transaction() as session: session['user_details'] = { 'id': api_user_active['id'], @@ -124,7 +126,6 @@ def test_should_send_email_and_redirect_to_info_page_if_user_needs_to_revalidate mock_send_verify_code.assert_called_with(api_user_active['id'], 'email', None, mocker.ANY) -@freeze_time('2020-01-27T12:00:00') def test_should_login_user_and_not_redirect_to_external_url( client, api_user_active, @@ -133,12 +134,12 @@ def test_should_login_user_and_not_redirect_to_external_url( mock_check_verify_code, mock_get_services_with_one_service, mock_create_event, + mock_email_validated_recently, ): with client.session_transaction() as session: session['user_details'] = { 'id': api_user_active['id'], 'email': api_user_active['email_address']} - api_user_active['email_access_validated_at'] = '2020-01-23T11:35:21.726132Z' response = client.post(url_for('main.two_factor_sms', next='http://www.google.com'), data={'sms_code': '12345'}) @@ -149,7 +150,6 @@ def test_should_login_user_and_not_redirect_to_external_url( @pytest.mark.parametrize('platform_admin', ( True, False, )) -@freeze_time('2020-01-27T12:00:00') def test_should_login_user_and_redirect_to_show_accounts( client, api_user_active, @@ -157,13 +157,13 @@ def test_should_login_user_and_redirect_to_show_accounts( mock_get_user_by_email, mock_check_verify_code, mock_create_event, + mock_email_validated_recently, platform_admin, ): with client.session_transaction() as session: session['user_details'] = { 'id': api_user_active['id'], 'email': api_user_active['email_address']} - api_user_active['email_access_validated_at'] = '2020-01-23T11:35:21.726132Z' api_user_active['platform_admin'] = platform_admin response = client.post(url_for('main.two_factor_sms'), @@ -192,7 +192,6 @@ def test_should_return_200_with_sms_code_error_when_sms_code_is_wrong( assert 'Code not found' in response.get_data(as_text=True) -@freeze_time('2020-01-27T12:00:00') def test_should_login_user_when_multiple_valid_codes_exist( client, api_user_active, @@ -201,19 +200,18 @@ def test_should_login_user_when_multiple_valid_codes_exist( mock_check_verify_code, mock_get_services_with_one_service, mock_create_event, + mock_email_validated_recently, ): with client.session_transaction() as session: session['user_details'] = { 'id': api_user_active['id'], 'email': api_user_active['email_address']} - api_user_active['email_access_validated_at'] = '2020-01-23T11:35:21.726132Z' response = client.post(url_for('main.two_factor_sms'), data={'sms_code': '23456'}) assert response.status_code == 302 -@freeze_time('2020-01-27T12:00:00') def test_two_factor_should_set_password_when_new_password_exists_in_session( client, api_user_active, @@ -222,13 +220,13 @@ def test_two_factor_should_set_password_when_new_password_exists_in_session( mock_get_services_with_one_service, mock_update_user_password, mock_create_event, + mock_email_validated_recently, ): with client.session_transaction() as session: session['user_details'] = { 'id': api_user_active['id'], 'email': api_user_active['email_address'], 'password': 'changedpassword'} - api_user_active['email_access_validated_at'] = '2020-01-23T11:35:21.726132Z' response = client.post(url_for('main.two_factor_sms'), data={'sms_code': '12345'}) @@ -279,7 +277,6 @@ def test_two_factor_get_should_redirect_to_sign_in_if_user_not_in_session( ) -@freeze_time('2020-01-27T12:00:00') def test_two_factor_should_activate_pending_user( client, mocker, @@ -287,10 +284,10 @@ def test_two_factor_should_activate_pending_user( mock_check_verify_code, mock_create_event, mock_activate_user, + mock_email_validated_recently, ): mocker.patch('app.user_api_client.get_user', return_value=api_user_pending) mocker.patch('app.service_api_client.get_services', return_value={'data': []}) - api_user_pending['email_access_validated_at'] = '2020-01-23T11:35:21.726132Z' with client.session_transaction() as session: session['user_details'] = { 'id': api_user_pending['id'], diff --git a/tests/app/main/views/test_webauthn_credentials.py b/tests/app/main/views/test_webauthn_credentials.py index f7afdddf2..1b4ba5f31 100644 --- a/tests/app/main/views/test_webauthn_credentials.py +++ b/tests/app/main/views/test_webauthn_credentials.py @@ -4,7 +4,6 @@ from unittest.mock import ANY, Mock import pytest from fido2 import cbor from flask import url_for -from freezegun.api import freeze_time from app.models.webauthn_credential import RegistrationError, WebAuthnCredential @@ -336,7 +335,6 @@ def test_complete_authentication_clears_session( assert 'webauthn_authentication_state' not in session -@freeze_time('2020-01-30') @pytest.mark.parametrize('url_kwargs, expected_redirect', [ ({}, '/accounts-or-dashboard'), ({'next': '/bar'}, '/bar'), @@ -350,7 +348,6 @@ def test_verify_webauthn_login_signs_user_in( expected_redirect, ): platform_admin_user['auth_type'] = 'webauthn_auth' - platform_admin_user['email_access_validated_at'] = '2020-01-25T00:00:00.000000Z' with client.session_transaction() as session: session['user_details'] = { @@ -360,6 +357,7 @@ def test_verify_webauthn_login_signs_user_in( mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user) mocker.patch('app.main.views.webauthn_credentials._verify_webauthn_authentication') mocker.patch('app.user_api_client.complete_webauthn_login_attempt', return_value=(True, None)) + mocker.patch('app.main.views.webauthn_credentials.email_needs_revalidating', return_value=False) resp = client.post(url_for('main.webauthn_complete_authentication', **url_kwargs)) @@ -397,7 +395,6 @@ def test_verify_webauthn_login_signs_user_in_doesnt_sign_user_in_if_api_rejects( assert resp.status_code == 403 -@freeze_time('2020-04-30') def test_verify_webauthn_login_signs_user_in_sends_revalidation_email_if_needed( client, mocker, @@ -405,7 +402,6 @@ def test_verify_webauthn_login_signs_user_in_sends_revalidation_email_if_needed( platform_admin_user, ): platform_admin_user['auth_type'] = 'webauthn_auth' - platform_admin_user['email_access_validated_at'] = '2020-01-25T00:00:00.000000Z' user_details = { 'id': platform_admin_user['id'], 'email': platform_admin_user['email_address'] @@ -417,6 +413,7 @@ def test_verify_webauthn_login_signs_user_in_sends_revalidation_email_if_needed( mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user) mocker.patch('app.main.views.webauthn_credentials._verify_webauthn_authentication') mocker.patch('app.user_api_client.complete_webauthn_login_attempt', return_value=(True, None)) + mocker.patch('app.main.views.webauthn_credentials.email_needs_revalidating', return_value=True) resp = client.post(url_for('main.webauthn_complete_authentication')) diff --git a/tests/app/test_utils.py b/tests/app/test_utils.py index 68536b22e..9503a2f51 100644 --- a/tests/app/test_utils.py +++ b/tests/app/test_utils.py @@ -3,12 +3,7 @@ from freezegun import freeze_time from app import format_datetime_relative from app.formatters import email_safe, round_to_significant_figures -from app.utils import ( - get_current_financial_year, - get_logo_cdn_domain, - is_less_than_days_ago, - merge_jsonlike, -) +from app.utils import get_logo_cdn_domain, merge_jsonlike @pytest.mark.parametrize('service_name, safe_email', [ @@ -79,16 +74,6 @@ def test_format_datetime_relative(time, human_readable_datetime): assert format_datetime_relative(time) == human_readable_datetime -@pytest.mark.parametrize("date_from_db, expected_result", [ - ('2019-11-17T11:35:21.726132Z', True), - ('2019-11-16T11:35:21.726132Z', False), - ('2019-11-16T11:35:21+0000', False), -]) -@freeze_time('2020-02-14T12:00:00') -def test_is_less_than_days_ago(date_from_db, expected_result): - assert is_less_than_days_ago(date_from_db, 90) == expected_result - - @pytest.mark.parametrize("source_object, destination_object, expected_result", [ # simple dicts: ({"a": "b"}, {"c": "d"}, {"a": "b", "c": "d"}), @@ -137,14 +122,3 @@ def test_merge_jsonlike_merges_jsonlike_objects_correctly(source_object, destina )) def test_round_to_significant_figures(value, significant_figures, expected_result): assert round_to_significant_figures(value, significant_figures) == expected_result - - -@pytest.mark.parametrize('datetime_string, financial_year', ( - ('2021-01-01T00:00:00+00:00', 2020), # Start of 2021 - ('2021-03-31T22:59:59+00:00', 2020), # One minute before midnight (BST) - ('2021-03-31T23:00:00+00:00', 2021), # Midnight (BST) - ('2021-12-12T12:12:12+01:00', 2021), # Later in the year -)) -def test_get_financial_year(datetime_string, financial_year): - with freeze_time(datetime_string): - assert get_current_financial_year() == financial_year diff --git a/tests/app/utils/test_login.py b/tests/app/utils/test_login.py new file mode 100644 index 000000000..c376561eb --- /dev/null +++ b/tests/app/utils/test_login.py @@ -0,0 +1,19 @@ +import pytest +from freezegun import freeze_time + +from app.models.user import User +from app.utils.login import email_needs_revalidating + + +@freeze_time('2020-11-27T12:00:00') +@pytest.mark.parametrize(('email_access_validated_at', 'expected_result'), ( + ('2020-10-01T11:35:21.726132Z', False), + ('2020-07-23T11:35:21.726132Z', True), +)) +def test_email_needs_revalidating( + api_user_active, + email_access_validated_at, + expected_result, +): + api_user_active['email_access_validated_at'] = email_access_validated_at + assert email_needs_revalidating(User(api_user_active)) == expected_result diff --git a/tests/app/utils/test_time.py b/tests/app/utils/test_time.py new file mode 100644 index 000000000..509154c41 --- /dev/null +++ b/tests/app/utils/test_time.py @@ -0,0 +1,25 @@ +import pytest +from freezegun import freeze_time + +from app.utils.time import get_current_financial_year, is_less_than_days_ago + + +@pytest.mark.parametrize("date_from_db, expected_result", [ + ('2019-11-17T11:35:21.726132Z', True), + ('2019-11-16T11:35:21.726132Z', False), + ('2019-11-16T11:35:21+0000', False), +]) +@freeze_time('2020-02-14T12:00:00') +def test_is_less_than_days_ago(date_from_db, expected_result): + assert is_less_than_days_ago(date_from_db, 90) == expected_result + + +@pytest.mark.parametrize('datetime_string, financial_year', ( + ('2021-01-01T00:00:00+00:00', 2020), # Start of 2021 + ('2021-03-31T22:59:59+00:00', 2020), # One minute before midnight (BST) + ('2021-03-31T23:00:00+00:00', 2021), # Midnight (BST) + ('2021-12-12T12:12:12+01:00', 2021), # Later in the year +)) +def test_get_financial_year(datetime_string, financial_year): + with freeze_time(datetime_string): + assert get_current_financial_year() == financial_year From 1c4a903b653292497db5f948f60f77fe0e9a3d1c Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Mon, 14 Jun 2021 12:44:06 +0100 Subject: [PATCH 5/6] Move tests to match the module they are in Previously some of the tests for code in the "formatters" module were in tests for the "utils" module. This moves them to where they should be. While two of these methods are probably more utils than formatters, I'd like to postpone a refactor of that module for now, and focus on slimming down test for utils/__init__.py. --- tests/app/main/test_formatters.py | 77 ++++++++++++++++++++++++++++++- tests/app/test_utils.py | 74 ----------------------------- 2 files changed, 76 insertions(+), 75 deletions(-) diff --git a/tests/app/main/test_formatters.py b/tests/app/main/test_formatters.py index 8e0198ee2..5ad260b5d 100644 --- a/tests/app/main/test_formatters.py +++ b/tests/app/main/test_formatters.py @@ -2,10 +2,14 @@ from functools import partial import pytest from flask import url_for +from freezegun import freeze_time -from app import ( +from app.formatters import ( + email_safe, + format_datetime_relative, format_notification_status_as_url, format_number_in_pounds_as_currency, + round_to_significant_figures, ) @@ -52,3 +56,74 @@ def test_format_notification_status_as_url( ]) def test_format_number_in_pounds_as_currency(input_number, formatted_number): assert format_number_in_pounds_as_currency(input_number) == formatted_number + + +@pytest.mark.parametrize('time, human_readable_datetime', [ + ('2018-03-14 09:00', '14 March at 9:00am'), + ('2018-03-14 15:00', '14 March at 3:00pm'), + + ('2018-03-15 09:00', '15 March at 9:00am'), + ('2018-03-15 15:00', '15 March at 3:00pm'), + + ('2018-03-19 09:00', '19 March at 9:00am'), + ('2018-03-19 15:00', '19 March at 3:00pm'), + ('2018-03-19 23:59', '19 March at 11:59pm'), + + ('2018-03-20 00:00', '19 March at midnight'), # we specifically refer to 00:00 as belonging to the day before. + ('2018-03-20 00:01', 'yesterday at 12:01am'), + ('2018-03-20 09:00', 'yesterday at 9:00am'), + ('2018-03-20 15:00', 'yesterday at 3:00pm'), + ('2018-03-20 23:59', 'yesterday at 11:59pm'), + + ('2018-03-21 00:00', 'yesterday at midnight'), # we specifically refer to 00:00 as belonging to the day before. + ('2018-03-21 00:01', 'today at 12:01am'), + ('2018-03-21 09:00', 'today at 9:00am'), + ('2018-03-21 12:00', 'today at midday'), + ('2018-03-21 15:00', 'today at 3:00pm'), + ('2018-03-21 23:59', 'today at 11:59pm'), + + ('2018-03-22 00:00', 'today at midnight'), # we specifically refer to 00:00 as belonging to the day before. + ('2018-03-22 00:01', 'tomorrow at 12:01am'), + ('2018-03-22 09:00', 'tomorrow at 9:00am'), + ('2018-03-22 15:00', 'tomorrow at 3:00pm'), + ('2018-03-22 23:59', 'tomorrow at 11:59pm'), + + ('2018-03-23 00:01', '23 March at 12:01am'), + ('2018-03-23 09:00', '23 March at 9:00am'), + ('2018-03-23 15:00', '23 March at 3:00pm'), + +]) +def test_format_datetime_relative(time, human_readable_datetime): + with freeze_time('2018-03-21 12:00'): + assert format_datetime_relative(time) == human_readable_datetime + + +@pytest.mark.parametrize('value, significant_figures, expected_result', ( + (0, 1, 0), + (0, 2, 0), + (12_345, 1, 10_000), + (12_345, 2, 12_000), + (12_345, 3, 12_300), + (12_345, 9, 12_345), + (12_345.6789, 1, 10_000), + (12_345.6789, 9, 12_345), + (-12_345, 1, -10_000), +)) +def test_round_to_significant_figures(value, significant_figures, expected_result): + assert round_to_significant_figures(value, significant_figures) == expected_result + + +@pytest.mark.parametrize('service_name, safe_email', [ + ('name with spaces', 'name.with.spaces'), + ('singleword', 'singleword'), + ('UPPER CASE', 'upper.case'), + ('Service - with dash', 'service.with.dash'), + ('lots of spaces', 'lots.of.spaces'), + ('name.with.dots', 'name.with.dots'), + ('name-with-other-delimiters', 'namewithotherdelimiters'), + ('.leading', 'leading'), + ('trailing.', 'trailing'), + ('üńïçödë wördś', 'unicode.words'), +]) +def test_email_safe_return_dot_separated_email_domain(service_name, safe_email): + assert email_safe(service_name) == safe_email diff --git a/tests/app/test_utils.py b/tests/app/test_utils.py index 9503a2f51..e60a9b7a6 100644 --- a/tests/app/test_utils.py +++ b/tests/app/test_utils.py @@ -1,27 +1,8 @@ import pytest -from freezegun import freeze_time -from app import format_datetime_relative -from app.formatters import email_safe, round_to_significant_figures from app.utils import get_logo_cdn_domain, merge_jsonlike -@pytest.mark.parametrize('service_name, safe_email', [ - ('name with spaces', 'name.with.spaces'), - ('singleword', 'singleword'), - ('UPPER CASE', 'upper.case'), - ('Service - with dash', 'service.with.dash'), - ('lots of spaces', 'lots.of.spaces'), - ('name.with.dots', 'name.with.dots'), - ('name-with-other-delimiters', 'namewithotherdelimiters'), - ('.leading', 'leading'), - ('trailing.', 'trailing'), - ('üńïçödë wördś', 'unicode.words'), -]) -def test_email_safe_return_dot_separated_email_domain(service_name, safe_email): - assert email_safe(service_name) == safe_email - - def test_get_cdn_domain_on_localhost(client, mocker): mocker.patch.dict('app.current_app.config', values={'ADMIN_BASE_URL': 'http://localhost:6012'}) domain = get_logo_cdn_domain() @@ -34,46 +15,6 @@ def test_get_cdn_domain_on_non_localhost(client, mocker): assert domain == 'static-logos.admintest.com' -@pytest.mark.parametrize('time, human_readable_datetime', [ - ('2018-03-14 09:00', '14 March at 9:00am'), - ('2018-03-14 15:00', '14 March at 3:00pm'), - - ('2018-03-15 09:00', '15 March at 9:00am'), - ('2018-03-15 15:00', '15 March at 3:00pm'), - - ('2018-03-19 09:00', '19 March at 9:00am'), - ('2018-03-19 15:00', '19 March at 3:00pm'), - ('2018-03-19 23:59', '19 March at 11:59pm'), - - ('2018-03-20 00:00', '19 March at midnight'), # we specifically refer to 00:00 as belonging to the day before. - ('2018-03-20 00:01', 'yesterday at 12:01am'), - ('2018-03-20 09:00', 'yesterday at 9:00am'), - ('2018-03-20 15:00', 'yesterday at 3:00pm'), - ('2018-03-20 23:59', 'yesterday at 11:59pm'), - - ('2018-03-21 00:00', 'yesterday at midnight'), # we specifically refer to 00:00 as belonging to the day before. - ('2018-03-21 00:01', 'today at 12:01am'), - ('2018-03-21 09:00', 'today at 9:00am'), - ('2018-03-21 12:00', 'today at midday'), - ('2018-03-21 15:00', 'today at 3:00pm'), - ('2018-03-21 23:59', 'today at 11:59pm'), - - ('2018-03-22 00:00', 'today at midnight'), # we specifically refer to 00:00 as belonging to the day before. - ('2018-03-22 00:01', 'tomorrow at 12:01am'), - ('2018-03-22 09:00', 'tomorrow at 9:00am'), - ('2018-03-22 15:00', 'tomorrow at 3:00pm'), - ('2018-03-22 23:59', 'tomorrow at 11:59pm'), - - ('2018-03-23 00:01', '23 March at 12:01am'), - ('2018-03-23 09:00', '23 March at 9:00am'), - ('2018-03-23 15:00', '23 March at 3:00pm'), - -]) -def test_format_datetime_relative(time, human_readable_datetime): - with freeze_time('2018-03-21 12:00'): - assert format_datetime_relative(time) == human_readable_datetime - - @pytest.mark.parametrize("source_object, destination_object, expected_result", [ # simple dicts: ({"a": "b"}, {"c": "d"}, {"a": "b", "c": "d"}), @@ -107,18 +48,3 @@ def test_format_datetime_relative(time, human_readable_datetime): def test_merge_jsonlike_merges_jsonlike_objects_correctly(source_object, destination_object, expected_result): merge_jsonlike(source_object, destination_object) assert source_object == expected_result - - -@pytest.mark.parametrize('value, significant_figures, expected_result', ( - (0, 1, 0), - (0, 2, 0), - (12_345, 1, 10_000), - (12_345, 2, 12_000), - (12_345, 3, 12_300), - (12_345, 9, 12_345), - (12_345.6789, 1, 10_000), - (12_345.6789, 9, 12_345), - (-12_345, 1, -10_000), -)) -def test_round_to_significant_figures(value, significant_figures, expected_result): - assert round_to_significant_figures(value, significant_figures) == expected_result From 9177ffa65ac50026078610193b67fdfd6ac8358b Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Wed, 16 Jun 2021 16:08:38 +0100 Subject: [PATCH 6/6] Clarify variable name in pagination tests We should avoid using abbreviations, as they aren't universally understood i.e. they're not worth the small saving in typing. --- tests/app/utils/test_pagination.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/app/utils/test_pagination.py b/tests/app/utils/test_pagination.py index 444942f5e..8bea3a7ac 100644 --- a/tests/app/utils/test_pagination.py +++ b/tests/app/utils/test_pagination.py @@ -2,19 +2,19 @@ from app.utils.pagination import generate_next_dict, generate_previous_dict def test_generate_previous_dict(client): - ret = generate_previous_dict('main.view_jobs', 'foo', 2, {}) - assert 'page=1' in ret['url'] - assert ret['title'] == 'Previous page' - assert ret['label'] == 'page 1' + result = generate_previous_dict('main.view_jobs', 'foo', 2, {}) + assert 'page=1' in result['url'] + assert result['title'] == 'Previous page' + assert result['label'] == 'page 1' def test_generate_next_dict(client): - ret = generate_next_dict('main.view_jobs', 'foo', 2, {}) - assert 'page=3' in ret['url'] - assert ret['title'] == 'Next page' - assert ret['label'] == 'page 3' + result = generate_next_dict('main.view_jobs', 'foo', 2, {}) + assert 'page=3' in result['url'] + assert result['title'] == 'Next page' + assert result['label'] == 'page 3' def test_generate_previous_next_dict_adds_other_url_args(client): - ret = generate_next_dict('main.view_notifications', 'foo', 2, {'message_type': 'blah'}) - assert 'notifications/blah' in ret['url'] + result = generate_next_dict('main.view_notifications', 'foo', 2, {'message_type': 'blah'}) + assert 'notifications/blah' in result['url']