Send files by email on for everyone and only depending on service

having contact details set up.

Display not set up yet for send files by email row when contact_link not set up

app/main/views/service_settings.py

@@ -293,28 +293,6 @@ def service_set_permission(service_id, permission):
     )
 
 
-@main.route("/services/<uuid:service_id>/service-settings/can-upload-document", methods=['GET', 'POST'])
-@user_has_permissions('manage_service')
-def service_switch_can_upload_document(service_id):
-    if current_service.contact_link:
-        current_service.force_permission('upload_document', on=(not current_service.has_permission('upload_document')))
-        return redirect(url_for(".service_settings", service_id=service_id))
-
-    form = ServiceContactDetailsForm()
-
-    if form.validate_on_submit():
-        contact_type = form.contact_details_type.data
-
-        current_service.update(
-            contact_link=form.data[contact_type]
-        )
-
-        current_service.force_permission('upload_document', on=(not current_service.has_permission('upload_document')))
-        return redirect(url_for(".service_settings", service_id=service_id))
-
-    return render_template('views/service-settings/contact_link.html', form=form)
-
-
 @main.route("/services/<uuid:service_id>/service-settings/archive", methods=['GET', 'POST'])
 @user_has_permissions('manage_service')
 def archive_service(service_id):
@@ -360,18 +338,20 @@ def resume_service(service_id):
         return service_settings(service_id)
 
 
-@main.route("/services/<uuid:service_id>/service-settings/contact-link", methods=['GET', 'POST'])
+@main.route("/services/<uuid:service_id>/service-settings/send-files-by-email", methods=['GET', 'POST'])
 @user_has_permissions('manage_service')
-def service_set_contact_link(service_id):
+def send_files_by_email_contact_details(service_id):
     form = ServiceContactDetailsForm()
+    contact_details = None
 
     if request.method == 'GET':
         contact_details = current_service.contact_link
-        contact_type = check_contact_details_type(contact_details)
-        field_to_update = getattr(form, contact_type)
+        if contact_details:
+            contact_type = check_contact_details_type(contact_details)
+            field_to_update = getattr(form, contact_type)
 
-        form.contact_details_type.data = contact_type
-        field_to_update.data = contact_details
+            form.contact_details_type.data = contact_type
+            field_to_update.data = contact_details
 
     if form.validate_on_submit():
         contact_type = form.contact_details_type.data
@@ -381,7 +361,9 @@ def service_set_contact_link(service_id):
         )
         return redirect(url_for('.service_settings', service_id=current_service.id))
 
-    return render_template('views/service-settings/contact_link.html', form=form)
+    return render_template(
+        'views/service-settings/send-files-by-email.html', form=form, contact_details=contact_details
+    )
 
 
 @main.route("/services/<uuid:service_id>/service-settings/set-reply-to-email", methods=['GET'])

app/navigation.py

@@ -287,7 +287,7 @@ class HeaderNavigation(Navigation):
         'service_preview_letter_branding',
         'service_set_auth_type',
         'service_set_channel',
-        'service_set_contact_link',
+        'send_files_by_email_contact_details',
         'service_set_email_branding',
         'service_set_inbound_number',
         'service_set_inbound_sms',
@@ -299,7 +299,6 @@ class HeaderNavigation(Navigation):
         'service_set_sms_prefix',
         'service_settings',
         'service_sms_senders',
-        'service_switch_can_upload_document',
         'service_switch_count_as_live',
         'service_switch_live',
         'service_set_permission',
@@ -433,7 +432,7 @@ class MainNavigation(Navigation):
             'service_preview_letter_branding',
             'service_set_auth_type',
             'service_set_channel',
-            'service_set_contact_link',
+            'send_files_by_email_contact_details',
             'service_set_email_branding',
             'service_set_inbound_number',
             'service_set_inbound_sms',
@@ -593,7 +592,6 @@ class MainNavigation(Navigation):
         'service_delete_letter_contact',
         'service_delete_sms_sender',
         'service_download_agreement',
-        'service_switch_can_upload_document',
         'service_switch_count_as_live',
         'service_switch_live',
         'service_set_permission',
@@ -865,7 +863,7 @@ class CaseworkNavigation(Navigation):
         'service_preview_letter_branding',
         'service_set_auth_type',
         'service_set_channel',
-        'service_set_contact_link',
+        'send_files_by_email_contact_details',
         'service_set_email_branding',
         'service_set_inbound_number',
         'service_set_inbound_sms',
