From 0f90bde9583baa3af79ed5d1b9eae705902879c9 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Tue, 6 Nov 2018 14:22:22 +0000 Subject: [PATCH] Store the sender_id that should be used for a job in S3 metadata Currently, a user can select a reply-to email address or text message sender when uploading a CSV file but this is ignored and the default is always used instead. As a first step towards changing this, this adds the sender_id (if selected) to the S3 metadata so that this information can be used when processing the job. --- app/main/views/send.py | 32 ++++++++++++++++------- tests/app/main/views/test_send.py | 42 +++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 9 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index 7f5ab7fba..cfab9b585 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -115,9 +115,16 @@ def send_messages(service_id, template_id): session['file_uploads'].keys()) ) - session['sender_id'] = None db_template = service_api_client.get_service_template(service_id, template_id)['data'] + email_reply_to = None + sms_sender = None + + if db_template['template_type'] == 'email': + email_reply_to = get_email_reply_to_address_from_session() + elif db_template['template_type'] == 'sms': + sms_sender = get_sms_sender_from_session() + if email_or_sms_not_enabled(db_template['template_type'], current_service.permissions): return redirect(url_for( '.action_blocked', @@ -139,6 +146,8 @@ def send_messages(service_id, template_id): filetype='png', page_count=get_page_count_for_letter(db_template), ), + email_reply_to=email_reply_to, + sms_sender=sms_sender, ) form = CsvUploadForm() @@ -626,17 +635,20 @@ def check_messages(service_id, template_id, upload_id, row_index=2): data['original_file_name'] = SanitiseASCII.encode(data.get('original_file_name', '')) - set_metadata_on_csv_upload( - service_id, - upload_id, - notification_count=data['count_of_recipients'], - template_id=str(template_id), - valid=True, - original_file_name=unicode_truncate( + metadata_kwargs = { + 'notification_count': data['count_of_recipients'], + 'template_id': str(template_id), + 'valid': True, + 'original_file_name': unicode_truncate( data['original_file_name'], 1600, ), - ) + } + + if session.get('sender_id'): + metadata_kwargs['sender_id'] = session['sender_id'] + + set_metadata_on_csv_upload(service_id, upload_id, **metadata_kwargs) return render_template('views/check/ok.html', **data) @@ -696,6 +708,8 @@ def start_job(service_id, upload_id): scheduled_for=request.form.get('scheduled_for', '') ) + session.pop('sender_id', None) + return redirect( url_for( 'main.view_job', diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index be9fef204..3325501c4 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -2659,6 +2659,48 @@ def test_check_messages_column_error_doesnt_show_optional_columns( ) +def test_check_messages_adds_sender_id_in_session_to_metadata( + client_request, + mocker, + mock_get_live_service, + mock_get_service_template, + mock_get_users_by_service, + mock_get_service_statistics, + mock_get_job_doesnt_exist, + mock_s3_set_metadata, + fake_uuid, +): + mocker.patch('app.main.views.send.s3download', return_value=( + 'phone number,\n07900900321' + )) + mocker.patch('app.main.views.send.get_sms_sender_from_session') + + with client_request.session_transaction() as session: + session['file_uploads'] = { + fake_uuid: {'template_id': fake_uuid} + } + session['sender_id'] = 'fake-sender' + + client_request.get( + 'main.check_messages', + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + upload_id=fake_uuid, + original_file_name='example.csv', + _test_page_title=False, + ) + + mock_s3_set_metadata.assert_called_once_with( + SERVICE_ONE_ID, + fake_uuid, + notification_count=1, + template_id=fake_uuid, + sender_id='fake-sender', + valid=True, + original_file_name='example.csv', + ) + + @pytest.mark.parametrize('extra_args', ( {}, {'from_test': True},