Merge pull request #1512 from alphagov/email-reply-tweaks

Tweak email reply to pages, add IDs

app/assets/stylesheets/components/tick-cross.scss

@@ -42,12 +42,12 @@
   &-list {
 
     @extend %grid-row;
-    margin-top: 5px;
     position: relative;
 
     &-permissions {
 
       @include grid-column(3/4);
+      margin-top: 5px;
 
       li {
         display: block;
@@ -59,7 +59,7 @@
     &-edit-link {
       text-align: right;
       position: absolute;
-      top: -1.6em;
+      top: -25px;
       right: -135px;
     }
 

app/main/forms.py

@@ -463,7 +463,7 @@ class ProviderForm(Form):
 
 class ServiceReplyToEmailForm(Form):
     email_address = email_address(label='Email reply to address')
-    is_default = BooleanField("Make this address the default")
+    is_default = BooleanField("Make this email address the default")
 
 
 class ServiceSmsSender(Form):

app/templates/components/api-key.html

@@ -1,7 +1,9 @@
-{% macro api_key(key, name, thing="API key") %}
+{% macro api_key(key, name=None, thing="API key") %}
-  <h2 class="api-key-name">
+  {% if name %}
-    {{ name }}
+    <h2 class="api-key-name">
-  </h2>
+      {{ name }}
+    </h2>
+  {% endif %}
   <div data-module="api-key" data-key="{{ key }}" data-thing="{{ thing }}" aria-live="assertive">
     <span class="api-key-key">{{ key }}</span>
   </div>

app/templates/views/service-settings.html

@@ -44,8 +44,9 @@
         {% if 'email' in current_service.permissions %}
 
           {% call row() %}
-            {{ text_field('Email reply to address') }}
+            {{ text_field('Email reply to addresses') }}
-            {% call field() %}
+            {% call field(status='default' if default_reply_to_email_address == "None" else '') %}
+
               {{ default_reply_to_email_address }}
               {% if reply_to_email_address_count > 1 %}
                 <div class="hint">
@@ -53,7 +54,10 @@
                 </div>
               {% endif %}
             {% endcall %}
-            {{ edit_field('Change', url_for('.service_email_reply_to', service_id=current_service.id)) }}
+            {{ edit_field(
+              'Manage' if reply_to_email_address_count else 'Change',
+              url_for('.service_email_reply_to', service_id=current_service.id))
+            }}
           {% endcall %}
 
         {% endif %}

app/templates/views/service-settings/email-reply-to/add.html

@@ -27,7 +27,7 @@
       'Add',
       back_link=url_for('.service_email_reply_to', service_id=current_service.id),
       back_link_text='Back'
-  ) }}
+    ) }}
   </form>
 
-{% endblock %}
+{% endblock %}

app/templates/views/service-settings/email-reply-to/edit.html

@@ -19,7 +19,9 @@
       safe_error_message=True
     ) }}
     {% if form.is_default.data %}
-      <p> This email address is the default </p>
+      <p class="form-group">
+        This is the default reply to address for {{ current_service.name }} emails
+      </p>
     {% else %}
       <div class="form-group">
         {{ checkbox(form.is_default) }}
@@ -32,4 +34,4 @@
   ) }}
   </form>
 
-{% endblock %}
+{% endblock %}

app/templates/views/service-settings/email_reply_to.html

@@ -1,5 +1,5 @@
 {% extends "withnav_template.html" %}
-{% from "components/textbox.html" import textbox %}
+{% from "components/api-key.html" import api_key %}
 {% from "components/page-footer.html" import page_footer %}
 {% from "components/table.html" import row_group, row, text_field, edit_field, field, boolean_field, list_table %}
 
@@ -19,27 +19,50 @@
 	   <a href="{{ url_for('.service_add_email_reply_to', service_id=current_service.id) }}" class="button align-with-heading">Add email address</a>
 	 </div>
   </div>
-  <div class="bottom-gutter-3-2 body-copy-table">
+  <div class="user-list">
-    {% call(item, row_number) list_table(
+    {% if not reply_to_email_addresses %}
-      reply_to_email_addresses,
+      <div class="user-list-item">
-      empty_message="You haven’t added any email addresses yet.",
+        <span class="hint">You haven’t added any email reply to addresses yet</span>
-      caption="Reply to email addresses",
+      </div>
-      caption_visible=false,
+    {% endif %}
-      field_headings=[
+    {% for item in reply_to_email_addresses %}
-        'Email addresses',
+      <div class="user-list-item">
-        'Action'
+        <h3>
-      ],
+          <span class="heading-small">{{ item.email_address }}</span>&ensp;<span class="hint">
-      field_headings_visible=False
+            {%- if item.is_default -%}
-    ) %}
+              (default)
-        {% call field() %}
+            {% endif %}
-          {{ item.email_address }}
+          </span>
-          {% if item.is_default %}
+        </h3>
-            <span class="hint">
+        <ul class="tick-cross-list">
-              {{ "(default)" }}
+          <li class="tick-cross-list-edit-link">
-            </span>
+            <a href="{{ url_for('.service_edit_email_reply_to', service_id =current_service.id, reply_to_email_id = item.id) }}">Change</a>
-          {% endif %}
+          </li>
-        {% endcall %}
+        </ul>
-		{{ edit_field('Change', url_for('.service_edit_email_reply_to', service_id =current_service.id, reply_to_email_id = item.id)) }}        
+        {% if reply_to_email_addresses|length  > 1 %}
-    {% endcall %}
+          {{ api_key(item.id, thing="ID") }}
+        {% endif %}
+      </div>
+    {% endfor %}
   </div>
