diff --git a/app/templates/views/signin.html b/app/templates/views/signin.html index c4dd5c609..f53f94c15 100644 --- a/app/templates/views/signin.html +++ b/app/templates/views/signin.html @@ -3,7 +3,11 @@ {% from "components/form.html" import form_wrapper %} {% block per_page_title %} - Sign in + {% if again %} + You need to sign in again + {% else %} + Sign in + {% endif %} {% endblock %} {% block maincolumn_content %} diff --git a/tests/app/main/test_errorhandlers.py b/tests/app/main/test_errorhandlers.py index eeb6bac00..8f9b2237d 100644 --- a/tests/app/main/test_errorhandlers.py +++ b/tests/app/main/test_errorhandlers.py @@ -31,26 +31,26 @@ def test_load_service_before_request_handles_404(client_request, mocker): '/user-profile/email/confirm/MALFORMED_TOKEN', '/verify-email/MALFORMED_TOKEN' ]) -def test_malformed_token_returns_page_not_found(logged_in_client, url): - response = logged_in_client.get(url) +def test_malformed_token_returns_page_not_found(client_request, url): + page = client_request.get_url(url, _expected_status=404) - assert response.status_code == 404 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.string.strip() == 'Page not found' flash_banner = page.find('div', class_='banner-dangerous').string.strip() assert flash_banner == "There’s something wrong with the link you’ve used." assert page.title.string.strip() == 'Page not found – GOV.UK Notify' -def test_csrf_returns_400(logged_in_client, mocker): +def test_csrf_returns_400(client_request, mocker): # we turn off CSRF handling for tests, so fake a CSRF response here. csrf_err = CSRFError('400 Bad Request: The CSRF tokens do not match.') mocker.patch('app.main.views.index.render_template', side_effect=csrf_err) - response = logged_in_client.get('/cookies') + page = client_request.get_url( + '/cookies', + _expected_status=400, + _test_page_title=False, + ) - assert response.status_code == 400 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.string.strip() == 'Sorry, there’s a problem with GOV.UK Notify' assert page.title.string.strip() == 'Sorry, there’s a problem with the service – GOV.UK Notify' diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index b44741051..3f0f5e782 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -89,7 +89,6 @@ from tests.conftest import ( @freeze_time('2020-01-01 01:00') def test_can_show_notifications( client_request, - logged_in_client, service_one, mock_get_notifications, mock_get_service_statistics, @@ -183,12 +182,12 @@ def test_can_show_notifications( to=expected_to_argument, ) - json_response = logged_in_client.get(url_for( + json_response = client_request.get_response( 'main.get_notifications_as_json', service_id=service_one['id'], status=status_argument, **extra_args - )) + ) json_content = json.loads(json_response.get_data(as_text=True)) assert json_content.keys() == {'counts', 'notifications', 'service_data_retention_days'} diff --git a/tests/app/main/views/test_agreement.py b/tests/app/main/views/test_agreement.py index e92f79e06..dfbbbd539 100644 --- a/tests/app/main/views/test_agreement.py +++ b/tests/app/main/views/test_agreement.py @@ -141,7 +141,7 @@ def test_unknown_gps_and_trusts_are_redirected( ), )) def test_download_service_agreement( - logged_in_client, + client_request, mocker, mock_get_service_organisation, crown, @@ -160,11 +160,12 @@ def test_download_service_agreement( return_value=MockS3Object(b'foo') ) - response = logged_in_client.get(url_for( + response = client_request.get(url_for( 'main.service_download_agreement', service_id=SERVICE_ONE_ID, + _expected_status=expected_status, + _raw_respons=True, )) - assert response.status_code == expected_status if expected_file_served: assert response.get_data() == b'foo' diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index 21e2f815d..90145bc99 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -555,17 +555,15 @@ def test_broadcast_dashboard_has_new_alert_button_if_user_has_permission_to_crea @freeze_time('2020-02-20 02:20') def test_broadcast_dashboard_json( - logged_in_client, + client_request, service_one, mock_get_broadcast_messages, ): service_one['permissions'] += ['broadcast'] - response = logged_in_client.get(url_for( + response = client_request.get_response( '.broadcast_dashboard_updates', service_id=SERVICE_ONE_ID, - )) - - assert response.status_code == 200 + ) json_response = json.loads(response.get_data(as_text=True)) diff --git a/tests/app/main/views/test_conversation.py b/tests/app/main/views/test_conversation.py index bc1090ee5..3129d1ea9 100644 --- a/tests/app/main/views/test_conversation.py +++ b/tests/app/main/views/test_conversation.py @@ -175,7 +175,7 @@ def test_view_conversation( def test_view_conversation_updates( - logged_in_client, + client_request, mocker, fake_uuid, mock_get_inbound_sms_by_id_with_no_messages, @@ -191,13 +191,12 @@ def test_view_conversation_updates( return_value={'messages': 'foo'} ) - response = logged_in_client.get(url_for( + response = client_request.get_response( 'main.conversation_updates', service_id=SERVICE_ONE_ID, notification_id=fake_uuid, - )) + ) - assert response.status_code == 200 assert json.loads(response.get_data(as_text=True)) == {'messages': 'foo'} mock_get_partials.assert_called_once_with(SERVICE_ONE_ID, '07123 456789') diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index d35078553..8031482ca 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -103,17 +103,18 @@ stub_template_stats = [ create_active_caseworking_user(), )) def test_redirect_from_old_dashboard( - logged_in_client, + client_request, user, mocker, ): mocker.patch('app.user_api_client.get_user', return_value=user) expected_location = 'http://localhost/services/{}'.format(SERVICE_ONE_ID) - response = logged_in_client.get('/services/{}/dashboard'.format(SERVICE_ONE_ID)) + client_request.get_url( + '/services/{}/dashboard'.format(SERVICE_ONE_ID), + _expected_redirect=expected_location, + ) - assert response.status_code == 302 - assert response.location == expected_location assert expected_location == url_for('main.service_dashboard', service_id=SERVICE_ONE_ID, _external=True) @@ -393,7 +394,7 @@ def test_anyone_can_see_inbox( def test_view_inbox_updates( - logged_in_client, + client_request, service_one, mocker, mock_get_most_recent_inbound_sms_with_no_messages, @@ -405,11 +406,10 @@ def test_view_inbox_updates( return_value={'messages': 'foo'}, ) - response = logged_in_client.get(url_for( + response = client_request.get_response( 'main.inbox_updates', service_id=SERVICE_ONE_ID, - )) + ) - assert response.status_code == 200 assert json.loads(response.get_data(as_text=True)) == {'messages': 'foo'} mock_get_partials.assert_called_once_with(SERVICE_ONE_ID) @@ -417,13 +417,13 @@ def test_view_inbox_updates( @freeze_time("2016-07-01 13:00") def test_download_inbox( - logged_in_client, + client_request, mock_get_inbound_sms, ): - response = logged_in_client.get( - url_for('main.inbox_download', service_id=SERVICE_ONE_ID) + response = client_request.get_response( + 'main.inbox_download', + service_id=SERVICE_ONE_ID, ) - assert response.status_code == 200 assert response.headers['Content-Type'] == ( 'text/csv; ' 'charset=utf-8' @@ -456,7 +456,7 @@ def test_download_inbox( ]) def test_download_inbox_strips_formulae( mocker, - logged_in_client, + client_request, fake_uuid, message_content, expected_cell, @@ -475,8 +475,10 @@ def test_download_inbox_strips_formulae( }] }, ) - response = logged_in_client.get( - url_for('main.inbox_download', service_id=SERVICE_ONE_ID) + response = client_request.get( + 'main.inbox_download', + service_id=SERVICE_ONE_ID, + _raw_response=True, ) assert expected_cell in response.get_data(as_text=True).split('\r\n')[1] @@ -1138,12 +1140,16 @@ def test_usage_page_displays_letters_split_by_month_and_postage( def test_usage_page_with_year_argument( - logged_in_client, + client_request, mock_get_usage, mock_get_billable_units, mock_get_free_sms_fragment_limit, ): - assert logged_in_client.get(url_for('main.usage', service_id=SERVICE_ONE_ID, year=2000)).status_code == 200 + client_request.get( + 'main.usage', + service_id=SERVICE_ONE_ID, + year=2000, + ) mock_get_billable_units.assert_called_once_with(SERVICE_ONE_ID, 2000) mock_get_usage.assert_called_once_with(SERVICE_ONE_ID, 2000) mock_get_free_sms_fragment_limit.assert_called_with(SERVICE_ONE_ID, 2000) diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 92a3c001e..1712b7cc7 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -683,7 +683,7 @@ def test_dont_cancel_letter_job_when_to_early_to_cancel( @freeze_time("2016-01-01 00:00:00.000001") def test_should_show_updates_for_one_job_as_json( - logged_in_client, + client_request, service_one, active_user_with_permissions, mock_get_notifications, @@ -693,9 +693,13 @@ def test_should_show_updates_for_one_job_as_json( mocker, fake_uuid, ): - response = logged_in_client.get(url_for('main.view_job_updates', service_id=service_one['id'], job_id=fake_uuid)) + response = client_request.get( + 'main.view_job_updates', + service_id=service_one['id'], + job_id=fake_uuid, + _raw_response=True, + ) - assert response.status_code == 200 content = json.loads(response.get_data(as_text=True)) assert 'sending' in content['counts'] assert 'delivered' in content['counts'] @@ -710,7 +714,7 @@ def test_should_show_updates_for_one_job_as_json( @freeze_time("2016-01-01 00:00:00.000001") def test_should_show_updates_for_scheduled_job_as_json( - logged_in_client, + client_request, service_one, active_user_with_permissions, mock_get_notifications, @@ -727,9 +731,13 @@ def test_should_show_updates_for_scheduled_job_as_json( processing_started='2016-06-01T15:00:00+00:00', )}) - response = logged_in_client.get(url_for('main.view_job_updates', service_id=service_one['id'], job_id=fake_uuid)) + response = client_request.get( + 'main.view_job_updates', + service_id=service_one['id'], + job_id=fake_uuid, + _raw_response=True, + ) - assert response.status_code == 200 content = response.json assert 'sending' in content['counts'] assert 'delivered' in content['counts'] diff --git a/tests/app/main/views/test_notifications.py b/tests/app/main/views/test_notifications.py index 069b34d78..7086828ec 100644 --- a/tests/app/main/views/test_notifications.py +++ b/tests/app/main/views/test_notifications.py @@ -615,7 +615,7 @@ def test_notification_page_shows_page_for_other_postage_classes( create_active_caseworking_user(), ]) def test_should_show_image_of_letter_notification( - logged_in_client, + client_request, fake_uuid, mocker, filetype, @@ -633,19 +633,18 @@ def test_should_show_image_of_letter_notification( } ) - response = logged_in_client.get(url_for( + response = client_request.get_response( 'main.view_letter_notification_as_preview', service_id=SERVICE_ONE_ID, notification_id=fake_uuid, filetype=filetype - )) + ) - assert response.status_code == 200 assert response.get_data(as_text=True) == 'foo' def test_should_show_image_of_letter_notification_that_failed_validation( - logged_in_client, + client_request, fake_uuid, mocker ): @@ -665,20 +664,19 @@ def test_should_show_image_of_letter_notification_that_failed_validation( } ) - response = logged_in_client.get(url_for( + response = client_request.get_response( 'main.view_letter_notification_as_preview', service_id=SERVICE_ONE_ID, notification_id=fake_uuid, filetype='png', - with_metadata=True - )) + with_metadata=True, + ) - assert response.status_code == 200 assert response.get_data(as_text=True) == 'foo', metadata def test_should_show_preview_error_image_letter_notification_on_preview_error( - logged_in_client, + client_request, fake_uuid, mocker, ): @@ -692,14 +690,14 @@ def test_should_show_preview_error_image_letter_notification_on_preview_error( mocker.patch("builtins.open", mock_open(read_data=b"preview error image")) - response = logged_in_client.get(url_for( + response = client_request.get( 'main.view_letter_notification_as_preview', service_id=SERVICE_ONE_ID, notification_id=fake_uuid, - filetype='png' - )) + filetype='png', + _raw_response=True, + ) - assert response.status_code == 200 assert response.get_data(as_text=True) == 'preview error image' @@ -877,7 +875,7 @@ def test_notification_page_has_expected_template_link_for_letter( def test_should_show_image_of_precompiled_letter_notification( - logged_in_client, + client_request, fake_uuid, mocker, ): @@ -895,14 +893,14 @@ def test_should_show_image_of_precompiled_letter_notification( } ) - response = logged_in_client.get(url_for( + response = client_request.get( 'main.view_letter_notification_as_preview', service_id=SERVICE_ONE_ID, notification_id=fake_uuid, - filetype="png" - )) + filetype="png", + _raw_response=True, + ) - assert response.status_code == 200 assert response.get_data(as_text=True) == 'foo' assert mock_pdf_page_count.called_once() diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index e2e88148f..62f5e7956 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -11,7 +11,6 @@ from uuid import uuid4 from zipfile import BadZipFile import pytest -from bs4 import BeautifulSoup from flask import url_for from freezegun import freeze_time from notifications_python_client.errors import HTTPError @@ -386,14 +385,15 @@ def test_example_spreadsheet_for_letters( @pytest.mark.parametrize( - "filename, acceptable_file", - list(zip(test_spreadsheet_files, repeat(True))) + - list(zip(test_non_spreadsheet_files, repeat(False))) + "filename, acceptable_file, expected_status", + list(zip(test_spreadsheet_files, repeat(True), repeat(302))) + + list(zip(test_non_spreadsheet_files, repeat(False), repeat(200))) ) def test_upload_files_in_different_formats( filename, acceptable_file, - logged_in_client, + expected_status, + client_request, service_one, mocker, mock_get_service_template, @@ -402,12 +402,14 @@ def test_upload_files_in_different_formats( fake_uuid, ): with open(filename, 'rb') as uploaded: - response = logged_in_client.post( - url_for('main.send_messages', service_id=service_one['id'], template_id=fake_uuid), - data={'file': (BytesIO(uploaded.read()), filename)}, - content_type='multipart/form-data' + page = client_request.post( + 'main.send_messages', + service_id=service_one['id'], + template_id=fake_uuid, + _data={'file': (BytesIO(uploaded.read()), filename)}, + _content_type='multipart/form-data', + _expected_status=expected_status, ) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') if acceptable_file: assert mock_s3_upload.call_args[0][1]['data'].strip() == ( @@ -429,7 +431,7 @@ def test_upload_files_in_different_formats( def test_send_messages_sanitises_and_truncates_file_name_for_metadata( - logged_in_client, + client_request, service_one, mocker, mock_get_service_template_with_placeholders, @@ -442,11 +444,13 @@ def test_send_messages_sanitises_and_truncates_file_name_for_metadata( ): filename = f"😁{'a' * 2000}.csv" - logged_in_client.post( - url_for('main.send_messages', service_id=service_one['id'], template_id=fake_uuid), - data={'file': (BytesIO(''.encode('utf-8')), filename)}, - content_type='multipart/form-data', - follow_redirects=False + client_request.post( + 'main.send_messages', + service_id=service_one['id'], + template_id=fake_uuid, + _data={'file': (BytesIO(''.encode('utf-8')), filename)}, + _content_type='multipart/form-data', + _follow_redirects=False, ) assert len( @@ -484,7 +488,7 @@ def test_send_messages_sanitises_and_truncates_file_name_for_metadata( )), ]) def test_shows_error_if_parsing_exception( - logged_in_client, + client_request, mocker, mock_get_service_template, exception, @@ -500,12 +504,14 @@ def test_shows_error_if_parsing_exception( side_effect=_raise_exception_or_partial_exception ) - response = logged_in_client.post( - url_for('main.send_messages', service_id=SERVICE_ONE_ID, template_id=fake_uuid), - data={'file': (BytesIO(b'example'), 'example.xlsx')}, - content_type='multipart/form-data' + page = client_request.post( + 'main.send_messages', + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + _data={'file': (BytesIO(b'example'), 'example.xlsx')}, + _content_type='multipart/form-data', + _expected_status=200, ) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert normalize_spaces(page.select_one('.banner-dangerous').text) == ( expected_error_message @@ -513,7 +519,7 @@ def test_shows_error_if_parsing_exception( def test_upload_csv_file_with_errors_shows_check_page_with_errors( - logged_in_client, + client_request, service_one, mocker, mock_get_service_template_with_placeholders, @@ -536,31 +542,29 @@ def test_upload_csv_file_with_errors_shows_check_page_with_errors( """ ) - response = logged_in_client.post( - url_for('main.send_messages', service_id=service_one['id'], template_id=fake_uuid), - data={'file': (BytesIO(''.encode('utf-8')), 'example.csv')}, - content_type='multipart/form-data', - follow_redirects=True + page = client_request.post( + 'main.send_messages', + service_id=service_one['id'], + template_id=fake_uuid, + _data={'file': (BytesIO(''.encode('utf-8')), 'example.csv')}, + _content_type='multipart/form-data', + _follow_redirects=True, ) - with logged_in_client.session_transaction() as session: + with client_request.session_transaction() as session: assert 'file_uploads' not in session - assert response.status_code == 200 - - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.select_one('input[type=file]').has_attr('accept') assert page.select_one('input[type=file]')['accept'] == '.csv,.xlsx,.xls,.ods,.xlsm,.tsv' - content = response.get_data(as_text=True) - assert 'There’s a problem with example.csv' in content - assert '+447700900986' in content - assert 'Missing' in content - assert 'Upload your file again' in content + assert 'There’s a problem with example.csv' in page.text + assert '+447700900986' in page.text + assert 'Missing' in page.text + assert 'Upload your file again' in page.text def test_upload_csv_file_with_empty_message_shows_check_page_with_errors( - logged_in_client, + client_request, service_one, mocker, mock_get_empty_service_template_with_optional_placeholder, @@ -583,18 +587,18 @@ def test_upload_csv_file_with_empty_message_shows_check_page_with_errors( """ ) - response = logged_in_client.post( - url_for('main.send_messages', service_id=service_one['id'], template_id=fake_uuid), - data={'file': (BytesIO(''.encode('utf-8')), 'example.csv')}, - content_type='multipart/form-data', - follow_redirects=True + page = client_request.post( + 'main.send_messages', + service_id=service_one['id'], + template_id=fake_uuid, + _data={'file': (BytesIO(''.encode('utf-8')), 'example.csv')}, + _content_type='multipart/form-data', + _follow_redirects=True, ) - with logged_in_client.session_transaction() as session: + with client_request.session_transaction() as session: assert 'file_uploads' not in session - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert normalize_spaces( page.select_one('.banner-dangerous').text ) == ( @@ -617,7 +621,7 @@ def test_upload_csv_file_with_empty_message_shows_check_page_with_errors( def test_upload_csv_file_with_very_long_placeholder_shows_check_page_with_errors( - logged_in_client, + client_request, service_one, mocker, mock_get_service_template_with_placeholders, @@ -640,18 +644,18 @@ def test_upload_csv_file_with_very_long_placeholder_shows_check_page_with_errors """ ) - response = logged_in_client.post( - url_for('main.send_messages', service_id=service_one['id'], template_id=fake_uuid), - data={'file': (BytesIO(''.encode('utf-8')), 'example.csv')}, - content_type='multipart/form-data', - follow_redirects=True + page = client_request.post( + 'main.send_messages', + service_id=service_one['id'], + template_id=fake_uuid, + _data={'file': (BytesIO(''.encode('utf-8')), 'example.csv')}, + _content_type='multipart/form-data', + _follow_redirects=True ) - with logged_in_client.session_transaction() as session: + with client_request.session_transaction() as session: assert 'file_uploads' not in session - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert normalize_spaces( page.select_one('.banner-dangerous').text ) == ( @@ -676,7 +680,7 @@ def test_upload_csv_file_with_very_long_placeholder_shows_check_page_with_errors def test_upload_csv_file_with_bad_postal_address_shows_check_page_with_errors( - logged_in_client, + client_request, service_one, mocker, mock_get_service_letter_template, @@ -704,15 +708,15 @@ def test_upload_csv_file_with_bad_postal_address_shows_check_page_with_errors( ''' ) - response = logged_in_client.post( - url_for('main.send_messages', service_id=service_one['id'], template_id=fake_uuid), - data={'file': (BytesIO(''.encode('utf-8')), 'example.csv')}, - content_type='multipart/form-data', - follow_redirects=True + page = client_request.post( + 'main.send_messages', + service_id=service_one['id'], + template_id=fake_uuid, + _data={'file': (BytesIO(''.encode('utf-8')), 'example.csv')}, + _content_type='multipart/form-data', + _follow_redirects=True, ) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert normalize_spaces( page.select_one('.banner-dangerous').text ) == ( @@ -740,7 +744,7 @@ def test_upload_csv_file_with_bad_postal_address_shows_check_page_with_errors( def test_upload_csv_file_with_international_letters_permission_shows_appropriate_errors( - logged_in_client, + client_request, service_one, mocker, mock_get_service_letter_template, @@ -766,15 +770,15 @@ def test_upload_csv_file_with_international_letters_permission_shows_appropriate ''' ) - response = logged_in_client.post( - url_for('main.send_messages', service_id=service_one['id'], template_id=fake_uuid), - data={'file': (BytesIO(''.encode('utf-8')), 'example.csv')}, - content_type='multipart/form-data', - follow_redirects=True + page = client_request.post( + 'main.send_messages', + service_id=service_one['id'], + template_id=fake_uuid, + _data={'file': (BytesIO(''.encode('utf-8')), 'example.csv')}, + _content_type='multipart/form-data', + _follow_redirects=True ) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert normalize_spaces( page.select_one('.banner-dangerous').text ) == ( @@ -949,41 +953,43 @@ def test_upload_csv_file_with_missing_columns_shows_error( def test_upload_csv_invalid_extension( - logged_in_client, + client_request, mock_login, service_one, mock_get_service_template, fake_uuid, ): - resp = logged_in_client.post( - url_for('main.send_messages', service_id=service_one['id'], template_id=fake_uuid), - data={'file': (BytesIO('contents'.encode('utf-8')), 'invalid.txt')}, - content_type='multipart/form-data', - follow_redirects=True + page = client_request.post( + 'main.send_messages', + service_id=service_one['id'], + template_id=fake_uuid, + _data={'file': (BytesIO('contents'.encode('utf-8')), 'invalid.txt')}, + _content_type='multipart/form-data', + _follow_redirects=True, ) - assert resp.status_code == 200 - assert "invalid.txt is not a spreadsheet that Notify can read" in resp.get_data(as_text=True) + assert "invalid.txt is not a spreadsheet that Notify can read" in page.text def test_upload_csv_size_too_big( - logged_in_client, + client_request, mock_login, service_one, mock_get_service_template, fake_uuid, ): - resp = logged_in_client.post( - url_for('main.send_messages', service_id=service_one['id'], template_id=fake_uuid), - data={'file': (BytesIO(randbytes(11_000_000)), 'invalid.csv')}, - content_type='multipart/form-data', - follow_redirects=True + page = client_request.post( + 'main.send_messages', + service_id=service_one['id'], + template_id=fake_uuid, + _data={'file': (BytesIO(randbytes(11_000_000)), 'invalid.csv')}, + _content_type='multipart/form-data', + _follow_redirects=True, ) - assert resp.status_code == 200 - assert "File must be smaller than 10Mb" in resp.get_data(as_text=True) + assert "File must be smaller than 10Mb" in page.text def test_upload_valid_csv_redirects_to_check_page( @@ -1324,7 +1330,7 @@ def test_send_one_off_does_not_send_without_the_correct_permissions( create_active_caseworking_user(), )) def test_send_one_off_has_correct_page_title( - logged_in_client, + client_request, service_one, mock_has_no_jobs, mock_get_no_contact_lists, @@ -1337,18 +1343,13 @@ def test_send_one_off_has_correct_page_title( mocker.patch('app.service_api_client.get_service_template', return_value={'data': template_data}) mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=9) - response = logged_in_client.get( - url_for( - 'main.send_one_off', - service_id=service_one['id'], - template_id=fake_uuid, - step_index=0 - ), - follow_redirects=True, + page = client_request.get( + 'main.send_one_off', + service_id=service_one['id'], + template_id=fake_uuid, + step_index=0, + _follow_redirects=True, ) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - - assert response.status_code == 200 assert page.h1.text.strip() == "Send ‘Two week reminder’" assert len(page.select('.banner-tour')) == 0 @@ -2453,7 +2454,7 @@ def test_send_one_off_letter_address_goes_to_next_placeholder(client_request, mo def test_download_example_csv( - logged_in_client, + client_request, mocker, api_user_active, mock_login, @@ -2462,12 +2463,11 @@ def test_download_example_csv( mock_has_permissions, fake_uuid ): - - response = logged_in_client.get( + response = client_request.get( url_for('main.get_example_csv', service_id=fake_uuid, template_id=fake_uuid), - follow_redirects=True + follow_redirects=True, + _raw_response=True, ) - assert response.status_code == 200 assert response.get_data(as_text=True) == ( 'phone number,name,date\r\n' '07700 900321,example,example\r\n' @@ -2476,7 +2476,7 @@ def test_download_example_csv( def test_upload_csvfile_with_valid_phone_shows_all_numbers( - logged_in_client, + client_request, mock_get_service_template, mock_get_users_by_service, mock_get_live_service, @@ -2497,13 +2497,15 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers( ]) ) mock_get_notification_count = mocker.patch('app.service_api_client.get_notification_count', return_value=0) - response = logged_in_client.post( - url_for('main.send_messages', service_id=service_one['id'], template_id=fake_uuid), - data={'file': (BytesIO(''.encode('utf-8')), 'example.csv')}, - content_type='multipart/form-data', - follow_redirects=True + page = client_request.post( + 'main.send_messages', + service_id=service_one['id'], + template_id=fake_uuid, + _data={'file': (BytesIO(''.encode('utf-8')), 'example.csv')}, + _content_type='multipart/form-data', + _follow_redirects=True, ) - with logged_in_client.session_transaction() as session: + with client_request.session_transaction() as session: assert 'file_uploads' not in session assert mock_s3_set_metadata.call_count == 2 @@ -2521,12 +2523,10 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers( original_file_name='example.csv', ) - content = response.get_data(as_text=True) - assert response.status_code == 200 - assert '07700 900701' in content - assert '07700 900749' in content - assert '07700 900750' not in content - assert 'Only showing the first 50 rows' in content + assert '07700 900701' in page.text + assert '07700 900749' in page.text + assert '07700 900750' not in page.text + assert 'Only showing the first 50 rows' in page.text mock_get_notification_count.assert_called_once_with(service_one['id']) @@ -2538,7 +2538,7 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers( def test_upload_csvfile_with_international_validates( mocker, api_user_active, - logged_in_client, + client_request, mock_get_service_template, mock_s3_set_metadata, mock_s3_get_metadata, @@ -2565,14 +2565,15 @@ def test_upload_csvfile_with_international_validates( )), ) - response = logged_in_client.post( - url_for('main.send_messages', service_id=fake_uuid, template_id=fake_uuid), - data={'file': (BytesIO(''.encode('utf-8')), 'example.csv')}, - content_type='multipart/form-data', - follow_redirects=True, + client_request.post( + 'main.send_messages', + service_id=fake_uuid, + template_id=fake_uuid, + _data={'file': (BytesIO(''.encode('utf-8')), 'example.csv')}, + _content_type='multipart/form-data', + _follow_redirects=True, ) - assert response.status_code == 200 assert mock_recipients.call_args[1]['allow_international_sms'] == should_allow_international @@ -2885,21 +2886,19 @@ def test_should_show_preview_letter_message( def test_dont_show_preview_letter_templates_for_bad_filetype( - logged_in_client, + client_request, mock_get_service_template, service_one, fake_uuid ): - resp = logged_in_client.get( - url_for( - 'no_cookie.check_messages_preview', - service_id=service_one['id'], - template_id=fake_uuid, - upload_id=fake_uuid, - filetype='blah' - ) + client_request.get_response( + 'no_cookie.check_messages_preview', + service_id=service_one['id'], + template_id=fake_uuid, + upload_id=fake_uuid, + filetype='blah', + _expected_status=404, ) - assert resp.status_code == 404 assert mock_get_service_template.called is False diff --git a/tests/app/main/views/test_sign_in.py b/tests/app/main/views/test_sign_in.py index 6a62b62af..2ddf888c3 100644 --- a/tests/app/main/views/test_sign_in.py +++ b/tests/app/main/views/test_sign_in.py @@ -46,17 +46,16 @@ def test_sign_in_explains_session_timeout(client): assert 'We signed you out because you have not used Notify for a while.' in response.get_data(as_text=True) -def test_sign_in_explains_other_browser(logged_in_client, api_user_active, mocker): +def test_sign_in_explains_other_browser(client_request, api_user_active, mocker): api_user_active['current_session_id'] = str(uuid.UUID(int=1)) mocker.patch('app.user_api_client.get_user', return_value=api_user_active) - with logged_in_client.session_transaction() as session: + with client_request.session_transaction() as session: session['current_session_id'] = str(uuid.UUID(int=2)) - response = logged_in_client.get(url_for('main.sign_in', next='/foo')) + page = client_request.get('main.sign_in', next='/foo') - assert response.status_code == 200 - assert 'We signed you out because you logged in to Notify on another device' in response.get_data(as_text=True) + assert 'We signed you out because you logged in to Notify on another device' in page.text def test_doesnt_redirect_to_sign_in_if_no_session_info( @@ -79,7 +78,7 @@ def test_doesnt_redirect_to_sign_in_if_no_session_info( (uuid.UUID(int=1), uuid.UUID(int=2)), # BAD - this person has just signed in on a different browser ]) def test_redirect_to_sign_in_if_logged_in_from_other_browser( - logged_in_client, + client_request, api_user_active, mocker, db_sess_id, @@ -87,12 +86,14 @@ def test_redirect_to_sign_in_if_logged_in_from_other_browser( ): api_user_active['current_session_id'] = db_sess_id mocker.patch('app.user_api_client.get_user', return_value=api_user_active) - with logged_in_client.session_transaction() as session: + with client_request.session_transaction() as session: session['current_session_id'] = str(cookie_sess_id) - response = logged_in_client.get(url_for('main.choose_account')) - assert response.status_code == 302 - assert response.location == url_for('main.sign_in', next='/accounts', _external=True) + client_request.get( + 'main.choose_account', + _expected_status=302, + _expected_redirect=url_for('main.sign_in', next='/accounts', _external=True), + ) def test_logged_in_user_redirects_to_account( diff --git a/tests/app/main/views/test_template_history.py b/tests/app/main/views/test_template_history.py index f649bbe6d..f04ad5121 100644 --- a/tests/app/main/views/test_template_history.py +++ b/tests/app/main/views/test_template_history.py @@ -2,7 +2,7 @@ from flask import url_for def test_view_template_version( - logged_in_client, + client_request, api_user_active, mock_login, mock_get_service, @@ -20,18 +20,17 @@ def test_view_template_version( service_id=service_id, template_id=template_id ) - resp = logged_in_client.get(url_for( + page = client_request.get( '.view_template_version', service_id=service_id, template_id=template_id, - version=version)) + version=version, + ) - assert resp.status_code == 200 - resp_data = resp.get_data(as_text=True) template = mock_get_template_version(service_id, template_id, version) - assert api_user_active['name'] in resp_data - assert template['data']['content'] in resp_data - assert all_versions_link in resp_data + assert api_user_active['name'] in page.text + assert template['data']['content'] in page.text + assert all_versions_link in str(page) mock_get_template_version.assert_called_with( service_id, template_id, @@ -40,7 +39,7 @@ def test_view_template_version( def test_view_template_versions( - logged_in_client, + client_request, api_user_active, mock_login, mock_get_service, @@ -53,17 +52,15 @@ def test_view_template_versions( ): service_id = fake_uuid template_id = fake_uuid - resp = logged_in_client.get(url_for( + page = client_request.get( '.view_template_versions', service_id=service_id, - template_id=template_id - )) + template_id=template_id, + ) - assert resp.status_code == 200 - resp_data = resp.get_data(as_text=True) versions = mock_get_template_versions(service_id, template_id) - assert api_user_active['name'] in resp_data - assert versions['data'][0]['content'] in resp_data + assert api_user_active['name'] in page.text + assert versions['data'][0]['content'] in page.text mock_get_template_versions.assert_called_with( service_id, template_id diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index ab0298d4c..898f77d56 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -1114,7 +1114,7 @@ def test_should_show_preview_letter_templates( view, extra_view_args, filetype, - logged_in_client, + client_request, mock_get_service_email_template, service_one, fake_uuid, @@ -1127,15 +1127,14 @@ def test_should_show_preview_letter_templates( service_id, template_id = service_one['id'], fake_uuid - response = logged_in_client.get(url_for( + response = client_request.get_response( view, service_id=service_id, template_id=template_id, filetype=filetype, **extra_view_args - )) + ) - assert response.status_code == 200 assert response.get_data(as_text=True) == 'foo' mock_get_service_email_template.assert_called_with(service_id, template_id, extra_view_args.get('version')) assert mocked_preview.call_args[0][0]['id'] == template_id @@ -1144,20 +1143,18 @@ def test_should_show_preview_letter_templates( def test_dont_show_preview_letter_templates_for_bad_filetype( - logged_in_client, + client_request, mock_get_service_template, service_one, fake_uuid ): - resp = logged_in_client.get( - url_for( - 'no_cookie.view_letter_template_preview', - service_id=service_one['id'], - template_id=fake_uuid, - filetype='blah' - ) + client_request.get_response( + 'no_cookie.view_letter_template_preview', + service_id=service_one['id'], + template_id=fake_uuid, + filetype='blah', + _expected_status=404, ) - assert resp.status_code == 404 assert mock_get_service_template.called is False @@ -2817,7 +2814,7 @@ def test_should_not_create_broadcast_template_with_placeholders( ), ) def test_content_count_json_endpoint( - logged_in_client, + client_request, service_one, template_type, prefix_sms, @@ -2826,17 +2823,15 @@ def test_content_count_json_endpoint( expected_class, ): service_one['prefix_sms'] = prefix_sms - response = logged_in_client.post( - url_for( - 'main.count_content_length', - service_id=SERVICE_ONE_ID, - template_type=template_type, - ), - data={ + response = client_request.post_response( + 'main.count_content_length', + service_id=SERVICE_ONE_ID, + template_type=template_type, + _data={ 'template_content': content, }, + _expected_status=200, ) - assert response.status_code == 200 html = json.loads(response.get_data(as_text=True))['html'] snippet = BeautifulSoup(html, 'html.parser').select_one('span') diff --git a/tests/app/main/views/test_tour.py b/tests/app/main/views/test_tour.py index 643f74e41..10999cc16 100644 --- a/tests/app/main/views/test_tour.py +++ b/tests/app/main/views/test_tour.py @@ -669,7 +669,7 @@ def test_shows_link_to_end_tour( def test_go_to_dashboard_after_tour_link( - logged_in_client, + client_request, mocker, api_user_active, mock_login, @@ -678,11 +678,15 @@ def test_go_to_dashboard_after_tour_link( mock_delete_service_template, fake_uuid ): - - resp = logged_in_client.get( - url_for('main.go_to_dashboard_after_tour', service_id=fake_uuid, example_template_id=fake_uuid) + client_request.get( + 'main.go_to_dashboard_after_tour', + service_id=fake_uuid, + example_template_id=fake_uuid, + _expected_redirect=url_for( + "main.service_dashboard", + service_id=fake_uuid, + _external=True, + ) ) - assert resp.status_code == 302 - assert resp.location == url_for("main.service_dashboard", service_id=fake_uuid, _external=True) mock_delete_service_template.assert_called_once_with(fake_uuid, fake_uuid) diff --git a/tests/app/main/views/test_two_factor.py b/tests/app/main/views/test_two_factor.py index b071e8821..14e966827 100644 --- a/tests/app/main/views/test_two_factor.py +++ b/tests/app/main/views/test_two_factor.py @@ -487,11 +487,10 @@ def test_two_factor_email_link_when_user_is_locked_out( def test_two_factor_email_link_used_when_user_already_logged_in( - logged_in_client, + client_request, valid_token ): - response = logged_in_client.post( - url_for_endpoint_with_token('main.two_factor_email', token=valid_token) + client_request.post_url( + url_for_endpoint_with_token('main.two_factor_email', token=valid_token), + _expected_redirect=url_for('main.show_accounts_or_dashboard', _external=True), ) - assert response.status_code == 302 - assert response.location == url_for('main.show_accounts_or_dashboard', _external=True) diff --git a/tests/app/main/views/test_user_profile.py b/tests/app/main/views/test_user_profile.py index 0368f8fca..21866e775 100644 --- a/tests/app/main/views/test_user_profile.py +++ b/tests/app/main/views/test_user_profile.py @@ -162,18 +162,20 @@ def test_should_render_change_email_continue_after_authenticate_email( def test_should_redirect_to_user_profile_when_user_confirms_email_link( notify_admin, - logged_in_client, + client_request, api_user_active, mock_update_user_attribute, ): token = generate_token(payload=json.dumps({'user_id': api_user_active['id'], 'email': 'new_email@gov.uk'}), secret=notify_admin.config['SECRET_KEY'], salt=notify_admin.config['DANGEROUS_SALT']) - response = logged_in_client.get(url_for_endpoint_with_token('main.user_profile_email_confirm', - token=token)) - - assert response.status_code == 302 - assert response.location == url_for('main.user_profile', _external=True) + client_request.get_url( + url_for_endpoint_with_token( + 'main.user_profile_email_confirm', + token=token, + ), + _expected_redirect=url_for('main.user_profile', _external=True), + ) def test_should_show_mobile_number_page( diff --git a/tests/app/main/views/uploads/test_upload_contact_list.py b/tests/app/main/views/uploads/test_upload_contact_list.py index 9d79f6fa5..45db1e48a 100644 --- a/tests/app/main/views/uploads/test_upload_contact_list.py +++ b/tests/app/main/views/uploads/test_upload_contact_list.py @@ -652,7 +652,7 @@ def test_view_contact_list_404s_for_non_existing_list( def test_download_contact_list( mocker, - logged_in_client, + client_request, fake_uuid, mock_get_contact_list, ): @@ -660,12 +660,12 @@ def test_download_contact_list( 'app.models.contact_list.s3download', return_value='phone number\n07900900321' ) - response = logged_in_client.get(url_for( + response = client_request.get( 'main.download_contact_list', service_id=SERVICE_ONE_ID, contact_list_id=fake_uuid, - )) - assert response.status_code == 200 + _raw_response=True, + ) assert response.headers['Content-Type'] == ( 'text/csv; ' 'charset=utf-8' diff --git a/tests/app/main/views/uploads/test_upload_letter.py b/tests/app/main/views/uploads/test_upload_letter.py index 20802ba3d..a0d13afc9 100644 --- a/tests/app/main/views/uploads/test_upload_letter.py +++ b/tests/app/main/views/uploads/test_upload_letter.py @@ -492,7 +492,7 @@ def test_uploaded_letter_preview_redirects_if_file_not_in_s3( )) def test_uploaded_letter_preview_image_shows_overlay_when_content_outside_printable_area_on_a_page( mocker, - logged_in_client, + client_request, mock_get_service, fake_uuid, invalid_pages, @@ -515,13 +515,11 @@ def test_uploaded_letter_preview_image_shows_overlay_when_content_outside_printa return_value=make_response('page.html', 200) ) - logged_in_client.get( - url_for( - 'main.view_letter_upload_as_preview', - file_id=fake_uuid, - service_id=SERVICE_ONE_ID, - page=page_requested, - ) + client_request.get_response( + 'main.view_letter_upload_as_preview', + file_id=fake_uuid, + service_id=SERVICE_ONE_ID, + page=page_requested, ) if overlay_expected: @@ -541,7 +539,7 @@ def test_uploaded_letter_preview_image_shows_overlay_when_content_outside_printa ) def test_uploaded_letter_preview_image_does_not_show_overlay_if_no_content_outside_printable_area( mocker, - logged_in_client, + client_request, mock_get_service, metadata, fake_uuid, @@ -554,8 +552,11 @@ def test_uploaded_letter_preview_image_does_not_show_overlay_if_no_content_outsi 'app.main.views.uploads.TemplatePreview.from_valid_pdf_file', return_value=make_response('page.html', 200)) - logged_in_client.get( - url_for('main.view_letter_upload_as_preview', file_id=fake_uuid, service_id=SERVICE_ONE_ID, page=1) + client_request.get_response( + 'main.view_letter_upload_as_preview', + file_id=fake_uuid, + service_id=SERVICE_ONE_ID, + page=1, ) template_preview_mock.assert_called_once_with('pdf_file', 1) diff --git a/tests/app/notify_client/test_notification_client.py b/tests/app/notify_client/test_notification_client.py index 408d548ce..744f025c8 100644 --- a/tests/app/notify_client/test_notification_client.py +++ b/tests/app/notify_client/test_notification_client.py @@ -76,7 +76,7 @@ def test_client_gets_notifications_for_service_and_job_by_page_posts_for_to(mock mock_post.assert_called_once_with(**expected_call) -def test_send_notification(mocker, logged_in_client, active_user_with_permissions): +def test_send_notification(mocker, client_request, active_user_with_permissions): mock_post = mocker.patch('app.notify_client.notification_api_client.NotificationApiClient.post') NotificationApiClient().send_notification( 'foo', @@ -96,7 +96,7 @@ def test_send_notification(mocker, logged_in_client, active_user_with_permission ) -def test_send_precompiled_letter(mocker, logged_in_client, active_user_with_permissions): +def test_send_precompiled_letter(mocker, client_request, active_user_with_permissions): mock_post = mocker.patch('app.notify_client.notification_api_client.NotificationApiClient.post') NotificationApiClient().send_precompiled_letter( 'abcd-1234', diff --git a/tests/conftest.py b/tests/conftest.py index 028d98905..b87432186 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2745,7 +2745,7 @@ def client(notify_admin): @pytest.fixture(scope='function') -def logged_in_client( +def _logged_in_client( client, active_user_with_permissions, mocker, @@ -2782,22 +2782,22 @@ def os_environ(): @pytest.fixture # noqa (C901 too complex) -def client_request(logged_in_client, mocker, service_one): # noqa (C901 too complex) +def client_request(_logged_in_client, mocker, service_one): # noqa (C901 too complex) class ClientRequest: @staticmethod @contextmanager def session_transaction(): - with logged_in_client.session_transaction() as session: + with _logged_in_client.session_transaction() as session: yield session @staticmethod def login(user, service=service_one): - logged_in_client.login(user, mocker, service) + _logged_in_client.login(user, mocker, service) @staticmethod def logout(): - logged_in_client.logout(None) + _logged_in_client.logout(None) @staticmethod def get( @@ -2832,7 +2832,7 @@ def client_request(logged_in_client, mocker, service_one): # noqa (C901 too com _raw_response=False, **endpoint_kwargs ): - resp = logged_in_client.get( + resp = _logged_in_client.get( url, follow_redirects=_follow_redirects, ) @@ -2887,14 +2887,33 @@ def client_request(logged_in_client, mocker, service_one): # noqa (C901 too com _raw_response=False, _content_type=None, **endpoint_kwargs + ): + return ClientRequest.post_url( + url_for(endpoint, **(endpoint_kwargs or {})), + _data=_data, + _expected_status=_expected_status, + _follow_redirects=_follow_redirects, + _expected_redirect=_expected_redirect, + _content_type=_content_type, + ) + + @staticmethod + def post_url( + url, + _data=None, + _expected_status=None, + _follow_redirects=False, + _expected_redirect=None, + _raw_response=False, + _content_type=None, ): if _expected_status is None: _expected_status = 200 if _follow_redirects else 302 post_kwargs = {} if _content_type: post_kwargs.update(content_type=_content_type) - resp = logged_in_client.post( - url_for(endpoint, **(endpoint_kwargs or {})), + resp = _logged_in_client.post( + url, data=_data, follow_redirects=_follow_redirects, **post_kwargs @@ -2913,12 +2932,32 @@ def client_request(logged_in_client, mocker, service_one): # noqa (C901 too com _optional_args="", **endpoint_kwargs ): - resp = logged_in_client.get( + resp = _logged_in_client.get( url_for(endpoint, **(endpoint_kwargs or {})) + _optional_args, ) assert resp.status_code == _expected_status return resp + @staticmethod + def post_response( + endpoint, + _data=None, + _expected_status=302, + _optional_args="", + _content_type=None, + **endpoint_kwargs + ): + post_kwargs = {} + if _content_type: + post_kwargs.update(content_type=_content_type) + resp = _logged_in_client.post( + url_for(endpoint, **(endpoint_kwargs or {})) + _optional_args, + data=_data, + **post_kwargs + ) + assert resp.status_code == _expected_status + return resp + return ClientRequest