mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-08-20 14:29:51 -04:00
Merge pull request #1968 from alphagov/fixed-session-overriding-while-uploading-csvs-2
Fixed sessions overriding while uploading csvs
This commit is contained in:
Chris Waszczuk
@@ -262,12 +262,8 @@ def test_should_not_allow_files_to_be_uploaded_without_the_correct_permission(
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"filename, acceptable_file",
|
||||
list(zip(
|
||||
test_spreadsheet_files, repeat(True)
|
||||
)) +
|
||||
list(zip(
|
||||
test_non_spreadsheet_files, repeat(False)
|
||||
))
|
||||
list(zip(test_spreadsheet_files, repeat(True))) +
|
||||
list(zip(test_non_spreadsheet_files, repeat(False)))
|
||||
)
|
||||
def test_upload_files_in_different_formats(
|
||||
filename,
|
||||
@@ -279,7 +275,6 @@ def test_upload_files_in_different_formats(
|
||||
mock_s3_upload,
|
||||
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),
|
||||
@@ -383,12 +378,14 @@ def test_upload_csvfile_with_errors_shows_check_page_with_errors(
|
||||
content_type='multipart/form-data',
|
||||
follow_redirects=True
|
||||
)
|
||||
|
||||
reupload = logged_in_client.post(
|
||||
url_for('main.check_messages', service_id=service_one['id'], template_type='sms', upload_id='abc123'),
|
||||
url_for('main.check_messages', service_id=service_one['id'], template_type='sms', upload_id=fake_uuid),
|
||||
data={'file': (BytesIO(''.encode('utf-8')), 'invalid.csv')},
|
||||
content_type='multipart/form-data',
|
||||
follow_redirects=True
|
||||
)
|
||||
|
||||
for response in [initial_upload, reupload]:
|
||||
assert response.status_code == 200
|
||||
content = response.get_data(as_text=True)
|
||||
@@ -579,7 +576,9 @@ def test_upload_valid_csv_shows_preview_and_table(
|
||||
):
|
||||
|
||||
with client_request.session_transaction() as session:
|
||||
session['upload_data'] = {'template_id': fake_uuid}
|
||||
session['file_uploads'] = {
|
||||
fake_uuid: {'template_id': fake_uuid}
|
||||
}
|
||||
|
||||
mocker.patch('app.main.views.send.s3download', return_value="""
|
||||
phone number,name,thing,thing,thing
|
||||
@@ -666,7 +665,9 @@ def test_show_all_columns_if_there_are_duplicate_recipient_columns(
|
||||
):
|
||||
|
||||
with client_request.session_transaction() as session:
|
||||
session['upload_data'] = {'template_id': fake_uuid}
|
||||
session['file_uploads'] = {
|
||||
fake_uuid: {'template_id': fake_uuid}
|
||||
}
|
||||
|
||||
mocker.patch('app.main.views.send.s3download', return_value="""
|
||||
phone number, phone_number, PHONENUMBER
|
||||
@@ -710,7 +711,9 @@ def test_404_for_previewing_a_row_out_of_range(
|
||||
):
|
||||
|
||||
with client_request.session_transaction() as session:
|
||||
session['upload_data'] = {'template_id': fake_uuid}
|
||||
session['file_uploads'] = {
|
||||
fake_uuid: {'template_id': fake_uuid}
|
||||
}
|
||||
|
||||
mocker.patch('app.main.views.send.s3download', return_value="""
|
||||
phone number,name,thing,thing,thing
|
||||
@@ -1443,9 +1446,9 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers(
|
||||
follow_redirects=True
|
||||
)
|
||||
with logged_in_client.session_transaction() as sess:
|
||||
assert sess['upload_data']['template_id'] == fake_uuid
|
||||
assert sess['upload_data']['original_file_name'] == 'valid.csv'
|
||||
assert sess['upload_data']['notification_count'] == 53
|
||||
assert sess['file_uploads'][fake_uuid]['template_id'] == fake_uuid
|
||||
assert sess['file_uploads'][fake_uuid]['original_file_name'] == 'valid.csv'
|
||||
assert sess['file_uploads'][fake_uuid]['notification_count'] == 53
|
||||
|
||||
content = response.get_data(as_text=True)
|
||||
assert response.status_code == 200
|
||||
@@ -1503,14 +1506,16 @@ def test_test_message_can_only_be_sent_now(
|
||||
mock_get_detailed_service_for_today,
|
||||
fake_uuid
|
||||
):
|
||||
|
||||
with logged_in_client.session_transaction() as session:
|
||||
session['upload_data'] = {
|
||||
'original_file_name': 'Test message',
|
||||
'template_id': fake_uuid,
|
||||
'notification_count': 1,
|
||||
'valid': True
|
||||
session['file_uploads'] = {
|
||||
fake_uuid: {
|
||||
'original_file_name': 'Test message',
|
||||
'template_id': fake_uuid,
|
||||
'notification_count': 1,
|
||||
'valid': True
|
||||
}
|
||||
}
|
||||
|
||||
response = logged_in_client.get(url_for(
|
||||
'main.check_messages',
|
||||
service_id=service_one['id'],
|
||||
@@ -1537,11 +1542,13 @@ def test_letter_can_only_be_sent_now(
|
||||
mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=1)
|
||||
|
||||
with logged_in_client.session_transaction() as session:
|
||||
session['upload_data'] = {
|
||||
'original_file_name': 'Test message',
|
||||
'template_id': fake_uuid,
|
||||
'notification_count': 1,
|
||||
'valid': True
|
||||
session['file_uploads'] = {
|
||||
fake_uuid: {
|
||||
'original_file_name': 'Test message',
|
||||
'template_id': fake_uuid,
|
||||
'notification_count': 1,
|
||||
'valid': True
|
||||
}
|
||||
}
|
||||
|
||||
response = logged_in_client.get(url_for(
|
||||
@@ -1577,12 +1584,15 @@ def test_create_job_should_call_api(
|
||||
template_id = data['template']
|
||||
notification_count = data['notification_count']
|
||||
with logged_in_client.session_transaction() as session:
|
||||
session['upload_data'] = {
|
||||
'original_file_name': original_file_name,
|
||||
'template_id': template_id,
|
||||
'notification_count': notification_count,
|
||||
'valid': True
|
||||
session['file_uploads'] = {
|
||||
fake_uuid: {
|
||||
'original_file_name': original_file_name,
|
||||
'template_id': template_id,
|
||||
'notification_count': notification_count,
|
||||
'valid': True
|
||||
}
|
||||
}
|
||||
|
||||
url = url_for('main.start_job', service_id=service_one['id'], upload_id=job_id)
|
||||
response = logged_in_client.post(url, data={'scheduled_for': when}, follow_redirects=True)
|
||||
|
||||
@@ -1604,14 +1614,16 @@ def test_can_start_letters_job(
|
||||
service_one,
|
||||
fake_uuid
|
||||
):
|
||||
|
||||
with logged_in_platform_admin_client.session_transaction() as session:
|
||||
session['upload_data'] = {
|
||||
'original_file_name': 'example.csv',
|
||||
'template_id': fake_uuid,
|
||||
'notification_count': 123,
|
||||
'valid': True
|
||||
session['file_uploads'] = {
|
||||
fake_uuid: {
|
||||
'original_file_name': 'example.csv',
|
||||
'template_id': fake_uuid,
|
||||
'notification_count': 123,
|
||||
'valid': True
|
||||
}
|
||||
}
|
||||
|
||||
response = logged_in_platform_admin_client.post(
|
||||
url_for('main.start_job', service_id=service_one['id'], upload_id=fake_uuid),
|
||||
data={}
|
||||
@@ -1667,12 +1679,15 @@ def test_should_show_preview_letter_message(
|
||||
service_id = service_one['id']
|
||||
template_id = fake_uuid
|
||||
with logged_in_platform_admin_client.session_transaction() as session:
|
||||
session['upload_data'] = {
|
||||
'original_file_name': 'example.csv',
|
||||
'template_id': fake_uuid,
|
||||
'notification_count': 1,
|
||||
'valid': True
|
||||
session['file_uploads'] = {
|
||||
fake_uuid: {
|
||||
'original_file_name': 'example.csv',
|
||||
'template_id': fake_uuid,
|
||||
'notification_count': 1,
|
||||
'valid': True
|
||||
}
|
||||
}
|
||||
|
||||
response = logged_in_platform_admin_client.get(
|
||||
url_for(
|
||||
'main.check_messages_preview',
|
||||
@@ -1736,10 +1751,15 @@ def test_check_messages_should_revalidate_file_when_uploading_file(
|
||||
)
|
||||
data = mock_get_job(SERVICE_ONE_ID, fake_uuid)['data']
|
||||
with logged_in_client.session_transaction() as session:
|
||||
session['upload_data'] = {'original_file_name': 'invalid.csv',
|
||||
'template_id': data['template'],
|
||||
'notification_count': data['notification_count'],
|
||||
'valid': True}
|
||||
session['file_uploads'] = {
|
||||
fake_uuid: {
|
||||
'original_file_name': 'invalid.csv',
|
||||
'template_id': data['template'],
|
||||
'notification_count': data['notification_count'],
|
||||
'valid': True
|
||||
}
|
||||
}
|
||||
|
||||
response = logged_in_client.post(
|
||||
url_for('main.start_job', service_id=SERVICE_ONE_ID, upload_id=data['id']),
|
||||
data={'file': (BytesIO(''.encode('utf-8')), 'invalid.csv')},
|
||||
@@ -1901,10 +1921,15 @@ def test_check_messages_back_link(
|
||||
):
|
||||
template_mock(mocker)
|
||||
with logged_in_client.session_transaction() as session:
|
||||
session['upload_data'] = {'original_file_name': 'valid.csv',
|
||||
'template_id': fake_uuid,
|
||||
'notification_count': 1,
|
||||
'valid': True}
|
||||
session['file_uploads'] = {
|
||||
fake_uuid: {
|
||||
'original_file_name': 'valid.csv',
|
||||
'template_id': fake_uuid,
|
||||
'notification_count': 1,
|
||||
'valid': True
|
||||
}
|
||||
}
|
||||
|
||||
response = logged_in_client.get(url_for(
|
||||
'main.check_messages',
|
||||
service_id=fake_uuid,
|
||||
@@ -1990,10 +2015,15 @@ def test_check_messages_shows_too_many_messages_errors(
|
||||
})
|
||||
|
||||
with logged_in_client.session_transaction() as session:
|
||||
session['upload_data'] = {'original_file_name': 'valid.csv',
|
||||
'template_id': fake_uuid,
|
||||
'notification_count': 1,
|
||||
'valid': True}
|
||||
session['file_uploads'] = {
|
||||
fake_uuid: {
|
||||
'original_file_name': 'valid.csv',
|
||||
'template_id': fake_uuid,
|
||||
'notification_count': 1,
|
||||
'valid': True
|
||||
}
|
||||
}
|
||||
|
||||
response = logged_in_client.get(url_for(
|
||||
'main.check_messages',
|
||||
service_id=fake_uuid,
|
||||
@@ -2018,18 +2048,25 @@ def test_check_messages_shows_trial_mode_error(
|
||||
mock_get_service_template,
|
||||
mock_has_permissions,
|
||||
mock_get_detailed_service_for_today,
|
||||
fake_uuid,
|
||||
mocker
|
||||
):
|
||||
mocker.patch('app.main.views.send.s3download', return_value=(
|
||||
'phone number,\n07900900321' # Not in team
|
||||
))
|
||||
|
||||
with logged_in_client.session_transaction() as session:
|
||||
session['upload_data'] = {'template_id': ''}
|
||||
session['file_uploads'] = {
|
||||
fake_uuid: {
|
||||
'template_id': '',
|
||||
}
|
||||
}
|
||||
|
||||
response = logged_in_client.get(url_for(
|
||||
'main.check_messages',
|
||||
service_id=uuid.uuid4(),
|
||||
template_type='sms',
|
||||
upload_id=uuid.uuid4()
|
||||
upload_id=fake_uuid
|
||||
))
|
||||
assert response.status_code == 200
|
||||
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
|
||||
@@ -2057,13 +2094,13 @@ def test_check_messages_shows_trial_mode_error_for_letters(
|
||||
mock_has_permissions,
|
||||
mock_get_users_by_service,
|
||||
mock_get_detailed_service_for_today,
|
||||
fake_uuid,
|
||||
mocker,
|
||||
service_mock,
|
||||
error_should_be_shown,
|
||||
number_of_rows,
|
||||
expected_error_message,
|
||||
):
|
||||
|
||||
service_mock(mocker, api_user_active)
|
||||
|
||||
mocker.patch('app.main.views.send.s3download', return_value='\n'.join(
|
||||
@@ -2072,13 +2109,17 @@ def test_check_messages_shows_trial_mode_error_for_letters(
|
||||
))
|
||||
|
||||
with client_request.session_transaction() as session:
|
||||
session['upload_data'] = {'template_id': ''}
|
||||
session['file_uploads'] = {
|
||||
fake_uuid: {
|
||||
'template_id': '',
|
||||
}
|
||||
}
|
||||
|
||||
page = client_request.get(
|
||||
'main.check_messages',
|
||||
service_id=SERVICE_ONE_ID,
|
||||
template_type='letter',
|
||||
upload_id=uuid.uuid4(),
|
||||
upload_id=fake_uuid,
|
||||
_test_page_title=False,
|
||||
)
|
||||
|
||||
@@ -2100,6 +2141,7 @@ def test_check_messages_shows_data_errors_before_trial_mode_errors_for_letters(
|
||||
mock_get_service_letter_template,
|
||||
mock_has_permissions,
|
||||
mock_get_users_by_service,
|
||||
fake_uuid,
|
||||
mock_get_detailed_service_for_today,
|
||||
):
|
||||
|
||||
@@ -2109,13 +2151,18 @@ def test_check_messages_shows_data_errors_before_trial_mode_errors_for_letters(
|
||||
))
|
||||
|
||||
with client_request.session_transaction() as session:
|
||||
session['upload_data'] = {'template_id': '', 'original_file_name': 'example.xlsx'}
|
||||
session['file_uploads'] = {
|
||||
fake_uuid: {
|
||||
'template_id': '',
|
||||
'original_file_name': 'example.xlsx',
|
||||
}
|
||||
}
|
||||
|
||||
page = client_request.get(
|
||||
'main.check_messages',
|
||||
service_id=SERVICE_ONE_ID,
|
||||
template_type='letter',
|
||||
upload_id=uuid.uuid4(),
|
||||
upload_id=fake_uuid,
|
||||
_test_page_title=False,
|
||||
)
|
||||
|
||||
@@ -2131,6 +2178,7 @@ def test_check_messages_column_error_doesnt_show_optional_columns(
|
||||
client_request,
|
||||
mock_get_service_letter_template,
|
||||
mock_has_permissions,
|
||||
fake_uuid,
|
||||
mock_get_users_by_service,
|
||||
mock_get_detailed_service_for_today,
|
||||
):
|
||||
@@ -2141,13 +2189,18 @@ def test_check_messages_column_error_doesnt_show_optional_columns(
|
||||
))
|
||||
|
||||
with client_request.session_transaction() as session:
|
||||
session['upload_data'] = {'template_id': '', 'original_file_name': ''}
|
||||
session['file_uploads'] = {
|
||||
fake_uuid: {
|
||||
'template_id': '',
|
||||
'original_file_name': '',
|
||||
}
|
||||
}
|
||||
|
||||
page = client_request.get(
|
||||
'main.check_messages',
|
||||
service_id=SERVICE_ONE_ID,
|
||||
template_type='letter',
|
||||
upload_id=uuid.uuid4(),
|
||||
upload_id=fake_uuid,
|
||||
_test_page_title=False,
|
||||
)
|
||||
|
||||
@@ -2164,6 +2217,7 @@ def test_generate_test_letter_doesnt_block_in_trial_mode(
|
||||
mock_get_service,
|
||||
mock_get_service_letter_template,
|
||||
mock_has_permissions,
|
||||
fake_uuid,
|
||||
mock_get_users_by_service,
|
||||
mock_get_detailed_service_for_today,
|
||||
):
|
||||
@@ -2174,13 +2228,17 @@ def test_generate_test_letter_doesnt_block_in_trial_mode(
|
||||
""")
|
||||
|
||||
with client_request.session_transaction() as session:
|
||||
session['upload_data'] = {'template_id': ''}
|
||||
session['file_uploads'] = {
|
||||
fake_uuid: {
|
||||
'template_id': '',
|
||||
}
|
||||
}
|
||||
|
||||
page = client_request.get(
|
||||
'main.check_messages',
|
||||
service_id=SERVICE_ONE_ID,
|
||||
template_type='letter',
|
||||
upload_id=uuid.uuid4(),
|
||||
upload_id=fake_uuid,
|
||||
from_test=True,
|
||||
_test_page_title=False,
|
||||
)
|
||||
@@ -2209,7 +2267,12 @@ def test_check_messages_shows_over_max_row_error(
|
||||
mock_recipients.too_many_rows.return_value = True
|
||||
|
||||
with logged_in_client.session_transaction() as session:
|
||||
session['upload_data'] = {'template_id': fake_uuid}
|
||||
session['file_uploads'] = {
|
||||
fake_uuid: {
|
||||
'template_id': fake_uuid,
|
||||
}
|
||||
}
|
||||
|
||||
response = logged_in_client.get(url_for(
|
||||
'main.check_messages',
|
||||
service_id=fake_uuid,
|
||||
@@ -2250,9 +2313,11 @@ def test_non_ascii_characters_in_letter_recipients_file_shows_error(
|
||||
)
|
||||
|
||||
with logged_in_client.session_transaction() as session:
|
||||
session['upload_data'] = {
|
||||
'template_id': fake_uuid,
|
||||
'original_file_name': 'unicode.csv',
|
||||
session['file_uploads'] = {
|
||||
fake_uuid: {
|
||||
'template_id': fake_uuid,
|
||||
'original_file_name': 'unicode.csv',
|
||||
}
|
||||
}
|
||||
|
||||
response = logged_in_client.get(url_for(
|
||||
@@ -2613,11 +2678,11 @@ def test_reply_to_is_previewed_if_chosen(
|
||||
mock_get_users_by_service,
|
||||
mock_get_detailed_service_for_today,
|
||||
get_default_reply_to_email_address,
|
||||
fake_uuid,
|
||||
endpoint,
|
||||
extra_args,
|
||||
reply_to_address,
|
||||
):
|
||||
|
||||
mocker.patch('app.main.views.send.s3download', return_value="""
|
||||
email_address,date,thing
|
||||
notify@digital.cabinet-office.gov.uk,foo,bar
|
||||
@@ -2626,11 +2691,13 @@ def test_reply_to_is_previewed_if_chosen(
|
||||
with client_request.session_transaction() as session:
|
||||
session['recipient'] = 'notify@digital.cabinet-office.gov.uk'
|
||||
session['placeholders'] = {}
|
||||
session['upload_data'] = {
|
||||
'original_file_name': 'example.csv',
|
||||
'template_id': fake_uuid(),
|
||||
'notification_count': 1,
|
||||
'valid': True
|
||||
session['file_uploads'] = {
|
||||
fake_uuid: {
|
||||
'original_file_name': 'example.csv',
|
||||
'template_id': fake_uuid,
|
||||
'notification_count': 1,
|
||||
'valid': True
|
||||
}
|
||||
}
|
||||
session['sender_id'] = reply_to_address
|
||||
|
||||
@@ -2664,6 +2731,7 @@ def test_sms_sender_is_previewed(
|
||||
mock_get_users_by_service,
|
||||
mock_get_detailed_service_for_today,
|
||||
get_default_sms_sender,
|
||||
fake_uuid,
|
||||
endpoint,
|
||||
extra_args,
|
||||
sms_sender,
|
||||
@@ -2677,11 +2745,13 @@ def test_sms_sender_is_previewed(
|
||||
with client_request.session_transaction() as session:
|
||||
session['recipient'] = '7700900986'
|
||||
session['placeholders'] = {}
|
||||
session['upload_data'] = {
|
||||
'original_file_name': 'example.csv',
|
||||
'template_id': fake_uuid(),
|
||||
'notification_count': 1,
|
||||
'valid': True
|
||||
session['file_uploads'] = {
|
||||
fake_uuid: {
|
||||
'original_file_name': 'example.csv',
|
||||
'template_id': fake_uuid,
|
||||
'notification_count': 1,
|
||||
'valid': True
|
||||
}
|
||||
}
|
||||
session['sender_id'] = sms_sender
|
||||
|
||||
@@ -2697,3 +2767,74 @@ def test_sms_sender_is_previewed(
|
||||
assert sms_sender_on_page.text.strip() == 'From: GOVUK'
|
||||
else:
|
||||
assert not sms_sender_on_page
|
||||
|
||||
|
||||
@pytest.mark.parametrize('endpoint, request_type, extra_args', [
|
||||
(
|
||||
'main.check_messages',
|
||||
'GET',
|
||||
{
|
||||
'template_type': 'email',
|
||||
'upload_id': fake_uuid(),
|
||||
}
|
||||
),
|
||||
(
|
||||
'main.check_messages_preview',
|
||||
'GET',
|
||||
{
|
||||
'template_type': 'email',
|
||||
'upload_id': fake_uuid(),
|
||||
'filetype': 'png'
|
||||
}
|
||||
),
|
||||
(
|
||||
'main.recheck_messages',
|
||||
'POST',
|
||||
{
|
||||
'template_type': 'email',
|
||||
'upload_id': fake_uuid()
|
||||
}
|
||||
),
|
||||
(
|
||||
'main.start_job',
|
||||
'POST',
|
||||
{
|
||||
'upload_id': fake_uuid()
|
||||
}
|
||||
)
|
||||
])
|
||||
@pytest.mark.parametrize('session_data', [
|
||||
{},
|
||||
{
|
||||
'6ce466d0-fd6a-11e5-82f5-e0accb9d11a4': {
|
||||
'template_id': '1234'
|
||||
}
|
||||
}
|
||||
])
|
||||
def test_redirects_to_choose_template_if_no_session_exists_for_upload_id(
|
||||
client_request,
|
||||
endpoint,
|
||||
request_type,
|
||||
session_data,
|
||||
extra_args,
|
||||
fake_uuid
|
||||
):
|
||||
with client_request.session_transaction() as session:
|
||||
session['file_uploads'] = session_data
|
||||
|
||||
if request_type == 'GET':
|
||||
client_request.get(
|
||||
endpoint,
|
||||
service_id=SERVICE_ONE_ID,
|
||||
**extra_args,
|
||||
_expected_status=301,
|
||||
_expected_redirect=url_for('main.choose_template', service_id=SERVICE_ONE_ID, _external=True)
|
||||
)
|
||||
else:
|
||||
client_request.post(
|
||||
endpoint,
|
||||
service_id=SERVICE_ONE_ID,
|
||||
**extra_args,
|
||||
_expected_status=301,
|
||||
_expected_redirect=url_for('main.choose_template', service_id=SERVICE_ONE_ID, _external=True)
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user