-{% endblock %}
+  <div class="grid-row">
+    <div class="column-five-sixths">
+      <p>
+        Your emails will be sent from
+        {{ current_service.email_from }}@notifications.service.gov.uk
+      </p>
+      <p>
+        This is so they have the best chance of being delivered.
+        This email address can’t receive replies.
+      </p>
+      <p>
+        Set up separate email addresses to receive replies
+        from your users.
+        {% if current_service.restricted and not reply_to_email_addresses %}
+          Your service can’t go live until you’ve added at least one
+          reply to address.
+        {% endif %}
+      </p>
+    </div>
+  </div>
+{% endblock %}

tests/app/main/views/test_service_settings.py

@@ -31,7 +31,7 @@ from tests.conftest import (
 
         'Label Value Action',
         'Send emails On Change',
-        'Email reply to address None Change',
+        'Email reply to addresses None Change',
 
         'Label Value Action',
         'Send text messages On Change',
@@ -50,7 +50,7 @@ from tests.conftest import (
 
         'Label Value Action',
         'Send emails On Change',
-        'Email reply to address None Change',
+        'Email reply to addresses None Change',
 
         'Label Value Action',
         'Send text messages On Change',
@@ -102,7 +102,7 @@ def test_should_show_overview(
 
         'Label Value Action',
         'Send emails On Change',
-        'Email reply to address test@example.com Change',
+        'Email reply to addresses test@example.com Manage',
 
         'Label Value Action',
         'Send text messages On Change',
@@ -121,7 +121,7 @@ def test_should_show_overview(
 
         'Label Value Action',
         'Send emails On Change',
-        'Email reply to address test@example.com Change',
+        'Email reply to addresses test@example.com Manage',
 
         'Label Value Action',
         'Send text messages On Change',
@@ -692,35 +692,54 @@ def test_reply_to_hint_appears_when_service_has_multiple_reply_to_addresses(
 
     assert normalize_spaces(
         page.select('tbody tr')[2].text
-    ) == "Email reply to address test@example.com …and 2 more Change"
+    ) == "Email reply to addresses test@example.com …and 2 more Manage"
+
+
+def test_single_reply_to_address_shows_default_but_without_id(
+    client_request,
+    single_reply_to_email_addresses
+):
+    rows = client_request.get(
+        'main.service_email_reply_to',
+        service_id=SERVICE_ONE_ID
+    ).select(
+        '.user-list-item'
+    )
+
+    assert normalize_spaces(rows[0].text) == "test@example.com (default) Change"
+    assert len(rows) == 1
 
 
 def test_default_email_reply_to_address_has_default_hint(
     client_request,
     multiple_reply_to_email_addresses
 ):
-    page = client_request.get(
+    rows = client_request.get(
         'main.service_email_reply_to',
         service_id=SERVICE_ONE_ID
+    ).select(
+        '.user-list-item'
     )
 
-    assert normalize_spaces(page.select('tbody tr')[0].text) == "test@example.com (default) Change"
+    assert normalize_spaces(rows[0].text) == "test@example.com (default) Change 1234"
-    assert normalize_spaces(page.select('tbody tr')[1].text) == "test2@example.com Change"
+    assert normalize_spaces(rows[1].text) == "test2@example.com Change 5678"
-    assert normalize_spaces(page.select('tbody tr')[2].text) == "test3@example.com Change"
+    assert normalize_spaces(rows[2].text) == "test3@example.com Change 9457"
-    assert len(page.select('tbody tr')) == 3
+    assert len(rows) == 3
 
 
 def test_no_reply_to_email_addresses_message_shows(
     client_request,
     no_reply_to_email_addresses
 ):
-    page = client_request.get(
+    rows = client_request.get(
         'main.service_email_reply_to',
         service_id=SERVICE_ONE_ID
+    ).select(
+        '.user-list-item'
     )
 
-    assert normalize_spaces(page.select('tbody tr')[0].text) == "You haven’t added any email addresses yet."
+    assert normalize_spaces(rows[0].text) == "You haven’t added any email reply to addresses yet"
-    assert len(page.select('tbody tr')) == 1
+    assert len(rows) == 1
 
 
 @pytest.mark.parametrize('reply_to_input, expected_error', [
@@ -844,7 +863,9 @@ def test_default_box_shows_on_non_default_email_addresses_while_editing(
     if checkbox_present:
         assert page.select_one('[name=is_default]')
     else:
-        assert normalize_spaces(page.select_one('form p').text) == "This email address is the default"
+        assert normalize_spaces(page.select_one('form p').text) == (
+            'This is the default reply to address for service one emails'
+        )
 
 
 def test_switch_service_to_research_mode(