darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-07-12 09:24:15 -04:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #3707 from alphagov/filenames-metadata-2

Browse Source 
Stop checking the query string for original_file_name
This commit is contained in:
Katie Smith
2020-11-05 10:28:47 +00:00
committed by GitHub
parent cebdf7afa1 247e7c2d93
commit 9ab3cb1d99
4 changed files with 4 additions and 218 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
app/main/views/send.py
View File

@@ -668,10 +668,7 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_
abort(404)
page_count = get_page_count_for_letter(db_template, template.values)
# TODO: stop checking the request.args for original_file_name once this change has been deployed for a
# while and we are confident it's only coming from metadata
metadata = get_csv_metadata(service_id, upload_id)
original_file_name = metadata.get('original_file_name', request.args.get('original_file_name', ''))
original_file_name = get_csv_metadata(service_id, upload_id).get('original_file_name', '')
return dict(
recipients=recipients,
@@ -732,16 +729,11 @@ def check_messages(service_id, template_id, upload_id, row_index=2):
):
return render_template('views/check/column-errors.html', **data)
data['original_file_name'] = SanitiseASCII.encode(data.get('original_file_name', ''))
metadata_kwargs = {
'notification_count': data['count_of_recipients'],
'template_id': template_id,
'valid': True,
'original_file_name': unicode_truncate(
data['original_file_name'],
1600,
),
'original_file_name': data.get('original_file_name', ''),
}
if session.get('sender_id'):

14
app/main/views/uploads.py
View File

