diff --git a/app/main/views/send.py b/app/main/views/send.py index 5c8555970..7835c9955 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -178,7 +178,7 @@ def set_sender(service_id, template_id): template = service_api_client.get_service_template(service_id, template_id)['data'] - if template['template_type'] != 'email': + if template['template_type'] == 'letter': return redirect_to_one_off sender_details = get_sender_details(service_id, template['template_type']) @@ -192,7 +192,12 @@ def set_sender(service_id, template_id): sender_choices=sender_context['value_and_label'], sender_label=sender_context['description'] ) - option_hints = {sender_context['default_id']: 'Default'} + option_hints = {sender_context['default_id']: '(Default)', + } + if sender_context.get('receives_text_message', None): + option_hints.update({sender_context['receives_text_message']: '(Receives replies)'}) + if sender_context.get('default_and_receives', None): + option_hints = {sender_context['default_and_receives']: '(Default and receives replies)'} if form.validate_on_submit(): session['sender_id'] = form.sender.data @@ -220,12 +225,24 @@ def get_sender_context(sender_details, template_type): 'title': 'Choose sender address', 'description': 'Select an address that recipients can reply to', 'field_name': 'contact_block' + }, + 'sms': { + 'title': 'Chose text message sender', + 'description': 'Select a text message sender that the recipients can reply to', + 'field_name': 'sms_sender' } }[template_type] sender_format = context['field_name'] context['default_id'] = next(sender['id'] for sender in sender_details if sender['is_default']) + if template_type == 'sms': + inbound = [sender['id'] for sender in sender_details if sender['inbound_number_id']] + if inbound: + context['receives_text_message'] = next(iter(inbound)) + if context['default_id'] == context.get('receives_text_message', None): + context['default_and_receives'] = context['default_id'] + context['value_and_label'] = [(sender['id'], sender[sender_format]) for sender in sender_details] return context @@ -233,7 +250,8 @@ def get_sender_context(sender_details, template_type): def get_sender_details(service_id, template_type): api_call = { 'email': service_api_client.get_reply_to_email_addresses, - 'letter': service_api_client.get_letter_contacts + 'letter': service_api_client.get_letter_contacts, + 'sms': service_api_client.get_sms_senders }[template_type] return api_call(service_id) @@ -248,7 +266,7 @@ def send_test(service_id, template_id): session['send_test_letter_page_count'] = None db_template = service_api_client.get_service_template(service_id, template_id)['data'] - if db_template['template_type'] != 'email': + if db_template['template_type'] == 'letter': session['sender_id'] = None if email_or_sms_not_enabled(db_template['template_type'], current_service['permissions']): @@ -311,7 +329,12 @@ def send_test_step(service_id, template_id, step_index): if not session.get('send_test_letter_page_count'): session['send_test_letter_page_count'] = get_page_count_for_letter(db_template) - + email_reply_to = None + sms_sender = None + if db_template['template_type'] == 'email': + email_reply_to = get_email_reply_to_address_from_session(service_id) + elif db_template['template_type'] == 'sms': + sms_sender = get_sms_sender_from_session(service_id) template = get_template( db_template, current_service, @@ -324,7 +347,8 @@ def send_test_step(service_id, template_id, step_index): filetype='png', ), page_count=session['send_test_letter_page_count'], - email_reply_to=get_email_reply_to_address_from_session(service_id), + email_reply_to=email_reply_to, + sms_sender=sms_sender ) placeholders = fields_to_fill_in( @@ -449,6 +473,12 @@ def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False): remaining_messages = (current_service['message_limit'] - sum(stat['requested'] for stat in statistics.values())) contents = s3download(service_id, upload_id) + email_reply_to = None + sms_sender = None + if template_type == 'email': + email_reply_to = get_email_reply_to_address_from_session(service_id) + elif template_type == 'sms': + sms_sender = get_sms_sender_from_session(service_id) template = get_template( service_api_client.get_service_template( service_id, @@ -463,7 +493,8 @@ def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False): upload_id=upload_id, filetype='png', ) if not letters_as_pdf else None, - email_reply_to=get_email_reply_to_address_from_session(service_id), + email_reply_to=email_reply_to, + sms_sender=sms_sender ) recipients = RecipientCSV( contents, @@ -723,12 +754,18 @@ def check_notification(service_id, template_id): def _check_notification(service_id, template_id, exception=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(service_id) + elif db_template['template_type'] == 'sms': + sms_sender = get_sms_sender_from_session(service_id) template = get_template( db_template, current_service, show_recipient=True, - email_reply_to=get_email_reply_to_address_from_session(service_id), + email_reply_to=email_reply_to, + sms_sender=sms_sender ) # go back to start of process @@ -814,3 +851,10 @@ def get_email_reply_to_address_from_session(service_id): return service_api_client.get_reply_to_email_address( service_id, session['sender_id'] )['email_address'] + + +def get_sms_sender_from_session(service_id): + if session.get('sender_id'): + return service_api_client.get_sms_sender( + service_id=service_id, sms_sender_id=session['sender_id'] + )['sms_sender'] diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 4c2cef034..9446b69ff 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -590,7 +590,7 @@ def service_sms_senders(service_id): if sender['is_default']: hints += ["default"] if sender['inbound_number_id']: - hints += ["recieves replies"] + hints += ["receives replies"] if hints: sender['hint'] = "(" + " and ".join(hints) + ")" diff --git a/app/utils.py b/app/utils.py index d7ad0ce15..f9ce51bd8 100644 --- a/app/utils.py +++ b/app/utils.py @@ -272,6 +272,7 @@ def get_template( page_count=1, redact_missing_personalisation=False, email_reply_to=None, + sms_sender=None ): if 'email' == template['template_type']: return EmailPreviewTemplate( @@ -287,7 +288,7 @@ def get_template( return SMSPreviewTemplate( template, prefix=service['name'], - sender=(service['sms_sender'] not in {'GOVUK', None}), + sender=sms_sender if sms_sender else (service['sms_sender'] not in {'GOVUK', None}), show_recipient=show_recipient, redact_missing_personalisation=redact_missing_personalisation, ) diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 82c1a0ef5..5d99b6d27 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -27,6 +27,10 @@ from tests.conftest import ( mock_get_live_service, multiple_reply_to_email_addresses, no_reply_to_email_addresses, + multiple_sms_senders, + no_sms_senders, + multiple_sms_senders_with_diff_default, + multiple_sms_senders_no_inbound ) template_types = ['email', 'sms'] @@ -43,6 +47,12 @@ test_non_spreadsheet_files = glob(path.join('tests', 'non_spreadsheet_files', '* 'Choose where to send replies', 'Select an email address that recipients can reply to' ), + ( + mock_get_service_template, + multiple_sms_senders, + 'Chose text message sender', + 'Select a text message sender that the recipients can reply to' + ) ]) def test_show_correct_title_and_description_for_sender_type( client_request, @@ -67,12 +77,47 @@ def test_show_correct_title_and_description_for_sender_type( assert normalize_spaces(page.select_one('legend').text) == expected_description +@pytest.mark.parametrize('template_mock, sender_data', [ + ( + mock_get_service_email_template, + multiple_reply_to_email_addresses, + ), + ( + mock_get_service_template, + multiple_sms_senders_with_diff_default + ), + ( + mock_get_service_template, + multiple_sms_senders_no_inbound + ) +]) def test_default_sender_is_checked_and_has_hint( client_request, service_one, fake_uuid, - mock_get_service_email_template, - multiple_reply_to_email_addresses + template_mock, + sender_data, + mocker +): + template_mock(mocker) + sender_data(mocker) + page = client_request.get( + '.set_sender', + service_id=service_one['id'], + template_id=fake_uuid + ) + + assert page.select('.multiple-choice input')[0].has_attr('checked') + assert normalize_spaces(page.select_one('.multiple-choice label .block-label-hint').text) == "(Default)" + assert not page.select('.multiple-choice input')[1].has_attr('checked') + + +def test_default_inbound_sender_is_checked_and_has_hint_with_default_and_receives_text( + client_request, + service_one, + fake_uuid, + mock_get_service_template, + multiple_sms_senders ): page = client_request.get( '.set_sender', @@ -81,18 +126,52 @@ def test_default_sender_is_checked_and_has_hint( ) assert page.select('.multiple-choice input')[0].has_attr('checked') - assert normalize_spaces(page.select_one('.multiple-choice label .block-label-hint').text) == "Default" + assert normalize_spaces( + page.select_one('.multiple-choice label .block-label-hint').text) == "(Default and receives replies)" assert not page.select('.multiple-choice input')[1].has_attr('checked') assert not page.select('.multiple-choice input')[2].has_attr('checked') +def test_sms_sender_has_receives_replies_hint( + client_request, + service_one, + fake_uuid, + mock_get_service_template, + multiple_sms_senders +): + page = client_request.get( + '.set_sender', + service_id=service_one['id'], + template_id=fake_uuid + ) + + assert page.select('.multiple-choice input')[0].has_attr('checked') + assert normalize_spaces( + page.select_one('.multiple-choice label .block-label-hint').text) == "(Default and receives replies)" + assert not page.select('.multiple-choice input')[1].has_attr('checked') + assert not page.select('.multiple-choice input')[2].has_attr('checked') + + +@pytest.mark.parametrize('template_mock, sender_data', [ + ( + mock_get_service_email_template, + multiple_reply_to_email_addresses, + ), + ( + mock_get_service_template, + multiple_sms_senders + ) +]) def test_sender_session_is_present_after_selected( logged_in_client, service_one, fake_uuid, - mock_get_service_email_template, - multiple_reply_to_email_addresses + template_mock, + sender_data, + mocker ): + template_mock(mocker) + sender_data(mocker) logged_in_client.post( url_for('.set_sender', service_id=service_one['id'], template_id=fake_uuid), data={'sender': '1234'} @@ -107,6 +186,10 @@ def test_sender_session_is_present_after_selected( mock_get_service_email_template, no_reply_to_email_addresses, ), + ( + mock_get_service_template, + no_sms_senders + ) ]) def test_set_sender_redirects_if_no_sender_data( logged_in_client, diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 3a7cc9884..998b32243 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -761,7 +761,7 @@ def test_api_ids_dont_show_on_option_pages_with_a_single_sender( ), ( 'main.service_sms_senders', multiple_sms_senders, - 'Example (default and recieves replies) Change 1234', + 'Example (default and receives replies) Change 1234', 'Example 2 Change 5678', 'Example 3 Change 9457' ), diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index 8a11e6283..cc3974357 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -335,13 +335,14 @@ def test_should_not_allow_creation_of_a_template_without_correct_permission( @pytest.mark.parametrize('fixture, expected_status_code', [ (mock_get_service_email_template, 200), - (mock_get_service_template, 302), + (mock_get_service_template, 200), (mock_get_service_letter_template, 302), ]) -def test_should_redirect_to_one_off_if_template_type_is_not_email( +def test_should_redirect_to_one_off_if_template_type_is_letter( logged_in_client, active_user_with_permissions, multiple_reply_to_email_addresses, + multiple_sms_senders, service_one, fake_uuid, mocker, diff --git a/tests/conftest.py b/tests/conftest.py index d3a2a1a7d..34c0f6cec 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -304,6 +304,66 @@ def multiple_sms_senders(mocker): return mocker.patch('app.service_api_client.get_sms_senders', side_effect=_get) +@pytest.fixture(scope='function') +def multiple_sms_senders_with_diff_default(mocker): + def _get(service_id): + return [ + { + 'id': '1234', + 'service_id': service_id, + 'sms_sender': 'Example', + 'is_default': True, + 'created_at': datetime.utcnow(), + 'inbound_number_id': None, + 'updated_at': None + }, { + 'id': '5678', + 'service_id': service_id, + 'sms_sender': 'Example 2', + 'is_default': False, + 'created_at': datetime.utcnow(), + 'inbound_number_id': None, + 'updated_at': None + }, { + 'id': '9457', + 'service_id': service_id, + 'sms_sender': 'Example 3', + 'is_default': False, + 'created_at': datetime.utcnow(), + 'inbound_number_id': '12354', + 'updated_at': None + } + ] + + return mocker.patch('app.service_api_client.get_sms_senders', side_effect=_get) + + +@pytest.fixture(scope='function') +def multiple_sms_senders_no_inbound(mocker): + def _get(service_id): + return [ + { + 'id': '1234', + 'service_id': service_id, + 'sms_sender': 'Example', + 'is_default': True, + 'created_at': datetime.utcnow(), + 'inbound_number_id': None, + 'updated_at': None + }, { + 'id': '5678', + 'service_id': service_id, + 'sms_sender': 'Example 2', + 'is_default': False, + 'created_at': datetime.utcnow(), + 'inbound_number_id': None, + 'updated_at': None + } + ] + + return mocker.patch('app.service_api_client.get_sms_senders', side_effect=_get) + + @pytest.fixture(scope='function') def no_sms_senders(mocker): def _get(service_id):