Merge pull request #1543 from gov-cjwaszczuk/master

Allow one-off email sending to select the reply-to address

app/main/forms.py

@@ -690,3 +690,13 @@ def get_placeholder_form_instance(
     return PlaceholderForm(
         placeholder_value=dict_to_populate_from.get(placeholder_name, '')
     )
+
+
+class SetSenderForm(Form):
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.sender.choices = kwargs['sender_choices']
+        self.sender.label.text = kwargs['sender_label']
+
+    sender = RadioField()

app/main/views/send.py

@@ -32,6 +32,7 @@ from app.main import main
 from app.main.forms import (
     CsvUploadForm,
     ChooseTimeForm,
+    SetSenderForm,
     get_placeholder_form_instance
 )
 from app.main.s3_client import (
@@ -92,7 +93,7 @@ def get_example_letter_address(key):
 @login_required
 @user_has_permissions('send_texts', 'send_emails', 'send_letters')
 def send_messages(service_id, template_id):
-
+    session['sender_id'] = None
     db_template = service_api_client.get_service_template(service_id, template_id)['data']
 
     if email_or_sms_not_enabled(db_template['template_type'], current_service['permissions']):
@@ -166,6 +167,77 @@ def get_example_csv(service_id, template_id):
     }
 
 
+@main.route("/services/<service_id>/send/<template_id>/set-sender", methods=['GET', 'POST'])
+@login_required
+@user_has_permissions('send_texts', 'send_emails', 'send_letters')
+def set_sender(service_id, template_id):
+    session['sender_id'] = None
+    redirect_to_one_off = redirect(
+        url_for('.send_one_off', service_id=service_id, template_id=template_id)
+    )
+
+    template = service_api_client.get_service_template(service_id, template_id)['data']
+
+    if template['template_type'] != 'email':
+        return redirect_to_one_off
+
+    sender_details = get_sender_details(service_id, template['template_type'])
+    if len(sender_details) <= 1:
+        return redirect_to_one_off
+
+    sender_context = get_sender_context(sender_details, template['template_type'])
+
+    form = SetSenderForm(
+        sender=sender_context['default_id'],
+        sender_choices=sender_context['value_and_label'],
+        sender_label=sender_context['description']
+    )
+    option_hints = {sender_context['default_id']: 'Default'}
+
+    if form.validate_on_submit():
+        session['sender_id'] = form.sender.data
+        return redirect(url_for('.send_one_off',
+                                service_id=service_id,
+                                template_id=template_id))
+
+    return render_template(
+        'views/templates/set-sender.html',
+        form=form,
+        template_id=template_id,
+        sender_context={'title': sender_context['title'], 'description': sender_context['description']},
+        option_hints=option_hints
+    )
+
+
+def get_sender_context(sender_details, template_type):
+    context = {
+        'email': {
+            'title': "Choose where to send replies",
+            'description': "Select an email address that recipients can reply to",
+            'field_name': 'email_address'
+        },
+        'letter': {
+            'title': 'Choose sender address',
+            'description': 'Select an address that recipients can reply to',
+            'field_name': 'contact_block'
+        }
+    }[template_type]
+
+    sender_format = context['field_name']
+
+    context['default_id'] = next(sender['id'] for sender in sender_details if sender['is_default'])
+    context['value_and_label'] = [(sender['id'], sender[sender_format]) for sender in sender_details]
+    return context
+
+
+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
+    }[template_type]
+    return api_call(service_id)
+
+
 @main.route("/services/<service_id>/send/<template_id>/test", endpoint='send_test')
 @main.route("/services/<service_id>/send/<template_id>/one-off", endpoint='send_one_off')
 @login_required
@@ -176,6 +248,8 @@ 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':
+        session['sender_id'] = None
 
     if email_or_sms_not_enabled(db_template['template_type'], current_service['permissions']):
         return redirect(url_for(
@@ -319,7 +393,6 @@ def send_test_step(service_id, template_id, step_index):
         )
     else:
         skip_link = None
-
     return render_template(
         'views/send-test.html',
         page_title=get_send_test_page_title(template.template_type, get_help_argument()),
@@ -707,13 +780,13 @@ def send_notification(service_id, template_id):
             service_id=service_id,
             template_id=template_id,
         ))
