Don’t ask for a user’s email address if we know it

If a user is logged in then we already know their name and email
address. So there’s no need for them to fill them again on the support
form.

One concern we might have about this is the user not realising we’re
doing this, and the feedback form looking like a bit of a black hole.
So we’re replaying their email address on this page to reassure them
that:
- we know who they are
- and that they’ll get a reply

app/main/views/feedback.py

@@ -1,5 +1,6 @@
 import requests
 from flask import render_template, url_for, redirect, flash, current_app, abort
+from flask_login import current_user
 from app.main import main
 from app.main.forms import SupportType, Feedback
 
@@ -21,15 +22,20 @@ def feedback(ticket_type):
         abort(404)
     form = Feedback()
     if form.validate_on_submit():
-        user_supplied_email = form.email_address.data != ''
+        if current_user.is_authenticated:
+            user_email = current_user.email_address
+            user_name = current_user.name
+        else:
+            user_email = form.email_address.data
+            user_name = form.name.data or None
         feedback_msg = 'Environment: {}\n{}\n{}'.format(
             url_for('main.index', _external=True),
-            '' if user_supplied_email else '{} (no email address supplied)'.format(form.name.data),
+            '' if user_email else '{} (no email address supplied)'.format(form.name.data),
             form.feedback.data
         )
         data = {
-            'person_email': form.email_address.data or current_app.config.get('DESKPRO_PERSON_EMAIL'),
+            'person_email': user_email or current_app.config.get('DESKPRO_PERSON_EMAIL'),
-            'person_name': form.name.data or None,
+            'person_name': user_name,
             'department_id': current_app.config.get('DESKPRO_DEPT_ID'),
             'agent_team_id': current_app.config.get('DESKPRO_ASSIGNED_AGENT_TEAM_ID'),
             'subject': 'Notify feedback',

app/templates/views/support/problem.html

@@ -22,10 +22,14 @@
         <p>What went wrong, if anything? What went well? How could we improve this service?</p>
         <form method="post">
             {{ textbox(form.feedback, width='1-1', hint='', rows=10) }}
-            <h3 class="heading-medium">Do you want a reply?</h3>
+            {% if not current_user.is_authenticated %}
-            <p>Leave your details below if you'd like a response.</p>
+              <h3 class="heading-medium">Do you want a reply?</h3>
-            {{ textbox(form.name, width='1-1') }}
+              <p>Leave your details below if you'd like a response.</p>
-            {{ textbox(form.email_address, width='1-1') }}
+              {{ textbox(form.name, width='1-1') }}
+              {{ textbox(form.email_address, width='1-1') }}
+            {% else %}
+              <p>We’ll reply to {{ current_user.email_address }}</p>
+            {% endif %}
             {{ page_footer(
               'Send',
               secondary_link=url_for('.support'),

app/templates/views/support/question.html

@@ -15,10 +15,14 @@
       <div class="column-two-thirds">
         <form method="post">
             {{ textbox(form.feedback, width='1-1', hint='', rows=10) }}
-            <h3 class="heading-medium">Do you want a reply?</h3>
+            {% if not current_user.is_authenticated %}
-            <p>Leave your details below if you'd like a response.</p>
+              <h3 class="heading-medium">Do you want a reply?</h3>
-            {{ textbox(form.name, width='1-1') }}
+              <p>Leave your details below if you'd like a response.</p>
-            {{ textbox(form.email_address, width='1-1') }}
+              {{ textbox(form.name, width='1-1') }}
+              {{ textbox(form.email_address, width='1-1') }}
+            {% else %}
+              <p>We’ll reply to {{ current_user.email_address }}</p>
+            {% endif %}
             {{ page_footer(
               'Send',
               secondary_link=url_for('.support'),

tests/app/main/views/test_feedback.py

@@ -1,4 +1,4 @@
-from bs4 import BeautifulSoup
+from bs4 import BeautifulSoup, element
 from functools import partial
 import pytest
 from flask import url_for
@@ -30,7 +30,24 @@ def test_get_support_index_page(client):
     ('problem', 'Report a problem'),
     ('question', 'Ask a question or give feedback'),
 ])
-def test_choose_support_type(client, support_type, expected_h1):
+@pytest.mark.parametrize('logged_in, expected_form_field, expected_contact_details', [
+    (True, type(None), 'We’ll reply to test@user.gov.uk'),
+    (True, type(None), 'We’ll reply to test@user.gov.uk'),
+    (False, element.Tag, 'Leave your details below if you\'d like a response.'),
+])
+def test_choose_support_type(
+    client,
+    api_user_active,
+    mock_get_user,
+    mock_get_services,
+    logged_in,
+    expected_form_field,
+    expected_contact_details,
+    support_type,
+    expected_h1
+):
+    if logged_in:
+        client.login(api_user_active)
     response = client.post(
         url_for('main.support'),
         data={'support_type': support_type}, follow_redirects=True
@@ -38,6 +55,9 @@ def test_choose_support_type(client, support_type, expected_h1):
     assert response.status_code == 200
     page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
     assert page.h1.string.strip() == expected_h1
+    assert isinstance(page.find('input', {'name': 'name'}), expected_form_field)
+    assert isinstance(page.find('input', {'name': 'email_address'}), expected_form_field)
+    assert page.find('form').find('p').text.strip() == expected_contact_details
 
 
 @pytest.mark.parametrize('ticket_type, expected_status_code', [
@@ -50,40 +70,56 @@ def test_get_feedback_page(client, ticket_type, expected_status_code):
     assert response.status_code == expected_status_code
 
 
-@pytest.mark.parametrize('data, expected_message, expected_person_name, expected_email', [
+@pytest.mark.parametrize('data, expected_message, expected_person_name, expected_email, logged_in', [
     (
         {'feedback': "blah", 'name': 'Fred'},
         'Environment: http://localhost/\nFred (no email address supplied)\nblah',
         'Fred',
         'donotreply@notifications.service.gov.uk',
+        False,
     ),
     (
         {'feedback': "blah"},
         'Environment: http://localhost/\n (no email address supplied)\nblah',
         None,
         'donotreply@notifications.service.gov.uk',
+        False,
     ),
     (
         {'feedback': "blah", 'name': "Steve Irwin", 'email_address': 'rip@gmail.com'},
         'Environment: http://localhost/\n\nblah',
         'Steve Irwin',
         'rip@gmail.com',
+        False,
+    ),
+    (
+        {'feedback': "blah"},
+        'Environment: http://localhost/\n\nblah',
+        'Test User',
+        'test@user.gov.uk',
+        True,
     ),
 ])
 @pytest.mark.parametrize('ticket_type', ['problem', 'question'])
-def test_post_feedback_with_name_but_no_email(
+def test_post_feedback(
     client,
+    api_user_active,
+    mock_get_user,
+    mock_get_services,
     mocker,
     ticket_type,
     data,
     expected_message,
     expected_person_name,
     expected_email,
+    logged_in,
 ):
     mock_post = mocker.patch(
         'app.main.views.feedback.requests.post',
         return_value=Mock(status_code=201)
     )
+    if logged_in:
+        client.login(api_user_active)
     resp = client.post(
         url_for('main.feedback', ticket_type=ticket_type),
         data=data,