Merge pull request #1425 from alphagov/split-settings

Split settings page into multiple sections

app/assets/stylesheets/components/table.scss

@@ -143,6 +143,25 @@
   @include bold-16;
 }
 
+
+.table-field-headings {
+
+  th {
+    padding: 1px; /* needs some height for the grey border to show */
+  }
+
+}
+
+.table-field-headings-visible {
+
+  height: auto;
+
+  th {
+    padding: .75em 1.25em .5625em 0;
+  }
+
+}
+
 .table-field-headings,
 .table-field-headings-visible {
 

app/templates/views/service-settings.html

@@ -11,20 +11,30 @@
 
     <h1 class="heading-large">Settings</h1>
 
-    <div class="bottom-gutter-3-2 body-copy-table">
+    <div class="bottom-gutter-3-2 dashboard-table body-copy-table">
 
       {% call mapping_table(
-        caption='Settings',
+        caption='General',
         field_headings=['Label', 'Value', 'Action'],
         field_headings_visible=False,
         caption_visible=False
       ) %}
+
         {% call row() %}
           {{ text_field('Service name') }}
           {{ text_field(current_service.name) }}
           {{ edit_field('Change', url_for('.service_name_change', service_id=current_service.id)) }}
         {% endcall %}
 
+      {% endcall %}
+
+      {% call mapping_table(
+        caption='Email',
+        field_headings=['Label', 'Value', 'Action'],
+        field_headings_visible=False,
+        caption_visible=True
+      ) %}
+
         {% call row() %}
           {{ text_field('Send emails') }}
           {{ boolean_field('email' in current_service.permissions) }}
@@ -44,6 +54,15 @@
 
         {% endif %}
 
+      {% endcall %}
+
+      {% call mapping_table(
+        caption='Text messages',
+        field_headings=['Label', 'Value', 'Action'],
+        field_headings_visible=False,
+        caption_visible=True
+      ) %}
+
         {% call row() %}
           {{ text_field('Send text messages') }}
           {{ boolean_field('sms' in current_service.permissions) }}
@@ -93,8 +112,17 @@
 
         {% endif %}
 
+      {% endcall %}
+
+      {% call mapping_table(
+        caption='Letters',
+        field_headings=['Label', 'Value', 'Action'],
+        field_headings_visible=False,
+        caption_visible=True
+      ) %}
+
         {% call row() %}
-          {{ text_field('Letters') }}
+          {{ text_field('Send letters') }}
           {{ boolean_field('letter' in current_service.permissions) }}
           {{ edit_field('Change', url_for('.service_set_letters', service_id=current_service.id)) }}
         {% endcall %}

tests/app/main/views/test_service_settings.py

@@ -16,29 +16,46 @@ from tests.conftest import normalize_spaces
 
 @pytest.mark.parametrize('user, expected_rows', [
     (active_user_with_permissions, [
+
         'Label Value Action',
         'Service name service one Change',
+
+        'Label Value Action',
         'Send emails On Change',
         'Email reply to address None Change',
+
+        'Label Value Action',
         'Send text messages On Change',
         'Text message sender GOVUK Change',
         'International text messages Off Change',
         'Receive text messages Off Change',
-        'Letters Off Change',
+
+        'Label Value Action',
+        'Send letters Off Change',
+
     ]),
     (platform_admin_user, [
+
         'Label Value Action',
         'Service name service one Change',
+
+        'Label Value Action',
         'Send emails On Change',
         'Email reply to address None Change',
+
+        'Label Value Action',
         'Send text messages On Change',
         'Text message sender GOVUK Change',
         'International text messages Off Change',
         'Receive text messages Off Change',
-        'Letters Off Change',
+
+        'Label Value Action',
+        'Send letters Off Change',
+
         'Label Value Action',
         'Email branding GOV.UK Change',
         'Letter branding HM Government Change',
+
     ]),
 ])
 def test_should_show_overview(
@@ -51,6 +68,7 @@ def test_should_show_overview(
         expected_rows,
         mock_get_inbound_number_for_service
 ):
+
     service_one['permissions'] = ['sms', 'email']
 
     client.login(user(fake_uuid), mocker, service_one)
@@ -60,7 +78,7 @@ def test_should_show_overview(
     assert response.status_code == 200
     page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
     assert page.find('h1').text == 'Settings'
-    rows = page.find_all('tr')
+    rows = page.select('tr')
     assert len(rows) == len(expected_rows)
     for index, row in enumerate(expected_rows):
         assert row == " ".join(rows[index].text.split())
@@ -69,25 +87,41 @@ def test_should_show_overview(
 
 @pytest.mark.parametrize('permissions, expected_rows', [
     (['email', 'sms', 'inbound_sms', 'international_sms'], [
+
         'Service name service one Change',
+
+        'Label Value Action',
         'Send emails On Change',
         'Email reply to address test@example.com Change',
+
+        'Label Value Action',
         'Send text messages On Change',
         'Text message sender 0781239871',
         'International text messages On Change',
         'Receive text messages On Change',
         'API endpoint for received text messages None Change',
-        'Letters Off Change',
+
+        'Label Value Action',
+        'Send letters Off Change',
+
     ]),
     (['email', 'sms'], [
+
         'Service name service one Change',
+
+        'Label Value Action',
         'Send emails On Change',
         'Email reply to address test@example.com Change',
+
+        'Label Value Action',
         'Send text messages On Change',
         'Text message sender elevenchars Change',
         'International text messages Off Change',
         'Receive text messages Off Change',
-        'Letters Off Change',
+
+        'Label Value Action',
+        'Send letters Off Change',
+
     ]),
 ])
 def test_should_show_overview_for_service_with_more_things_set(
@@ -191,7 +225,7 @@ def test_letter_contact_block_shows_none_if_not_set(
     ))
 
     page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-    div = page.find_all('tr')[5].find_all('td')[1].div
+    div = page.find_all('tr')[8].find_all('td')[1].div
     assert div.text.strip() == 'None'
     assert 'default' in div.attrs['class'][0]
 
@@ -210,7 +244,7 @@ def test_escapes_letter_contact_block(
     ))
 
     page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-    div = str(page.find_all('tr')[5].find_all('td')[1].div)
+    div = str(page.find_all('tr')[8].find_all('td')[1].div)
     assert 'foo<br/>bar' in div
     assert '<script>' not in div