diff --git a/app/templates/views/providers/edit-provider.html b/app/templates/views/providers/edit-provider.html index 0720851c0..98c2a7245 100644 --- a/app/templates/views/providers/edit-provider.html +++ b/app/templates/views/providers/edit-provider.html @@ -5,7 +5,7 @@ {% from "components/back-link/macro.njk" import govukBackLink %} {% block per_page_title %} -Provider - {{provider.display_name}} + {{provider.display_name}} {% endblock %} {% block backLink %} diff --git a/app/templates/views/providers/provider.html b/app/templates/views/providers/provider.html index 5d32e2619..8bb831da1 100644 --- a/app/templates/views/providers/provider.html +++ b/app/templates/views/providers/provider.html @@ -5,7 +5,7 @@ {% from "components/back-link/macro.njk" import govukBackLink %} {% block per_page_title %} -Provider versions + {{ provider_versions[0].display_name }} {% endblock %} {% block backLink %} diff --git a/app/templates/views/providers/providers.html b/app/templates/views/providers/providers.html index f5f632f90..ee6e933ac 100644 --- a/app/templates/views/providers/providers.html +++ b/app/templates/views/providers/providers.html @@ -70,7 +70,7 @@ Providers {% endcall %} -

International SMS Providers

+

International SMS Providers

