mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-09-10 18:22:37 -04:00
Stop checking query string for filename when sending a job
We look for `original_file_name` in the metadata now. Initially we were still checking the query string too, but now that the change to add the filename to the metadata has been deployed for a while there shouldn't be any cases of the filename still being in a query string. Since the `original_file_name` is not being added to the metadata in `.check_messages` (it has happened earlier in the process) a few tests are no longer needed.
This commit is contained in:
+2
-10
@@ -668,10 +668,7 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_
|
|||||||
abort(404)
|
abort(404)
|
||||||
|
|
||||||
page_count = get_page_count_for_letter(db_template, template.values)
|
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
|
original_file_name = get_csv_metadata(service_id, upload_id).get('original_file_name', '')
|
||||||
# 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', ''))
|
|
||||||
|
|
||||||
return dict(
|
return dict(
|
||||||
recipients=recipients,
|
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)
|
return render_template('views/check/column-errors.html', **data)
|
||||||
|
|
||||||
data['original_file_name'] = SanitiseASCII.encode(data.get('original_file_name', ''))
|
|
||||||
|
|
||||||
metadata_kwargs = {
|
metadata_kwargs = {
|
||||||
'notification_count': data['count_of_recipients'],
|
'notification_count': data['count_of_recipients'],
|
||||||
'template_id': template_id,
|
'template_id': template_id,
|
||||||
'valid': True,
|
'valid': True,
|
||||||
'original_file_name': unicode_truncate(
|
'original_file_name': data.get('original_file_name', ''),
|
||||||
data['original_file_name'],
|
|
||||||
1600,
|
|
||||||
),
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if session.get('sender_id'):
|
if session.get('sender_id'):
|
||||||
|
|||||||
@@ -1,5 +1,4 @@
|
|||||||
# -*- coding: utf-8 -*-
|
# -*- coding: utf-8 -*-
|
||||||
import sys
|
|
||||||
import uuid
|
import uuid
|
||||||
from functools import partial
|
from functools import partial
|
||||||
from glob import glob
|
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
|
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(
|
def test_show_all_columns_if_there_are_duplicate_recipient_columns(
|
||||||
client_request,
|
client_request,
|
||||||
mocker,
|
mocker,
|
||||||
|
|||||||
Reference in New Issue
Block a user