@@ -427,20 +427,13 @@ def check_contact_list(service_id, upload_id):
contents = ContactList.download(service_id, upload_id)
first_row = contents.splitlines()[0].strip().rstrip(',') if contents else ''
metadata = ContactList.get_metadata(service_id, upload_id)
original_file_name = ContactList.get_metadata(service_id, upload_id).get('original_file_name', '')
template_type = {
'emailaddress': 'email',
'phonenumber': 'sms',
}.get(Columns.make_key(first_row))
# TODO: stop looking in the query string for metadata once we are sure all uploaded
# contact lists now have original_file_name in the metadata
original_file_name = metadata.get(
'original_file_name',
SanitiseASCII.encode(request.args.get('original_file_name', ''))
)
recipients = RecipientCSV(
contents,
template=get_sample_template(template_type or 'sms'),
@@ -493,10 +486,7 @@ def check_contact_list(service_id, upload_id):
metadata_kwargs = {
'row_count': len(recipients),
'valid': True,
'original_file_name': unicode_truncate(
original_file_name,
1600,
),
'original_file_name': original_file_name,
'template_type': template_type
}

137
tests/app/main/views/test_send.py
View File

@@ -1,5 +1,4 @@
# -*- coding: utf-8 -*-
import sys
import uuid
from functools import partial
from glob import glob
@@ -1108,142 +1107,6 @@ def test_upload_valid_csv_only_sets_meta_if_filename_known(
assert len(mock_s3_set_metadata.call_args_list) == 0
def test_file_name_truncated_to_fit_in_s3_metadata(
client_request,
mocker,
mock_get_live_service,
mock_get_service_template_with_placeholders,
mock_get_users_by_service,
mock_get_service_statistics,
mock_get_job_doesnt_exist,
mock_get_jobs,
mock_s3_set_metadata,
fake_uuid,
):
with client_request.session_transaction() as session:
session['file_uploads'] = {
fake_uuid: {'template_id': fake_uuid}
}
mocker.patch('app.main.views.send.s3download', return_value="""
phone number,name,thing,thing,thing
07700900001, A, foo, foo, foo
""")
file_name = 'ü😁' * 2000
mocker.patch(
'app.main.views.send.get_csv_metadata',
return_value={'original_file_name': file_name},
)
client_request.get(
'main.check_messages',
service_id=SERVICE_ONE_ID,
template_id=fake_uuid,
upload_id=fake_uuid,
)
assert sys.getsizeof(
file_name.encode('utf-8')
) > 2000
assert sys.getsizeof(''.join((
'{}{}'.format(key, value) for key, value in
mock_s3_set_metadata.call_args_list[0][1].items()
)).encode('utf-8')) == 1726
def test_file_name_is_taken_from_query_string_if_not_in_metadata(
client_request,
mocker,
mock_get_live_service,
mock_get_service_template_with_placeholders,
mock_get_users_by_service,
mock_get_service_statistics,
mock_get_job_doesnt_exist,
mock_get_jobs,
mock_s3_set_metadata,
fake_uuid,
):
with client_request.session_transaction() as session:
session['file_uploads'] = {
fake_uuid: {'template_id': fake_uuid}
}
mocker.patch('app.main.views.send.s3download', return_value="""
phone number,name,thing,thing,thing
07700900001, A, foo, foo, foo
""")
mocker.patch(
'app.main.views.send.get_csv_metadata',
return_value={},
)
client_request.get(
'main.check_messages',
service_id=SERVICE_ONE_ID,
template_id=fake_uuid,
upload_id=fake_uuid,
original_file_name='example.csv',
)
mock_s3_set_metadata.assert_called_once_with(
SERVICE_ONE_ID,
fake_uuid,
notification_count=1,
template_id=fake_uuid,
valid=True,
original_file_name='example.csv',
)
def test_check_messages_replaces_invalid_characters_in_file_name(
client_request,
mocker,
mock_get_live_service,
mock_get_service_template_with_placeholders,
mock_get_users_by_service,
mock_get_service_statistics,
mock_get_job_doesnt_exist,
mock_get_jobs,
mock_s3_set_metadata,
fake_uuid,
):
with client_request.session_transaction() as session:
session['file_uploads'] = {
fake_uuid: {'template_id': fake_uuid}
}
mocker.patch('app.main.views.send.s3download', return_value="""
phone number,name,thing,thing,thing
07700900001, A, foo, foo, foo
""")
file_name = 'ü😁’€'
mocker.patch(
'app.main.views.send.get_csv_metadata',
return_value={'original_file_name': file_name},
)
client_request.get(
'main.check_messages',
service_id=SERVICE_ONE_ID,
template_id=fake_uuid,
upload_id=fake_uuid,
)
mock_s3_set_metadata.assert_called_once_with(
SERVICE_ONE_ID,
fake_uuid,
notification_count=1,
original_file_name="u?'?",
template_id=fake_uuid,
valid=True
)
def test_show_all_columns_if_there_are_duplicate_recipient_columns(
client_request,
mocker,

59
tests/app/main/views/uploads/test_upload_contact_list.py
View File

@@ -394,65 +394,6 @@ def test_upload_csv_shows_ok_page(
)
def test_upload_csv_shows_ok_page_when_filename_is_in_query_string(
client_request,
mock_get_live_service,
mock_get_users_by_service,
mock_get_job_doesnt_exist,
fake_uuid,
mocker
):
mocker.patch('app.models.contact_list.s3download', return_value='\n'.join(
['email address'] + ['test@example.com'] * 51
))
mocker.patch('app.models.contact_list.get_csv_metadata', return_value={})
mock_metadata_set = mocker.patch('app.models.contact_list.set_metadata_on_csv_upload')
page = client_request.get(
'main.check_contact_list',
service_id=SERVICE_ONE_ID,
upload_id=fake_uuid,
original_file_name='good times.xlsx',
_test_page_title=False,
)
mock_metadata_set.assert_called_once_with(
SERVICE_ONE_ID,
fake_uuid,
bucket='test-contact-list',
row_count=51,
original_file_name='good times.xlsx',
template_type='email',
valid=True,
)
assert normalize_spaces(page.select_one('h1').text) == (
'good times.xlsx'
)
assert normalize_spaces(page.select_one('main p').text) == (
'51 email addresses found'
)
assert page.select_one('form')['action'] == url_for(
'main.save_contact_list',
service_id=SERVICE_ONE_ID,
upload_id=fake_uuid,
)
assert normalize_spaces(page.select_one('form [type=submit]').text) == (
'Save contact list'
)
assert normalize_spaces(page.select_one('thead').text) == (
'Row in file 1 email address'
)
assert len(page.select('tbody tr')) == 50
assert normalize_spaces(page.select_one('tbody tr').text) == (
'2 test@example.com'
)
assert normalize_spaces(page.select_one('.table-show-more-link').text) == (
'Only showing the first 50 rows'
)
def test_save_contact_list(
mocker,
client_request,