{% call(item, row_number) list_table( intl_sms_providers, diff --git a/tests/app/main/views/accounts/test_show_accounts_or_dashboard.py b/tests/app/main/views/accounts/test_show_accounts_or_dashboard.py index d9337dd0c..66041aa27 100644 --- a/tests/app/main/views/accounts/test_show_accounts_or_dashboard.py +++ b/tests/app/main/views/accounts/test_show_accounts_or_dashboard.py @@ -27,130 +27,128 @@ def user_with_orgs_and_services(num_orgs, num_services, platform_admin=False): (1, 0, '.organisation_dashboard', {'org_id': 'org1'}), ]) def test_show_accounts_or_dashboard_redirects_to_choose_account_or_service_dashboard( - client, - mocker, + client_request, mock_get_organisations_and_services_for_user, num_orgs, num_services, endpoint, endpoint_kwargs ): - client.login(user_with_orgs_and_services(num_orgs=num_orgs, num_services=num_services), mocker=mocker) + client_request.login(user_with_orgs_and_services(num_orgs=num_orgs, num_services=num_services)) - response = client.get(url_for('main.show_accounts_or_dashboard')) - - assert response.status_code == 302 - assert response.location == url_for(endpoint, _external=True, **endpoint_kwargs) - - -def test_show_accounts_or_dashboard_redirects_if_service_in_session(client, mocker, mock_get_service): - client.login(user_with_orgs_and_services(num_orgs=1, num_services=1), mocker=mocker) - with client.session_transaction() as session: - session['service_id'] = 'service1' - session['organisation_id'] = None - - response = client.get(url_for('.show_accounts_or_dashboard')) - - assert response.status_code == 302 - assert response.location == url_for( - 'main.service_dashboard', - service_id='service1', - _external=True + client_request.get( + 'main.show_accounts_or_dashboard', + _expected_redirect=url_for(endpoint, _external=True, **endpoint_kwargs) ) -def test_show_accounts_or_dashboard_redirects_if_org_in_session(client, mocker): - client.login(user_with_orgs_and_services(num_orgs=1, num_services=1), mocker=mocker) - with client.session_transaction() as session: +def test_show_accounts_or_dashboard_redirects_if_service_in_session(client_request, mock_get_service): + client_request.login(user_with_orgs_and_services(num_orgs=1, num_services=1)) + with client_request.session_transaction() as session: + session['service_id'] = 'service1' + session['organisation_id'] = None + + client_request.get( + '.show_accounts_or_dashboard', + _expected_redirect=url_for( + 'main.service_dashboard', + service_id='service1', + _external=True + ), + ) + + +def test_show_accounts_or_dashboard_redirects_if_org_in_session(client_request): + client_request.login(user_with_orgs_and_services(num_orgs=1, num_services=1)) + with client_request.session_transaction() as session: session['service_id'] = None session['organisation_id'] = 'org1' - response = client.get(url_for('.show_accounts_or_dashboard')) - - assert response.status_code == 302 - assert response.location == url_for( - 'main.organisation_dashboard', - org_id='org1', - _external=True + client_request.get( + '.show_accounts_or_dashboard', + _expected_redirect=url_for( + 'main.organisation_dashboard', + org_id='org1', + _external=True + ), ) def test_show_accounts_or_dashboard_doesnt_redirect_to_service_dashboard_if_user_not_part_of_service_in_session( - client, - mocker, + client_request, mock_get_organisations_and_services_for_user, mock_get_service ): - client.login(user_with_orgs_and_services(num_orgs=1, num_services=1), mocker=mocker) - with client.session_transaction() as session: + client_request.login(user_with_orgs_and_services(num_orgs=1, num_services=1)) + with client_request.session_transaction() as session: session['service_id'] = 'service2' session['organisation_id'] = None - response = client.get(url_for('.show_accounts_or_dashboard')) - - assert response.status_code == 302 - assert response.location == url_for('main.organisation_dashboard', org_id='org1', _external=True) + client_request.get( + '.show_accounts_or_dashboard', + _expected_redirect=url_for('main.organisation_dashboard', org_id='org1', _external=True) + ) def test_show_accounts_or_dashboard_doesnt_redirect_to_org_dashboard_if_user_not_part_of_org_in_session( - client, - mocker, + client_request, mock_get_organisations_and_services_for_user, ): - client.login(user_with_orgs_and_services(num_orgs=1, num_services=1), mocker=mocker) - with client.session_transaction() as session: + client_request.login(user_with_orgs_and_services(num_orgs=1, num_services=1)) + with client_request.session_transaction() as session: session['service_id'] = None session['organisation_id'] = 'org2' - response = client.get(url_for('.show_accounts_or_dashboard')) - - assert response.status_code == 302 - assert response.location == url_for('main.organisation_dashboard', org_id='org1', _external=True) + client_request.get( + '.show_accounts_or_dashboard', + _expected_redirect=url_for('main.organisation_dashboard', org_id='org1', _external=True) + ) def test_show_accounts_or_dashboard_redirects_if_not_logged_in( - client, + client_request, notify_admin, ): - response = client.get(url_for('main.show_accounts_or_dashboard')) - assert response.status_code == 302 - assert response.location == url_for('main.index', _external=True) + client_request.logout() + client_request.get( + 'main.show_accounts_or_dashboard', + _expected_redirect=url_for('main.index', _external=True), + ) def test_show_accounts_or_dashboard_redirects_to_service_dashboard_if_platform_admin( - client, + client_request, mocker, mock_get_service ): - client.login(user_with_orgs_and_services(num_orgs=1, num_services=1, platform_admin=True), mocker=mocker) - with client.session_transaction() as session: + client_request.login(user_with_orgs_and_services(num_orgs=1, num_services=1, platform_admin=True)) + with client_request.session_transaction() as session: session['service_id'] = 'service2' session['organisation_id'] = None - response = client.get(url_for('.show_accounts_or_dashboard')) - - assert response.status_code == 302 - assert response.location == url_for( - 'main.service_dashboard', - service_id='service2', - _external=True + client_request.get( + '.show_accounts_or_dashboard', + _expected_redirect=url_for( + 'main.service_dashboard', + service_id='service2', + _external=True + ), ) def test_show_accounts_or_dashboard_redirects_to_org_dashboard_if_platform_admin( - client, - mocker + client_request, ): - client.login(user_with_orgs_and_services(num_orgs=1, num_services=1, platform_admin=True), mocker=mocker) - with client.session_transaction() as session: + client_request.login(user_with_orgs_and_services(num_orgs=1, num_services=1, platform_admin=True)) + with client_request.session_transaction() as session: session['service_id'] = None session['organisation_id'] = 'org2' - response = client.get(url_for('.show_accounts_or_dashboard')) - - assert response.status_code == 302 - assert response.location == url_for( - 'main.organisation_dashboard', - org_id='org2', - _external=True + client_request.get( + '.show_accounts_or_dashboard', + _expected_redirect=url_for( + 'main.organisation_dashboard', + org_id='org2', + _external=True + ), ) diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index 4e8c35ac2..2744c9cc1 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -9,17 +9,16 @@ from tests.conftest import normalize_spaces def test_non_gov_user_cannot_see_add_service_button( - client, + client_request, mock_login, mock_get_non_govuser, api_nongov_user_active, mock_get_organisations, mock_get_organisations_and_services_for_user, ): - client.login(api_nongov_user_active) - response = client.get(url_for('main.choose_account')) - assert 'Add a new service' not in response.get_data(as_text=True) - assert response.status_code == 200 + client_request.login(api_nongov_user_active) + page = client_request.get('main.choose_account') + assert 'Add a new service' not in page.text @pytest.mark.parametrize('org_json', ( diff --git a/tests/app/main/views/test_api_integration.py b/tests/app/main/views/test_api_integration.py index 521c52ce5..7857d199f 100644 --- a/tests/app/main/views/test_api_integration.py +++ b/tests/app/main/views/test_api_integration.py @@ -171,21 +171,19 @@ def test_api_documentation_page_should_redirect( def test_should_show_empty_api_keys_page( - client, + client_request, api_user_active, mock_login, mock_get_no_api_keys, mock_get_service, mock_has_permissions, ): - client.login(api_user_active) - service_id = str(uuid.uuid4()) - response = client.get(url_for('main.api_keys', service_id=service_id)) + client_request.login(api_user_active) + page = client_request.get('main.api_keys', service_id=SERVICE_ONE_ID) - assert response.status_code == 200 - assert 'You have not created any API keys yet' in response.get_data(as_text=True) - assert 'Create an API key' in response.get_data(as_text=True) - mock_get_no_api_keys.assert_called_once_with(service_id) + assert 'You have not created any API keys yet' in page.text + assert 'Create an API key' in page.text + mock_get_no_api_keys.assert_called_once_with(SERVICE_ONE_ID) def test_should_show_api_keys_page( diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 685498990..0bbaec0ff 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -3,7 +3,6 @@ import json from datetime import datetime import pytest -from bs4 import BeautifulSoup from flask import url_for from freezegun import freeze_time @@ -1183,21 +1182,20 @@ def test_future_usage_page( mock_get_free_sms_fragment_limit.assert_called_with(SERVICE_ONE_ID, 2014) -def _test_dashboard_menu(mocker, notify_admin, usr, service, permissions): - with notify_admin.test_request_context(): - with notify_admin.test_client() as client: - usr['permissions'][str(service['id'])] = permissions - usr['services'] = [service['id']] - mocker.patch('app.user_api_client.check_verify_code', return_value=(True, '')) - mocker.patch('app.service_api_client.get_services', return_value={'data': [service]}) - mocker.patch('app.user_api_client.get_user', return_value=usr) - mocker.patch('app.user_api_client.get_user_by_email', return_value=usr) - mocker.patch('app.service_api_client.get_service', return_value={'data': service}) - client.login(usr) - return client.get(url_for('main.service_dashboard', service_id=service['id'])) +def _test_dashboard_menu(client_request, mocker, usr, service, permissions): + usr['permissions'][str(service['id'])] = permissions + usr['services'] = [service['id']] + mocker.patch('app.user_api_client.check_verify_code', return_value=(True, '')) + mocker.patch('app.service_api_client.get_services', return_value={'data': [service]}) + mocker.patch('app.user_api_client.get_user', return_value=usr) + mocker.patch('app.user_api_client.get_user_by_email', return_value=usr) + mocker.patch('app.service_api_client.get_service', return_value={'data': service}) + client_request.login(usr) + return client_request.get('main.service_dashboard', service_id=service['id']) def test_menu_send_messages( + client_request, mocker, notify_admin, api_user_active, @@ -1213,29 +1211,29 @@ def test_menu_send_messages( ): service_one['permissions'] = ['email', 'sms', 'letter', 'upload_letters'] - with notify_admin.test_request_context(): - resp = _test_dashboard_menu( - mocker, - notify_admin, - api_user_active, - service_one, - ['view_activity', 'send_texts', 'send_emails', 'send_letters']) - page = resp.get_data(as_text=True) - assert url_for( - 'main.choose_template', - service_id=service_one['id'], - ) in page - assert url_for('main.uploads', service_id=service_one['id']) in page - assert url_for('main.manage_users', service_id=service_one['id']) in page + page = _test_dashboard_menu( + client_request, + mocker, + api_user_active, + service_one, + ['view_activity', 'send_texts', 'send_emails', 'send_letters'] + ) + page = str(page) + assert url_for( + 'main.choose_template', + service_id=service_one['id'], + ) in page + assert url_for('main.uploads', service_id=service_one['id']) in page + assert url_for('main.manage_users', service_id=service_one['id']) in page - assert url_for('main.service_settings', service_id=service_one['id']) not in page - assert url_for('main.api_keys', service_id=service_one['id']) not in page - assert url_for('main.view_providers') not in page + assert url_for('main.service_settings', service_id=service_one['id']) not in page + assert url_for('main.api_keys', service_id=service_one['id']) not in page + assert url_for('main.view_providers') not in page def test_menu_send_messages_when_service_does_not_have_upload_letters_permission( + client_request, mocker, - notify_admin, api_user_active, service_one, mock_get_service_templates, @@ -1247,21 +1245,20 @@ def test_menu_send_messages_when_service_does_not_have_upload_letters_permission mock_get_free_sms_fragment_limit, mock_get_returned_letter_statistics_with_no_returned_letters, ): - with notify_admin.test_request_context(): - resp = _test_dashboard_menu( - mocker, - notify_admin, - api_user_active, - service_one, - ['view_activity', 'send_texts', 'send_emails', 'send_letters']) - page = BeautifulSoup(resp.data.decode('utf-8'), 'html.parser') - assert page.select_one('.navigation') - assert url_for('main.uploads', service_id=service_one['id']) not in page.select_one('.navigation') + page = _test_dashboard_menu( + client_request, + mocker, + api_user_active, + service_one, + ['view_activity', 'send_texts', 'send_emails', 'send_letters']) + + assert page.select_one('.navigation') + assert url_for('main.uploads', service_id=service_one['id']) not in page.select_one('.navigation') def test_menu_manage_service( + client_request, mocker, - notify_admin, api_user_active, service_one, mock_get_service_templates, @@ -1273,27 +1270,26 @@ def test_menu_manage_service( mock_get_returned_letter_statistics_with_no_returned_letters, mock_get_free_sms_fragment_limit, ): - with notify_admin.test_request_context(): - resp = _test_dashboard_menu( - mocker, - notify_admin, - api_user_active, - service_one, - ['view_activity', 'manage_templates', 'manage_users', 'manage_settings']) - page = resp.get_data(as_text=True) - assert url_for( - 'main.choose_template', - service_id=service_one['id'], - ) in page - assert url_for('main.manage_users', service_id=service_one['id']) in page - assert url_for('main.service_settings', service_id=service_one['id']) in page + page = _test_dashboard_menu( + client_request, + mocker, + api_user_active, + service_one, + ['view_activity', 'manage_templates', 'manage_users', 'manage_settings']) + page = str(page) + assert url_for( + 'main.choose_template', + service_id=service_one['id'], + ) in page + assert url_for('main.manage_users', service_id=service_one['id']) in page + assert url_for('main.service_settings', service_id=service_one['id']) in page - assert url_for('main.api_keys', service_id=service_one['id']) not in page + assert url_for('main.api_keys', service_id=service_one['id']) not in page def test_menu_manage_api_keys( + client_request, mocker, - notify_admin, api_user_active, service_one, mock_get_service_templates, @@ -1305,25 +1301,24 @@ def test_menu_manage_api_keys( mock_get_returned_letter_statistics_with_no_returned_letters, mock_get_free_sms_fragment_limit, ): - with notify_admin.test_request_context(): - resp = _test_dashboard_menu( - mocker, - notify_admin, - api_user_active, - service_one, - ['view_activity', 'manage_api_keys']) + page = _test_dashboard_menu( + client_request, + mocker, + api_user_active, + service_one, + ['view_activity', 'manage_api_keys']) - page = resp.get_data(as_text=True) + page = str(page) - assert url_for('main.choose_template', service_id=service_one['id'],) in page - assert url_for('main.manage_users', service_id=service_one['id']) in page - assert url_for('main.service_settings', service_id=service_one['id']) in page - assert url_for('main.api_integration', service_id=service_one['id']) in page + assert url_for('main.choose_template', service_id=service_one['id'],) in page + assert url_for('main.manage_users', service_id=service_one['id']) in page + assert url_for('main.service_settings', service_id=service_one['id']) in page + assert url_for('main.api_integration', service_id=service_one['id']) in page def test_menu_all_services_for_platform_admin_user( + client_request, mocker, - notify_admin, platform_admin_user, service_one, mock_get_service_templates, @@ -1335,20 +1330,19 @@ def test_menu_all_services_for_platform_admin_user( mock_get_returned_letter_statistics_with_no_returned_letters, mock_get_free_sms_fragment_limit, ): - with notify_admin.test_request_context(): - resp = _test_dashboard_menu( - mocker, - notify_admin, - platform_admin_user, - service_one, - []) - page = resp.get_data(as_text=True) - assert url_for('main.choose_template', service_id=service_one['id']) in page - assert url_for('main.manage_users', service_id=service_one['id']) in page - assert url_for('main.service_settings', service_id=service_one['id']) in page - assert url_for('main.view_notifications', service_id=service_one['id'], message_type='email') in page - assert url_for('main.view_notifications', service_id=service_one['id'], message_type='sms') in page - assert url_for('main.api_keys', service_id=service_one['id']) not in page + page = _test_dashboard_menu( + client_request, + mocker, + platform_admin_user, + service_one, + []) + page = str(page) + assert url_for('main.choose_template', service_id=service_one['id']) in page + assert url_for('main.manage_users', service_id=service_one['id']) in page + assert url_for('main.service_settings', service_id=service_one['id']) in page + assert url_for('main.view_notifications', service_id=service_one['id'], message_type='email') in page + assert url_for('main.view_notifications', service_id=service_one['id'], message_type='sms') in page + assert url_for('main.api_keys', service_id=service_one['id']) not in page def test_route_for_service_permissions( diff --git a/tests/app/main/views/test_feedback.py b/tests/app/main/views/test_feedback.py index edbf42ac0..5e8116c27 100644 --- a/tests/app/main/views/test_feedback.py +++ b/tests/app/main/views/test_feedback.py @@ -374,7 +374,7 @@ ids, params = zip(*[ params, ids=ids ) def test_redirects_to_triage( - client, + client_request, api_user_active, mocker, mock_get_user, @@ -391,12 +391,15 @@ def test_redirects_to_triage( return_value=[{}, {}] if has_live_services else [], ) mocker.patch('app.main.views.feedback.in_business_hours', return_value=is_in_business_hours) - if logged_in: - client.login(api_user_active) + if not logged_in: + client_request.logout() - response = client.get(url_for('main.feedback', ticket_type=ticket_type)) - assert response.status_code == expected_status - assert response.location == expected_redirect(_external=True) + client_request.get( + 'main.feedback', + ticket_type=ticket_type, + _expected_status=expected_status, + _expected_redirect=expected_redirect(_external=True), + ) @pytest.mark.parametrize('ticket_type, expected_h1', ( @@ -573,7 +576,7 @@ def test_back_link_from_form( ] ) def test_should_be_shown_the_bat_email( - client, + client_request, active_user_with_permissions, mocker, service_one, @@ -590,16 +593,20 @@ def test_should_be_shown_the_bat_email( feedback_page = url_for('main.feedback', ticket_type=PROBLEM_TICKET_TYPE, severe=severe) - response = client.get(feedback_page) - - assert response.status_code == expected_status_code - assert response.location == expected_redirect(_external=True) + client_request.logout() + client_request.get_url( + feedback_page, + _expected_status=expected_status_code, + _expected_redirect=expected_redirect(_external=True), + ) # logged in users should never be redirected to the bat email page - client.login(active_user_with_permissions, mocker, service_one) - logged_in_response = client.get(feedback_page) - assert logged_in_response.status_code == expected_status_code_when_logged_in - assert logged_in_response.location == expected_redirect_when_logged_in(_external=True) + client_request.login(active_user_with_permissions) + client_request.get_url( + feedback_page, + _expected_status=expected_status_code_when_logged_in, + _expected_redirect=expected_redirect_when_logged_in(_external=True), + ) @pytest.mark.parametrize( @@ -625,7 +632,7 @@ def test_should_be_shown_the_bat_email( ] ) def test_should_be_shown_the_bat_email_for_general_questions( - client, + client_request, active_user_with_permissions, mocker, service_one, @@ -641,42 +648,45 @@ def test_should_be_shown_the_bat_email_for_general_questions( feedback_page = url_for('main.feedback', ticket_type=GENERAL_TICKET_TYPE, severe=severe) - response = client.get(feedback_page) - - assert response.status_code == expected_status_code - assert response.location == expected_redirect(_external=True) + client_request.logout() + client_request.get_url( + feedback_page, + _expected_status=expected_status_code, + _expected_redirect=expected_redirect(_external=True), + ) # logged in users should never be redirected to the bat email page - client.login(active_user_with_permissions, mocker, service_one) - logged_in_response = client.get(feedback_page) - assert logged_in_response.status_code == expected_status_code_when_logged_in - assert logged_in_response.location == expected_redirect_when_logged_in(_external=True) + client_request.login(active_user_with_permissions) + client_request.get_url( + feedback_page, + _expected_status=expected_status_code_when_logged_in, + _expected_redirect=expected_redirect_when_logged_in(_external=True), + ) def test_bat_email_page( - client, + client_request, active_user_with_permissions, mocker, service_one, ): - bat_phone_page = url_for('main.bat_phone') + bat_phone_page = 'main.bat_phone' - response = client.get(bat_phone_page) - assert response.status_code == 200 + client_request.logout() + page = client_request.get(bat_phone_page) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.select_one('.govuk-back-link').text == 'Back' assert page.select_one('.govuk-back-link')['href'] == url_for('main.support') assert page.select('main a')[1].text == 'Fill in this form' assert page.select('main a')[1]['href'] == url_for('main.feedback', ticket_type=PROBLEM_TICKET_TYPE, severe='no') - next_page_response = client.get(page.select('main a')[1]['href']) - next_page = BeautifulSoup(next_page_response.data.decode('utf-8'), 'html.parser') + next_page = client_request.get_url(page.select('main a')[1]['href']) assert next_page.h1.text.strip() == 'Report a problem' - client.login(active_user_with_permissions, mocker, service_one) - logged_in_response = client.get(bat_phone_page) - assert logged_in_response.status_code == 302 - assert logged_in_response.location == url_for('main.feedback', ticket_type=PROBLEM_TICKET_TYPE, _external=True) + client_request.login(active_user_with_permissions) + client_request.get( + bat_phone_page, + _expected_redirect=url_for('main.feedback', ticket_type=PROBLEM_TICKET_TYPE, _external=True) + ) @pytest.mark.parametrize('out_of_hours_emergency, email_address_provided, out_of_hours, message', ( diff --git a/tests/app/main/views/test_find_users.py b/tests/app/main/views/test_find_users.py index 4510e7a62..4f443c7eb 100644 --- a/tests/app/main/views/test_find_users.py +++ b/tests/app/main/views/test_find_users.py @@ -1,9 +1,7 @@ import uuid import pytest -from bs4 import BeautifulSoup from flask import url_for -from lxml import html from notifications_python_client.errors import HTTPError from tests import user_json @@ -92,11 +90,12 @@ def test_find_users_by_email_validates_against_empty_search_submission( def test_user_information_page_shows_information_about_user( - client, + client_request, platform_admin_user, mocker, fake_uuid, ): + client_request.login(platform_admin_user) user_service_one = uuid.uuid4() user_service_two = uuid.uuid4() mocker.patch('app.user_api_client.get_user', side_effect=[ @@ -112,32 +111,43 @@ def test_user_information_page_shows_information_about_user( ]}, autospec=True ) - client.login(platform_admin_user) - response = client.get(url_for('main.user_information', user_id=fake_uuid)) - assert response.status_code == 200 + page = client_request.get('main.user_information', user_id=fake_uuid) - document = html.fromstring(response.get_data(as_text=True)) + assert normalize_spaces(page.select_one('h1').text) == 'Apple Bloom' - assert document.xpath("//h1/text()[normalize-space()='Apple Bloom']") - assert document.xpath("//p/text()[normalize-space()='test@gov.uk']") - assert document.xpath("//p/text()[normalize-space()='+447700900986']") + assert [ + normalize_spaces(p.text) for p in page.select('main p') + ] == [ + 'test@gov.uk', + '+447700900986', + 'Last logged in just now', + ] - assert document.xpath("//h2/text()[normalize-space()='Live services']") - assert document.xpath("//a/text()[normalize-space()='Nature Therapy']") + assert '0 failed login attempts' not in page.text - assert document.xpath("//h2/text()[normalize-space()='Trial mode services']") - assert document.xpath("//a/text()[normalize-space()='Fresh Orchard Juice']") + assert [ + normalize_spaces(h2.text) for h2 in page.select('main h2') + ] == [ + 'Live services', + 'Trial mode services', + 'Last login', + ] - assert document.xpath("//h2/text()[normalize-space()='Last login']") - assert not document.xpath("//p/text()[normalize-space()='0 failed login attempts']") + assert [ + normalize_spaces(a.text) for a in page.select('main li a') + ] == [ + 'Nature Therapy', + 'Fresh Orchard Juice', + ] def test_user_information_page_displays_if_there_are_failed_login_attempts( - client, + client_request, platform_admin_user, mocker, fake_uuid, ): + client_request.login(platform_admin_user) mocker.patch('app.user_api_client.get_user', side_effect=[ platform_admin_user, user_json(name="Apple Bloom", failed_login_count=2) @@ -148,12 +158,11 @@ def test_user_information_page_displays_if_there_are_failed_login_attempts( return_value={'organisations': [], 'services': []}, autospec=True ) - client.login(platform_admin_user) - response = client.get(url_for('main.user_information', user_id=fake_uuid)) - assert response.status_code == 200 + page = client_request.get('main.user_information', user_id=fake_uuid) - document = html.fromstring(response.get_data(as_text=True)) - assert document.xpath("//p/text()[normalize-space()='2 failed login attempts']") + assert normalize_spaces(page.select('main p')[-1].text) == ( + '2 failed login attempts' + ) def test_user_information_page_shows_archive_link_for_active_users( @@ -174,19 +183,20 @@ def test_user_information_page_shows_archive_link_for_active_users( def test_user_information_page_does_not_show_archive_link_for_inactive_users( mocker, - client, + client_request, platform_admin_user, mock_get_organisations_and_services_for_user, ): - inactive_user = user_json(state='inactive') + inactive_user_id = uuid.uuid4() + inactive_user = user_json(id_=inactive_user_id, state='inactive') + client_request.login(platform_admin_user) mocker.patch('app.user_api_client.get_user', side_effect=[platform_admin_user, inactive_user], autospec=True) - client.login(platform_admin_user) - response = client.get( - url_for('main.user_information', user_id=inactive_user['id']) - ) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - archive_url = url_for('main.archive_user', user_id=inactive_user['id']) + page = client_request.get( + 'main.user_information', user_id=inactive_user_id + ) + + archive_url = url_for('main.archive_user', user_id=inactive_user_id) assert not page.find('a', {'href': archive_url}) diff --git a/tests/app/main/views/test_letters.py b/tests/app/main/views/test_letters.py index e806d7767..6c3b71956 100644 --- a/tests/app/main/views/test_letters.py +++ b/tests/app/main/views/test_letters.py @@ -34,7 +34,7 @@ def test_letters_access_restricted( @pytest.mark.parametrize('url', letters_urls) def test_letters_lets_in_without_permission( - client, + client_request, mocker, mock_login, mock_has_permissions, @@ -46,11 +46,10 @@ def test_letters_lets_in_without_permission( service_one['permissions'] = ['letter'] mocker.patch('app.service_api_client.get_service', return_value={"data": service_one}) - client.login(api_user_active) - response = client.get(url(service_id=service_one['id'])) + client_request.login(api_user_active) + client_request.get_url(url(service_id=service_one['id'])) assert api_user_active['permissions'] == {} - assert response.status_code == 200 @pytest.mark.parametrize('permissions, choices', [ diff --git a/tests/app/main/views/test_providers.py b/tests/app/main/views/test_providers.py index 4ac011f70..9f3c886cc 100644 --- a/tests/app/main/views/test_providers.py +++ b/tests/app/main/views/test_providers.py @@ -1,10 +1,8 @@ import copy -import re from datetime import datetime from unittest.mock import call import pytest -from bs4 import BeautifulSoup from flask import url_for from freezegun import freeze_time @@ -146,15 +144,14 @@ stub_provider_history = { def test_should_show_all_providers( - client, + client_request, platform_admin_user, mocker, ): mocker.patch('app.provider_client.get_all_providers', return_value=copy.deepcopy(stub_providers)) - client.login(platform_admin_user, mocker) - response = client.get(url_for('main.view_providers')) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + client_request.login(platform_admin_user) + page = client_request.get('main.view_providers') h1 = [header.text.strip() for header in page.find_all('h1')] @@ -259,17 +256,15 @@ def test_add_monthly_traffic(): def test_should_show_edit_provider_form( - client, + client_request, platform_admin_user, mocker, fake_uuid, ): mocker.patch('app.provider_client.get_provider_by_id', return_value=copy.deepcopy(stub_provider)) - client.login(platform_admin_user, mocker) - response = client.get(url_for('main.edit_provider', provider_id=fake_uuid)) - - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + client_request.login(platform_admin_user) + page = client_request.get('main.edit_provider', provider_id=fake_uuid) h1 = [header.text.strip() for header in page.find_all('h1')] @@ -283,102 +278,115 @@ def test_should_show_edit_provider_form( def test_should_show_error_on_bad_provider_priority( - client, + client_request, platform_admin_user, mocker, ): mocker.patch('app.provider_client.get_provider_by_id', return_value=copy.deepcopy(stub_provider)) - client.login(platform_admin_user, mocker) - response = client.post( - url_for('main.edit_provider', provider_id=stub_provider['provider_details']['id']), - data={'priority': "not valid"}) + client_request.login(platform_admin_user) + page = client_request.post( + 'main.edit_provider', + provider_id=stub_provider['provider_details']['id'], + _data={'priority': "not valid"}, + _expected_status=200, + ) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert response.status_code == 200 - assert "Not a valid integer value" in str(page.find_all("span", {"class": re.compile(r"error-message")})[0]) + assert normalize_spaces( + page.select_one('.govuk-error-message').text + ) == "Error: Not a valid integer value." def test_should_show_error_on_negative_provider_priority( - client, + client_request, platform_admin_user, mocker, ): mocker.patch('app.provider_client.get_provider_by_id', return_value=copy.deepcopy(stub_provider)) - client.login(platform_admin_user, mocker) - response = client.post( - url_for('main.edit_provider', provider_id=stub_provider['provider_details']['id']), - data={'priority': -1}) + client_request.login(platform_admin_user) + page = client_request.post( + 'main.edit_provider', + provider_id=stub_provider['provider_details']['id'], + _data={'priority': -1}, + _expected_status=200, + ) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert response.status_code == 200 - assert "Must be between 1 and 100" in str(page.find_all("span", {"class": re.compile(r"error-message")})[0]) + assert normalize_spaces( + page.select_one('.govuk-error-message').text + ) == "Error: Must be between 1 and 100" def test_should_show_error_on_too_big_provider_priority( - client, + client_request, platform_admin_user, mocker, ): mocker.patch('app.provider_client.get_provider_by_id', return_value=copy.deepcopy(stub_provider)) - client.login(platform_admin_user, mocker) - response = client.post( - url_for('main.edit_provider', provider_id=stub_provider['provider_details']['id']), - data={'priority': 101}) + client_request.login(platform_admin_user) + page = client_request.post( + 'main.edit_provider', + provider_id=stub_provider['provider_details']['id'], + _data={'priority': 101}, + _expected_status=200, + ) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert response.status_code == 200 - assert "Must be between 1 and 100" in str(page.find_all("span", {"class": re.compile(r"error-message")})[0]) + assert normalize_spaces( + page.select_one('.govuk-error-message').text + ) == "Error: Must be between 1 and 100" def test_should_show_error_on_too_little_provider_priority( - client, + client_request, platform_admin_user, mocker, ): mocker.patch('app.provider_client.get_provider_by_id', return_value=copy.deepcopy(stub_provider)) - client.login(platform_admin_user, mocker) - response = client.post( - url_for('main.edit_provider', provider_id=stub_provider['provider_details']['id']), - data={'priority': 0}) + client_request.login(platform_admin_user) + page = client_request.post( + 'main.edit_provider', + provider_id=stub_provider['provider_details']['id'], + _data={'priority': 0}, + _expected_status=200, + ) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert response.status_code == 200 - assert "Must be between 1 and 100" in str(page.find_all("span", {"class": re.compile(r"error-message")})[0]) + assert normalize_spaces( + page.select_one('.govuk-error-message').text + ) == "Error: Must be between 1 and 100" def test_should_update_provider_priority( - client, + client_request, platform_admin_user, mocker, ): mocker.patch('app.provider_client.get_provider_by_id', return_value=copy.deepcopy(stub_provider)) mocker.patch('app.provider_client.update_provider', return_value=copy.deepcopy(stub_provider)) - client.login(platform_admin_user, mocker) - response = client.post( - url_for('main.edit_provider', provider_id=stub_provider['provider_details']['id']), - data={'priority': 2}) + client_request.login(platform_admin_user) + client_request.post( + 'main.edit_provider', + provider_id=stub_provider['provider_details']['id'], + _data={'priority': 2}, + _expected_redirect='http://localhost/providers', + ) app.provider_client.update_provider.assert_called_with(stub_provider['provider_details']['id'], 2) - assert response.status_code == 302 - assert response.location == 'http://localhost/providers' def test_should_show_provider_version_history( - client, + client_request, platform_admin_user, mocker ): mocker.patch('app.provider_client.get_provider_versions', return_value=copy.deepcopy(stub_provider_history)) - client.login(platform_admin_user, mocker) - response = client.get( - url_for('main.view_provider', provider_id=stub_provider_history['data'][0]['id'])) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + client_request.login(platform_admin_user) + page = client_request.get( + 'main.view_provider', provider_id=stub_provider_history['data'][0]['id'] + ) table = page.find('table') table_rows = table.find_all('tr') @@ -386,8 +394,6 @@ def test_should_show_provider_version_history( first_row = table_rows[1].find_all('td') second_row = table_rows[2].find_all('td') - assert response.status_code == 200 - assert page.find_all('h1')[0].text.strip() == stub_provider_history['data'][0]["display_name"] assert len(table_rows) == 3 diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 600f6cebc..d71e8a312 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -6,7 +6,6 @@ from urllib.parse import parse_qs, urlparse from uuid import UUID, uuid4 import pytest -from bs4 import BeautifulSoup from flask import url_for from freezegun import freeze_time from notifications_python_client.errors import HTTPError @@ -118,7 +117,7 @@ def mock_get_service_settings_page_common( ]), ]) def test_should_show_overview( - client, + client_request, mocker, api_user_active, no_reply_to_email_addresses, @@ -137,12 +136,11 @@ def test_should_show_overview( ) mocker.patch('app.service_api_client.get_service', return_value={'data': service_one}) - client.login(user, mocker, service_one) - response = client.get(url_for( + client_request.login(user, service_one) + page = client_request.get( 'main.service_settings', service_id=SERVICE_ONE_ID - )) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + ) + assert page.find('h1').text == 'Settings' rows = page.select('tr') assert len(rows) == len(expected_rows) @@ -152,7 +150,7 @@ def test_should_show_overview( def test_platform_admin_sees_only_relevant_settings_for_broadcast_service( - client, + client_request, mocker, api_user_active, no_reply_to_email_addresses, @@ -170,12 +168,11 @@ def test_platform_admin_sees_only_relevant_settings_for_broadcast_service( ) mocker.patch('app.service_api_client.get_service', return_value={'data': service_one}) - client.login(create_platform_admin_user(), mocker, service_one) - response = client.get(url_for( + client_request.login(create_platform_admin_user(), service_one) + page = client_request.get( 'main.service_settings', service_id=SERVICE_ONE_ID - )) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + ) + assert page.find('h1').text == 'Settings' rows = page.select('tr') @@ -212,7 +209,7 @@ def test_platform_admin_sees_only_relevant_settings_for_broadcast_service( ] ) def test_platform_admin_sees_correct_description_of_broadcast_service_setting( - client, + client_request, mocker, api_user_active, no_reply_to_email_addresses, @@ -236,12 +233,11 @@ def test_platform_admin_sees_correct_description_of_broadcast_service_setting( ) mocker.patch('app.service_api_client.get_service', return_value={'data': service_one}) - client.login(create_platform_admin_user(), mocker, service_one) - response = client.get(url_for( + client_request.login(create_platform_admin_user(), service_one) + page = client_request.get( 'main.service_settings', service_id=SERVICE_ONE_ID - )) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + ) + broadcast_setting_row = page.find(string=re.compile("Emergency alerts")).find_parent('tr') broadcast_setting_description = broadcast_setting_row.select('td')[1].text assert normalize_spaces(broadcast_setting_description) == expected_text @@ -398,7 +394,7 @@ def test_send_files_by_email_row_on_settings_page( ]), ]) def test_should_show_overview_for_service_with_more_things_set( - client, + client_request, active_user_with_permissions, mocker, service_one, @@ -410,13 +406,12 @@ def test_should_show_overview_for_service_with_more_things_set( permissions, expected_rows ): - client.login(active_user_with_permissions, mocker, service_one) + client_request.login(active_user_with_permissions) service_one['permissions'] = permissions service_one['email_branding'] = uuid4() - response = client.get(url_for( + page = client_request.get( 'main.service_settings', service_id=service_one['id'] - )) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + ) for index, row in enumerate(expected_rows): assert row == " ".join(page.find_all('tr')[index + 1].text.split()) diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index c6d75ada1..09bec2c77 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -1793,7 +1793,7 @@ def test_should_not_allow_template_edits_without_correct_permission( def test_should_403_when_edit_template_with_process_type_of_priority_for_non_platform_admin( - client, + client_request, active_user_with_permissions, mocker, mock_get_service_template, @@ -1802,7 +1802,7 @@ def test_should_403_when_edit_template_with_process_type_of_priority_for_non_pla service_one, ): service_one['users'] = [active_user_with_permissions] - client.login(active_user_with_permissions, mocker, service_one) + client_request.login(active_user_with_permissions) mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions]) template_id = fake_uuid data = { @@ -1813,16 +1813,18 @@ def test_should_403_when_edit_template_with_process_type_of_priority_for_non_pla 'service': service_one['id'], 'process_type': 'priority' } - response = client.post(url_for( + client_request.post( '.edit_service_template', service_id=service_one['id'], - template_id=template_id), data=data) - assert response.status_code == 403 + template_id=template_id, + _data=data, + _expected_status=403, + ) assert mock_update_service_template.called is False def test_should_403_when_create_template_with_process_type_of_priority_for_non_platform_admin( - client, + client_request, active_user_with_permissions, mocker, mock_get_service_template, @@ -1831,7 +1833,7 @@ def test_should_403_when_create_template_with_process_type_of_priority_for_non_p service_one, ): service_one['users'] = [active_user_with_permissions] - client.login(active_user_with_permissions, mocker, service_one) + client_request.login(active_user_with_permissions, service_one) mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions]) template_id = fake_uuid data = { @@ -1842,11 +1844,13 @@ def test_should_403_when_create_template_with_process_type_of_priority_for_non_p 'service': service_one['id'], 'process_type': 'priority' } - response = client.post(url_for( + client_request.post( '.add_service_template', service_id=service_one['id'], - template_type='sms'), data=data) - assert response.status_code == 403 + template_type='sms', + _data=data, + _expected_status=403, + ) assert mock_update_service_template.called is False diff --git a/tests/app/utils/test_user.py b/tests/app/utils/test_user.py index f06a50642..266eadfa3 100644 --- a/tests/app/utils/test_user.py +++ b/tests/app/utils/test_user.py @@ -7,7 +7,7 @@ from app.utils.user import user_has_permissions def _test_permissions( - client, + client_request, usr, permissions, will_succeed, @@ -15,7 +15,7 @@ def _test_permissions( ): request.view_args.update({'service_id': 'foo'}) if usr: - client.login(usr) + client_request.login(usr) decorator = user_has_permissions(*permissions, **(kwargs or {})) decorated_index = decorator(index) @@ -34,93 +34,93 @@ def _test_permissions( def test_user_has_permissions_on_endpoint_fail( - client, + client_request, mocker, mock_get_service, ): user = _user_with_permissions() mocker.patch('app.user_api_client.get_user', return_value=user) _test_permissions( - client, + client_request, user, ['send_messages'], will_succeed=False) def test_user_has_permissions_success( - client, + client_request, mocker, ): user = _user_with_permissions() mocker.patch('app.user_api_client.get_user', return_value=user) _test_permissions( - client, + client_request, user, ['manage_service'], will_succeed=True) def test_user_has_permissions_or( - client, + client_request, mocker, ): user = _user_with_permissions() mocker.patch('app.user_api_client.get_user', return_value=user) _test_permissions( - client, + client_request, user, ['send_messages', 'manage_service'], will_succeed=True) def test_user_has_permissions_multiple( - client, + client_request, mocker, ): user = _user_with_permissions() mocker.patch('app.user_api_client.get_user', return_value=user) _test_permissions( - client, + client_request, user, ['manage_templates', 'manage_service'], will_succeed=True) def test_exact_permissions( - client, + client_request, mocker, ): user = _user_with_permissions() mocker.patch('app.user_api_client.get_user', return_value=user) _test_permissions( - client, + client_request, user, ['manage_service', 'manage_templates'], will_succeed=True) def test_platform_admin_user_can_access_page_that_has_no_permissions( - client, + client_request, platform_admin_user, mocker, ): mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user) _test_permissions( - client, + client_request, platform_admin_user, [], will_succeed=True) def test_platform_admin_user_can_not_access_page( - client, + client_request, platform_admin_user, mocker, mock_get_service, ): mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user) _test_permissions( - client, + client_request, platform_admin_user, [], will_succeed=False, @@ -128,25 +128,24 @@ def test_platform_admin_user_can_not_access_page( def test_no_user_returns_401_unauth( - client + client_request ): - from flask_login import current_user - assert not current_user.is_authenticated + client_request.logout() _test_permissions( - client, + client_request, None, [], will_succeed=False) def test_user_has_permissions_for_organisation( - client, + client_request, mocker, ): user = _user_with_permissions() user['organisations'] = ['org_1', 'org_2'] mocker.patch('app.user_api_client.get_user', return_value=user) - client.login(user) + client_request.login(user) request.view_args = {'org_id': 'org_2'} @@ -158,13 +157,13 @@ def test_user_has_permissions_for_organisation( def test_platform_admin_can_see_orgs_they_dont_have( - client, + client_request, platform_admin_user, mocker, ): platform_admin_user['organisations'] = [] mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user) - client.login(platform_admin_user) + client_request.login(platform_admin_user) request.view_args = {'org_id': 'org_2'} @@ -176,12 +175,12 @@ def test_platform_admin_can_see_orgs_they_dont_have( def test_cant_use_decorator_without_view_args( - client, + client_request, platform_admin_user, mocker, ): mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user) - client.login(platform_admin_user) + client_request.login(platform_admin_user) request.view_args = {} @@ -194,13 +193,13 @@ def test_cant_use_decorator_without_view_args( def test_user_doesnt_have_permissions_for_organisation( - client, + client_request, mocker, ): user = _user_with_permissions() user['organisations'] = ['org_1', 'org_2'] mocker.patch('app.user_api_client.get_user', return_value=user) - client.login(user) + client_request.login(user) request.view_args = {'org_id': 'org_3'} @@ -213,12 +212,12 @@ def test_user_doesnt_have_permissions_for_organisation( def test_user_with_no_permissions_to_service_goes_to_templates( - client, + client_request, mocker ): user = _user_with_permissions() mocker.patch('app.user_api_client.get_user', return_value=user) - client.login(user) + client_request.login(user) request.view_args = {'service_id': 'bar'} @user_has_permissions()