-
     try:
         noti = notification_api_client.send_notification(
             service_id,
             template_id=template_id,
             recipient=session['recipient'],
-            personalisation=session['placeholders']
+            personalisation=session['placeholders'],
+            sender_id=session['sender_id'] if 'sender_id' in session else None
         )
     except HTTPError as exception:
         current_app.logger.info('Service {} could not send notification: "{}"'.format(
@@ -724,6 +797,7 @@ def send_notification(service_id, template_id):
 
     session.pop('placeholders')
     session.pop('recipient')
+    session.pop('sender_id', None)
 
     return redirect(url_for(
         '.view_notification',

app/main/views/service_settings.py

@@ -581,6 +581,7 @@ def service_set_branding_and_org(service_id):
     organisations = organisations_client.get_organisations()
 
     form = ServiceBrandingOrg(branding_type=current_service.get('branding'))
+
     # dynamically create org choices, including the null option
     form.organisation.choices = [('None', 'None')] + get_branding_as_value_and_label(organisations)
 

app/main/views/templates.py

@@ -79,7 +79,7 @@ def view_template(service_id, template_id):
             show_recipient=True,
             page_count=get_page_count_for_letter(template),
         ),
-        default_letter_contact_block_id=default_letter_contact_block_id
+        default_letter_contact_block_id=default_letter_contact_block_id,
     )
 
 
@@ -487,7 +487,7 @@ def delete_service_template(service_id, template_id):
                 filetype='png',
             ),
             show_recipient=True,
-        )
+        ),
     )
 
 
@@ -495,10 +495,12 @@ def delete_service_template(service_id, template_id):
 @login_required
 @user_has_permissions('manage_templates', admin_override=True)
 def confirm_redact_template(service_id, template_id):