@@ -876,7 +874,6 @@ class CaseworkNavigation(Navigation):
         'service_set_sms_prefix',
         'service_settings',
         'service_sms_senders',
-        'service_switch_can_upload_document',
         'service_switch_count_as_live',
         'service_switch_live',
         'service_set_permission',
@@ -1155,7 +1152,7 @@ class OrgNavigation(Navigation):
         'service_preview_letter_branding',
         'service_set_auth_type',
         'service_set_channel',
-        'service_set_contact_link',
+        'send_files_by_email_contact_details',
         'service_set_email_branding',
         'service_set_inbound_number',
         'service_set_inbound_sms',
@@ -1166,7 +1163,6 @@ class OrgNavigation(Navigation):
         'service_set_sms_prefix',
         'service_settings',
         'service_sms_senders',
-        'service_switch_can_upload_document',
         'service_switch_count_as_live',
         'service_switch_live',
         'service_set_permission',

app/templates/views/service-settings.html

@@ -45,19 +45,6 @@
             )
           }}
         {% endcall %}
-
-        {% call settings_row(if_has_permission='upload_document') %}
-          {{ text_field('Contact details') }}
-          {{ text_field(current_service.contact_link, truncate=true) }}
-          {{ edit_field(
-              'Change',
-              url_for('.service_set_contact_link',
-              service_id=current_service.id),
-              permissions=['manage_service']
-            )
-          }}
-        {% endcall %}
-
       {% endcall %}
 
       {% call mapping_table(
@@ -112,10 +99,10 @@
 
         {% call settings_row(if_has_permission='email') %}
           {{ text_field('Send files by email') }}
-          {{ boolean_field('upload_document' in current_service.permissions) }}
+          {{ text_field(current_service.contact_link if current_service.contact_link else "Not set up", truncate=true) }}
           {{ edit_field(
-            'Change',
-            url_for('.service_switch_can_upload_document', service_id=current_service.id),
+            'Manage',
+            url_for('.send_files_by_email_contact_details', service_id=current_service.id),
             permissions=['manage_service'],
           )}}
         {% endcall %}

app/templates/views/service-settings/send-files-by-email.html

@@ -7,20 +7,25 @@
 {% from "components/form.html" import form_wrapper %}
 
 {% block service_page_title %}
-  {{ 'Change' if 'upload_document' in current_service.permissions else 'Add' }} contact details for ‘Download your document’ page
+  Send files by email
 {% endblock %}
 
 {% block maincolumn_content %}
   <div class="grid-row">
     <div class="column-five-sixths">
       {{ page_header(
-        '{} contact details for ‘Download your document’ page'.format('Change' if 'upload_document' in current_service.permissions else 'Add'),
+        'Send files by email',
         back_link=url_for('main.service_settings', service_id=current_service.id)
       ) }}
       <p>
-        When you send users a document to download, you need to include the contact details for your service
-        on the download page. This is so users can contact you if there’s a problem (for example,
-        if the link to download the document has expired).
+        This is an API-only feature.
+        </p>
+        <p>
+        To send a file by email, follow the instructions in our <a href={{ url_for('main.documentation') }}>API documentation</a>.
+      </p>
+      <h2 class="heading-medium">{% if contact_details %}Change contact details for{% else %}Add contact details to{% endif %} the file download page</h2>
+      <p>
+        You need to include contact details for your service so your users can get in touch if there’s a problem. For example, if the link to download the file you sent them has expired.
       </p>
       {% call form_wrapper() %}
 

tests/__init__.py

@@ -175,7 +175,7 @@ def service_json(
         'inbound_api': inbound_api,
         'service_callback_api': service_callback_api,
         'prefix_sms': prefix_sms,
-        'contact_link': None,
+        'contact_link': contact_link,
         'volume_email': 111111,
         'volume_sms': 222222,
         'volume_letter': 333333,

tests/app/main/views/test_find_services.py

@@ -42,7 +42,7 @@ def test_find_services_by_name_displays_multiple_services(
     )
     document = client_request.post('main.find_services_by_name', _data={"search": "Tadfield"}, _expected_status=200)
 
-    results = document.find_all('a', {'class': 'browse-list-item'})
+    results = document.find_all('li', {'class': 'browse-list-item'})
     assert len(results) == 2
     assert sorted([result.text.strip() for result in results]) == ["Tadfield Air Base", "Tadfield Police"]
 

tests/app/main/views/test_service_settings.py

@@ -61,7 +61,7 @@ def mock_get_service_settings_page_common(
         'Send emails On Change',
         'Reply-to email addresses Not set Manage',
         'Email branding GOV.UK Change',
-        'Send files by email Off Change',
+        'Send files by email contact_us@gov.uk Manage',
 
         'Label Value Action',
         'Send text messages On Change',
@@ -84,7 +84,7 @@ def mock_get_service_settings_page_common(
         'Send emails On Change',
         'Reply-to email addresses Not set Manage',
         'Email branding GOV.UK Change',
-        'Send files by email Off Change',
+        'Send files by email contact_us@gov.uk Manage',
 
         'Label Value Action',
         'Send text messages On Change',
@@ -120,10 +120,13 @@ def test_should_show_overview(
         expected_rows,
         mock_get_service_settings_page_common,
 ):
-    service_one = service_json(SERVICE_ONE_ID,
-                               users=[api_user_active['id']],
-                               permissions=['sms', 'email'],
-                               organisation_id=ORGANISATION_ID)
+    service_one = service_json(
+        SERVICE_ONE_ID,
+        users=[api_user_active['id']],
+        permissions=['sms', 'email'],
+        organisation_id=ORGANISATION_ID,
+        contact_link='contact_us@gov.uk',
+    )
     mocker.patch('app.service_api_client.get_service', return_value={'data': service_one})
 
     client.login(user, mocker, service_one)
@@ -188,6 +191,40 @@ def test_organisation_name_links_to_org_dashboard(
     assert normalize_spaces(org_row.find('a').text) == 'Test Organisation'
 
 
+@pytest.mark.parametrize('service_contact_link,expected_text', [
+    ('contact.me@gov.uk', 'Send files by email contact.me@gov.uk Manage'),
+    (None, 'Send files by email Not set up Manage'),
+])
+def test_send_files_by_email_row_on_settings_page(
+    client_request,
+    platform_admin_user,
+    no_reply_to_email_addresses,
+    no_letter_contact_blocks,
+    single_sms_sender,
+    mock_get_service_settings_page_common,
+    mocker,
+    mock_get_service_organisation,
+    service_contact_link,
+    expected_text
+):
+    service_one = service_json(
+        SERVICE_ONE_ID,
+        permissions=['sms', 'email'],
+        organisation_id=ORGANISATION_ID,
+        contact_link=service_contact_link
+    )
+
+    mocker.patch('app.service_api_client.get_service', return_value={'data': service_one})
+
+    client_request.login(platform_admin_user, service_one)
+    response = client_request.get(
+        'main.service_settings', service_id=SERVICE_ONE_ID
+    )
+
+    org_row = find_element_by_tag_and_partial_text(response, tag='tr', string='Send files by email')
+    assert normalize_spaces(org_row.get_text()) == expected_text
+
+
 @pytest.mark.parametrize('permissions, expected_rows', [
     (['email', 'sms', 'inbound_sms', 'international_sms'], [
 
@@ -198,7 +235,7 @@ def test_organisation_name_links_to_org_dashboard(
         'Send emails On Change',
         'Reply-to email addresses test@example.com Manage',
         'Email branding Organisation name Change',
-        'Send files by email Off Change',
+        'Send files by email Not set up Manage',
 
         'Label Value Action',
         'Send text messages On Change',
@@ -220,7 +257,7 @@ def test_organisation_name_links_to_org_dashboard(
         'Send emails On Change',
         'Reply-to email addresses test@example.com Manage',
         'Email branding Organisation name Change',
-        'Send files by email Off Change',
+        'Send files by email Not set up Manage',
 
         'Label Value Action',
         'Send text messages On Change',
@@ -3714,85 +3751,6 @@ def test_switch_service_enable_international_sms(
     assert mocked_fn.call_args[0][0] == service_one['id']
 
 
-@pytest.mark.parametrize('start_permissions, contact_details, end_permissions', [
-    (['upload_document'], 'http://example.com/', []),
-    ([], '0207 123 4567', ['upload_document']),
-])
-def test_service_switch_can_upload_document_changes_permission_if_service_contact_details_exist(
-    platform_admin_client,
-    service_one,
-    mock_update_service,
-    mock_get_service_settings_page_common,
-    mock_get_service_organisation,
-    no_reply_to_email_addresses,
-    no_letter_contact_blocks,
-    single_sms_sender,
-    start_permissions,
-    contact_details,
-    end_permissions,
-):
-    service_one['permissions'] = start_permissions
-    service_one['contact_link'] = contact_details
-
-    response = platform_admin_client.get(
-        url_for('main.service_switch_can_upload_document', service_id=SERVICE_ONE_ID),
-        follow_redirects=True
-    )
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-    assert normalize_spaces(page.h1.text) == 'Settings'
-    mock_update_service.assert_called_with(SERVICE_ONE_ID, permissions=end_permissions)
-
-
-def test_service_switch_can_upload_document_turning_permission_on_with_no_contact_details_shows_form(
-    platform_admin_client,
-    service_one,
-    mock_get_service_settings_page_common,
-    mock_get_service_organisation,
-    no_reply_to_email_addresses,
-    no_letter_contact_blocks,
-    single_sms_sender,
-):
-    response = platform_admin_client.get(
-        url_for('main.service_switch_can_upload_document', service_id=SERVICE_ONE_ID),
-        follow_redirects=True
-    )
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-
-    assert 'upload_document' not in service_one['permissions']
-    assert normalize_spaces(page.h1.text) == "Add contact details for ‘Download your document’ page"
-
-
-@pytest.mark.parametrize('contact_details_type, contact_details_value', [
-    ('url', 'http://example.com/'),
-    ('email_address', 'old@example.com'),
-    ('phone_number', '0207 12345'),
-])
-def test_service_switch_can_upload_document_lets_contact_details_be_added_and_changes_setting(
-    platform_admin_client,
-    service_one,
-    mock_update_service,
-    mock_get_service_settings_page_common,
-    mock_get_service_organisation,
-    no_reply_to_email_addresses,
-    no_letter_contact_blocks,
-    single_sms_sender,
-    contact_details_type,
-    contact_details_value,
-):
-    service_one['permissions'] = []
-    data = {'contact_details_type': contact_details_type, contact_details_type: contact_details_value}
-
-    response = platform_admin_client.post(
-        url_for('main.service_switch_can_upload_document', service_id=SERVICE_ONE_ID),
-        data=data,
-        follow_redirects=True
-    )
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-
-    assert normalize_spaces(page.h1.text) == 'Settings'
-    mock_update_service.assert_called_with(SERVICE_ONE_ID, permissions=['upload_document'])
-
-
 @pytest.mark.parametrize('user', (
     create_platform_admin_user(),
     create_active_user_with_permissions(),
@@ -3996,7 +3954,7 @@ def test_cant_resume_active_service(
     ('email_address', 'me@example.com'),
     ('phone_number', '0207 123 4567'),
 ])
-def test_service_set_contact_link_prefills_the_form_with_the_existing_contact_details(
+def test_send_files_by_email_contact_details_prefills_the_form_with_the_existing_contact_details(
     client_request,
     service_one,
     contact_details_type,
@@ -4005,7 +3963,7 @@ def test_service_set_contact_link_prefills_the_form_with_the_existing_contact_de
     service_one['contact_link'] = contact_details_value
 
     page = client_request.get(
-        'main.service_set_contact_link', service_id=SERVICE_ONE_ID
+        'main.send_files_by_email_contact_details', service_id=SERVICE_ONE_ID
     )
     assert page.find('input', attrs={'name': 'contact_details_type', 'value': contact_details_type}).has_attr('checked')
     assert page.find('input', {'id': contact_details_type}).get('value') == contact_details_value
@@ -4016,7 +3974,7 @@ def test_service_set_contact_link_prefills_the_form_with_the_existing_contact_de
     ('email_address', 'old@example.com', 'new@example.com'),
     ('phone_number', '0207 12345', '0207 56789'),
 ])
-def test_service_set_contact_link_updates_contact_details_and_redirects_to_settings_page(
+def test_send_files_by_email_contact_details_updates_contact_details_and_redirects_to_settings_page(
     client_request,
     service_one,
     mock_update_service,
@@ -4032,7 +3990,7 @@ def test_service_set_contact_link_updates_contact_details_and_redirects_to_setti
     service_one['contact_link'] = old_value
 
     page = client_request.post(
-        'main.service_set_contact_link', service_id=SERVICE_ONE_ID,
+        'main.send_files_by_email_contact_details', service_id=SERVICE_ONE_ID,
         _data={
             'contact_details_type': contact_details_type,
             contact_details_type: new_value,
@@ -4044,7 +4002,7 @@ def test_service_set_contact_link_updates_contact_details_and_redirects_to_setti
     mock_update_service.assert_called_once_with(SERVICE_ONE_ID, contact_link=new_value)
 
 
-def test_service_set_contact_link_updates_contact_details_for_the_selected_field_when_multiple_textboxes_contain_data(
+def test_send_files_by_email_contact_details_uses_the_selected_field_when_multiple_textboxes_contain_data(
     client_request,
     service_one,
     mock_update_service,
@@ -4057,7 +4015,7 @@ def test_service_set_contact_link_updates_contact_details_for_the_selected_field
     service_one['contact_link'] = 'http://www.old-url.com'
 
     page = client_request.post(
-        'main.service_set_contact_link', service_id=SERVICE_ONE_ID,
+        'main.send_files_by_email_contact_details', service_id=SERVICE_ONE_ID,
         _data={
             'contact_details_type': 'url',
             'url': 'http://www.new-url.com',
@@ -4071,12 +4029,33 @@ def test_service_set_contact_link_updates_contact_details_for_the_selected_field
     mock_update_service.assert_called_once_with(SERVICE_ONE_ID, contact_link='http://www.new-url.com')
 
 
-def test_service_set_contact_link_displays_error_message_when_no_radio_button_selected(
+@pytest.mark.parametrize(
+    'contact_link, subheader, button_selected',
+    [
+        ('contact.me@gov.uk', 'Change contact details for the file download page', True),
+        (None, 'Add contact details to the file download page', False),
+    ]
+)
+def test_send_files_by_email_contact_details_page(
+    client_request, service_one, active_user_with_permissions, contact_link, subheader, button_selected
+):
+    service_one["contact_link"] = contact_link
+    page = client_request.get(
+        'main.send_files_by_email_contact_details', service_id=SERVICE_ONE_ID
+    )
+    assert normalize_spaces(page.find_all('h2')[1].text) == subheader
+    if button_selected:
+        assert 'checked' in page.find('input', {'name': 'contact_details_type', 'value': 'email_address'}).attrs
+    else:
+        assert 'checked' not in page.find('input', {'name': 'contact_details_type', 'value': 'email_address'}).attrs
+
+
+def test_send_files_by_email_contact_details_displays_error_message_when_no_radio_button_selected(
     client_request,
     service_one
 ):
     page = client_request.post(
-        'main.service_set_contact_link', service_id=SERVICE_ONE_ID,
+        'main.send_files_by_email_contact_details', service_id=SERVICE_ONE_ID,
         _data={
             'contact_details_type': None,
             'url': '',
@@ -4086,7 +4065,7 @@ def test_service_set_contact_link_displays_error_message_when_no_radio_button_se
         _follow_redirects=True
     )
     assert normalize_spaces(page.find('span', class_='error-message').text) == 'Not a valid choice'
-    assert normalize_spaces(page.h1.text) == "Add contact details for ‘Download your document’ page"
+    assert normalize_spaces(page.h1.text) == "Send files by email"
 
 
 @pytest.mark.parametrize('contact_details_type, invalid_value, error', [
@@ -4094,7 +4073,7 @@ def test_service_set_contact_link_displays_error_message_when_no_radio_button_se
     ('email_address', 'me@co', 'Enter a valid email address'),
     ('phone_number', 'abcde', 'Must be a valid phone number'),
 ])
-def test_service_set_contact_link_does_not_update_invalid_contact_details(
+def test_send_files_by_email_contact_details_does_not_update_invalid_contact_details(
     mocker,
     client_request,
     service_one,
@@ -4106,7 +4085,7 @@ def test_service_set_contact_link_does_not_update_invalid_contact_details(
     service_one['permissions'].append('upload_document')
 
     page = client_request.post(
-        'main.service_set_contact_link', service_id=SERVICE_ONE_ID,
+        'main.send_files_by_email_contact_details', service_id=SERVICE_ONE_ID,
         _data={
             'contact_details_type': contact_details_type,
             contact_details_type: invalid_value,
@@ -4115,24 +4094,7 @@ def test_service_set_contact_link_does_not_update_invalid_contact_details(
     )
 
     assert normalize_spaces(page.find('span', class_='error-message').text) == error
-    assert normalize_spaces(page.h1.text) == "Change contact details for ‘Download your document’ page"
-
-
-def test_contact_link_is_displayed_with_upload_document_permission(
-    client_request,
-    service_one,
-    mock_get_service_settings_page_common,
-    mock_get_service_organisation,
-    no_reply_to_email_addresses,
-    no_letter_contact_blocks,
-    single_sms_sender,
-):
-    service_one['permissions'] = ['upload_document']
-    page = client_request.get(
-        'main.service_settings',
-        service_id=SERVICE_ONE_ID,
-    )
-    assert 'Contact details' in page.text
+    assert normalize_spaces(page.h1.text) == "Send files by email"
 
 
 def test_contact_link_is_not_displayed_without_the_upload_document_permission(