+    template = service_api_client.get_service_template(service_id, template_id)['data']
+
     return render_template(
         'views/templates/template.html',
         template=get_template(
-            service_api_client.get_service_template(service_id, template_id)['data'],
+            template,
             current_service,
             expand_emails=True,
             letter_preview_url=url_for(

app/notify_client/notification_api_client.py

@@ -54,12 +54,14 @@ class NotificationApiClient(NotifyAdminAPIClient):
                 params=params
             )
 
-    def send_notification(self, service_id, *, template_id, recipient, personalisation):
+    def send_notification(self, service_id, *, template_id, recipient, personalisation, sender_id):
         data = {
             'template_id': template_id,
             'to': recipient,
             'personalisation': personalisation,
         }
+        if sender_id:
+            data['sender_id'] = sender_id
         data = _attach_current_user(data)
         return self.post(url='/service/{}/send-notification'.format(service_id), data=data)
 

app/templates/views/templates/_template.html

@@ -13,7 +13,7 @@
           </a>
         </div>
         <div class="{{ 'column-half' if template.template_type == 'letter' else 'column-third' }}">
-          <a href="{{ url_for(".send_one_off", service_id=current_service.id, template_id=template.id) }}" class="pill-separate-item">
+          <a href="{{ url_for(".set_sender", service_id=current_service.id, template_id=template.id) }}" class="pill-separate-item">
             {{ 'Print a test letter' if template.template_type == 'letter' else 'Send to one recipient' }}
           </a>
         </div>

app/templates/views/templates/set-sender.html

@@ -0,0 +1,29 @@
+{% extends "withnav_template.html" %}
+{% from "components/radios.html" import radios, branding_radios %}
+{% from "components/page-footer.html" import page_footer %}
+
+{% block service_page_title %}
+  {{sender_context['title']}}
+{% endblock %}
+
+{% block maincolumn_content %}
+
+  <h1 class="heading-large">{{sender_context['title']}}</h1>
+  <div class="grid-row">
+    <div class="column-three-quarters">
+      <form method="post">
+        {{ radios(
+          form.sender, 
+          option_hints=option_hints
+          ) 
+        }}
+        {{ page_footer(
+          'Continue',
+          back_link=url_for('.view_template', service_id=current_service.id, template_id=template_id),
+          back_link_text='Back to template'
+        ) }}
+      </form>
+    </div>
+  </div>
+
+{% endblock %}

tests/app/main/views/test_send.py

@@ -26,6 +26,12 @@ from tests.conftest import (
     SERVICE_ONE_ID,
     mock_get_service,
     mock_get_live_service,
+    multiple_reply_to_email_addresses,
+    multiple_letter_contact_blocks,
+    multiple_sms_senders,
+    no_reply_to_email_addresses,
+    no_letter_contact_blocks,
+    no_sms_senders
 )
 
 template_types = ['email', 'sms']
@@ -35,6 +41,101 @@ test_spreadsheet_files = glob(path.join('tests', 'spreadsheet_files', '*'))
 test_non_spreadsheet_files = glob(path.join('tests', 'non_spreadsheet_files', '*'))
 
 
+@pytest.mark.parametrize('template_mock, sender_data, expected_title, expected_description', [
+    (
+        mock_get_service_email_template,
+        multiple_reply_to_email_addresses,
+        'Choose where to send replies',
+        'Select an email address that recipients can reply to'
+    ),
+])
+def test_show_correct_title_and_description_for_sender_type(
+    client_request,
+    service_one,
+    fake_uuid,
+    template_mock,
+    sender_data,
+    expected_title,
+    expected_description,
+    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_one('h1').text == expected_title
+    assert normalize_spaces(page.select_one('legend').text) == expected_description
+
+
+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
+):
+    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')
+    assert not page.select('.multiple-choice input')[2].has_attr('checked')
+
+
+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
+):
+    response = logged_in_client.post(
+        url_for('.set_sender', service_id=service_one['id'], template_id=fake_uuid),
+        data={'sender': '1234'}
+    )
+
+    with logged_in_client.session_transaction() as session:
+        assert session['sender_id'] == '1234'
+
+
+@pytest.mark.parametrize('template_mock, sender_data', [
+    (
+        mock_get_service_email_template,
+        no_reply_to_email_addresses,
+    ),
+])
+def test_set_sender_redirects_if_no_sender_data(
+    logged_in_client,
+    service_one,
+    fake_uuid,
+    template_mock,
+    sender_data,
+    mocker
+):
+    template_mock(mocker)
+    sender_data(mocker)
+    response = logged_in_client.get(
+        url_for('.set_sender', service_id=service_one['id'], template_id=fake_uuid)
+    )
+    assert response.status_code == 302
+    expected_url = url_for(
+        '.send_one_off',
+        service_id=service_one['id'],
+        template_id=fake_uuid,
+        _external=True,
+    )
+    assert response.location == expected_url
+
+
 def test_that_test_files_exist():
     assert len(test_spreadsheet_files) == 8
     assert len(test_non_spreadsheet_files) == 6
@@ -1965,7 +2066,8 @@ def test_send_notification_submits_data(
         service_one['id'],
         template_id=fake_uuid,
         recipient='07700900001',
-        personalisation={'a': 'b'}
+        personalisation={'a': 'b'},
+        sender_id=None
     )
 
 

tests/app/main/views/test_templates.py

@@ -116,11 +116,11 @@ def test_should_show_page_for_one_template(
     ),
     (
         ['send_texts', 'send_emails', 'send_letters'],
-        ['.send_messages', '.send_one_off']
+        ['.send_messages', '.set_sender']
     ),
     (
         ['send_texts', 'send_emails', 'send_letters', 'manage_templates'],
-        ['.send_messages', '.send_one_off', '.edit_service_template']
+        ['.send_messages', '.set_sender', '.edit_service_template']
     ),
 ])
 def test_should_be_able_to_view_a_template_with_links(
@@ -335,6 +335,30 @@ 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_letter_template, 302),
+])
+def test_should_redirect_to_one_off_if_template_type_is_not_email(
+    logged_in_client,
+    active_user_with_permissions,
+    multiple_reply_to_email_addresses,
+    service_one,
+    fake_uuid,
+    mocker,
+    fixture,
+    expected_status_code
+):
+    fixture(mocker)
+
+    page = logged_in_client.get(
+        url_for('.set_sender', service_id=service_one['id'], template_id=fake_uuid)
+    )
+
+    assert page.status_code == expected_status_code
+
+
 def test_should_redirect_when_saving_a_template(
     logged_in_client,
     active_user_with_permissions,

tests/app/notify_client/test_notification_client.py

@@ -42,7 +42,13 @@ def test_client_gets_notifications_for_service_and_job_by_page(mocker, arguments
 
 def test_send_notification(mocker, logged_in_client, active_user_with_permissions):
     mock_post = mocker.patch('app.notify_client.notification_api_client.NotificationApiClient.post')
-    NotificationApiClient().send_notification('foo', template_id='bar', recipient='07700900001', personalisation=None)
+    NotificationApiClient().send_notification(
+        'foo',
+        template_id='bar',
+        recipient='07700900001',
+        personalisation=None,
+        sender_id=None
+    )
     mock_post.assert_called_once_with(
         url='/service/foo/send-notification',
         data={

tests/conftest.py

@@ -270,6 +270,92 @@ def mock_update_letter_contact(mocker):
     return mocker.patch('app.service_api_client.update_letter_contact', side_effect=_update_letter_contact)
 
 
+@pytest.fixture(scope='function')
+def multiple_sms_senders(mocker):
+    def _get(service_id):
+        return [
+            {
+                'id': '1234',
+                'service_id': service_id,
+                'sms_sender': 'Example',
+                'is_default': True,
+                'created_at': datetime.utcnow(),
+                'updated_at': None
+            }, {
+                'id': '5678',
+                'service_id': service_id,
+                'sms_sender': 'Example 2',
+                'is_default': False,
+                'created_at': datetime.utcnow(),
+                'updated_at': None
+            }, {
+                'id': '9457',
+                'service_id': service_id,
+                'sms_sender': 'Example 3',
+                'is_default': False,
+                'created_at': datetime.utcnow(),
+                '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):
+        return []
+
+    return mocker.patch('app.service_api_client.get_sms_senders', side_effect=_get)
+
+
+@pytest.fixture(scope='function')
+def single_sms_sender(mocker):
+    def _get(service_id):
+        return [
+            {
+                'id': '1234',
+                'service_id': service_id,
+                'sms_sender': 'Example',
+                'is_default': True,
+                'created_at': datetime.utcnow(),
+                'updated_at': None
+            }
+        ]
+
+    return mocker.patch('app.service_api_client.get_sms_senders', side_effect=_get)
+
+
+@pytest.fixture(scope='function')
+def get_default_sms_sender(mocker):
+    def _get(service_id, sms_sender_id):
+        return {
+            'id': '1234',
+            'service_id': service_id,
+            'sms_sender': 'Example',
+            'is_default': True,
+            'created_at': datetime.utcnow(),
+            'updated_at': None
+        }
+
+    return mocker.patch('app.service_api_client.get_sms_sender', side_effect=_get)
+
+
+@pytest.fixture(scope='function')
+def get_non_default_sms_sender(mocker):
+    def _get(service_id, sms_sender_id):
+        return {
+            'id': '1234',
+            'service_id': service_id,
+            'service_id': service_id,
+            'is_default': False,
+            'created_at': datetime.utcnow(),
+            'updated_at': None
+        }
+
+    return mocker.patch('app.service_api_client.get_sms_sender', side_effect=_get)
+
+
 @pytest.fixture(scope='function')
 def fake_uuid():
     return sample_uuid()
@@ -2014,7 +2100,7 @@ def mock_get_notification(
 @pytest.fixture
 def mock_send_notification(mocker, fake_uuid):
     def _send_notification(
-        service_id, *, template_id, recipient, personalisation
+        service_id, *, template_id, recipient, personalisation, sender_id
     ):
         return {'id': fake_uuid}