handle if user doesn't provide name/email

also clean up usage of DESKPRO_PERSON_EMAIL and put it in the conf rather than env

README.md

@@ -61,7 +61,7 @@ in a separate terminal from the app
 
 ```
 echo "
-export NOTIFY_ADMIN_ENVIRONMENT='config.Development'
+export NOTIFY_ENVIRONMENT='development'
 export ADMIN_CLIENT_SECRET='dev-notify-secret-key'
 export ADMIN_CLIENT_USER_NAME='dev-notify-admin'
 export API_HOST_NAME='http://localhost:6011'
@@ -69,7 +69,6 @@ export DANGEROUS_SALT='dev-notify-salt'
 export SECRET_KEY='dev-notify-secret-key'
 export DESKPRO_API_HOST=""
 export DESKPRO_API_KEY=""
-export DESKPRO_PERSON_EMAIL=""
 export DESKPRO_DEPT_ID=""
 export DESKPRO_ASSIGNED_AGENT_TEAM_ID=""
 "> environment.sh

app/main/views/feedback.py

@@ -8,18 +8,19 @@ from app.main.forms import Feedback
 def feedback():
     form = Feedback()
     if form.validate_on_submit():
+        user_supplied_email = form.email_address.data != ''
+        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),
+            form.feedback.data
+        )
         data = {
-            'person_email': form.email_address.data,
-            'person_name': form.name.data,
+            'person_email': form.email_address.data or current_app.config.get('DESKPRO_PERSON_EMAIL'),
+            'person_name': form.name.data or None,
             'department_id': current_app.config.get('DESKPRO_DEPT_ID'),
             'agent_team_id': current_app.config.get('DESKPRO_ASSIGNED_AGENT_TEAM_ID'),
             'subject': 'Notify feedback',
-            'message': 'Environment: {}\n\n{}\n{}\n{}'.format(
-                url_for('main.index', _external=True),
-                form.name.data,
-                form.email_address.data,
-                form.feedback.data
-            )
+            'message': feedback_msg
         }
         headers = {
             "X-DeskPRO-API-Key": current_app.config.get('DESKPRO_API_KEY'),

app/main/views/service_settings.py

@@ -108,9 +108,7 @@ def service_request_to_go_live(service_id):
             'department_id': current_app.config.get('DESKPRO_DEPT_ID'),
             'agent_team_id': current_app.config.get('DESKPRO_ASSIGNED_AGENT_TEAM_ID'),
             'subject': 'Request to go live',
-            'message': "From {} <{}> on behalf of {} ({})\n\nUsage estimate\n---\n\n{}".format(
-                current_user.name,
-                current_user.email_address,
+            'message': "On behalf of {} ({})\n\nUsage estimate\n---\n\n{}".format(
                 current_service['name'],
                 url_for('main.service_dashboard', service_id=current_service['id'], _external=True),
                 form.usage.data

config.py

@@ -35,7 +35,7 @@ class Config(object):
     CSV_UPLOAD_BUCKET_NAME = 'local-notifications-csv-upload'
     DESKPRO_API_HOST = os.environ['DESKPRO_API_HOST']
     DESKPRO_API_KEY = os.environ['DESKPRO_API_KEY']
-    DESKPRO_PERSON_EMAIL = os.environ['DESKPRO_PERSON_EMAIL']
+    DESKPRO_PERSON_EMAIL = 'donotreply@notifications.service.gov.uk'
     DESKPRO_DEPT_ID = os.environ['DESKPRO_DEPT_ID']
     DESKPRO_ASSIGNED_AGENT_TEAM_ID = os.environ['DESKPRO_ASSIGNED_AGENT_TEAM_ID']
     ACTIVITY_STATS_LIMIT_DAYS = 7

environment_test.sh

@@ -6,6 +6,5 @@ export DANGEROUS_SALT='dev-notify-salt'
 export SECRET_KEY='dev-notify-secret-key'
 export DESKPRO_API_HOST=""
 export DESKPRO_API_KEY=""
-export DESKPRO_PERSON_EMAIL=""
 export DESKPRO_DEPT_ID=""
 export DESKPRO_ASSIGNED_AGENT_TEAM_ID=""

scripts/run_tests.sh

@@ -30,8 +30,6 @@ display_result $? 1 "Code style check"
 npm test
 display_result $? 2 "Front end code style check"
 
-export NOTIFY_ADMIN_ENVIRONMENT='config.Test'
-
 ## Code coverage
 py.test -n2 --cov=app --cov-report=term-missing tests/
 display_result $? 3 "Code coverage"

tests/app/main/views/test_feedback.py

@@ -26,17 +26,27 @@ def test_get_feedback_page(app_):
             assert resp.status_code == 200
 
 
-def test_post_feedback_with_no_name_email(app_, mocker):
+def test_post_feedback_with_name_but_no_email(app_, mocker):
     mock_post = mocker.patch(
         'app.main.views.feedback.requests.post',
         return_value=Mock(status_code=201))
     with app_.test_request_context():
         with app_.test_client() as client:
-            resp = client.post(url_for('main.feedback'), data={'feedback': "blah"})
+            resp = client.post(url_for('main.feedback'), data={'feedback': "blah", 'name': 'Fred'})
             assert resp.status_code == 302
+            mock_post.assert_called_with(
+                ANY,
+                data={
+                    'department_id': ANY,
+                    'agent_team_id': ANY,
+                    'subject': 'Notify feedback',
+                    'message': 'Environment: http://localhost/\nFred (no email address supplied)\nblah',
+                    'person_email': app_.config['DESKPRO_PERSON_EMAIL'],
+                    'person_name': 'Fred'},
+                headers=ANY)
 
 
-def test_post_feedback_with_no_name_email(app_, mocker):
+def test_post_feedback_with_no_name_or_email(app_, mocker):
     mock_post = mocker.patch(
         'app.main.views.feedback.requests.post',
         return_value=Mock(status_code=201))
@@ -50,8 +60,9 @@ def test_post_feedback_with_no_name_email(app_, mocker):
                     'department_id': ANY,
                     'agent_team_id': ANY,
                     'subject': 'Notify feedback',
-                    'message': 'Environment: http://localhost/\n\n\n\nblah',
-                    'person_email': ANY},
+                    'message': 'Environment: http://localhost/\n (no email address supplied)\nblah',
+                    'person_email': app_.config['DESKPRO_PERSON_EMAIL'],
+                    'person_name': None},
                 headers=ANY)
 
 
@@ -71,8 +82,9 @@ def test_post_feedback_with_name_email(app_, mocker):
                     'subject': 'Notify feedback',
                     'department_id': ANY,
                     'agent_team_id': ANY,
-                    'message': 'Environment: http://localhost/\n\nSteve Irwin\nrip@gmail.com\nblah',
-                    'person_email': ANY},
+                    'message': 'Environment: http://localhost/\n\nblah',
+                    'person_name': 'Steve Irwin',
+                    'person_email': 'rip@gmail.com'},
                 headers=ANY)
 
 
@@ -91,14 +103,6 @@ def test_log_error_on_post(app_, mocker):
                 resp = client.post(
                     url_for('main.feedback'),
                     data={'feedback': "blah", 'name': "Steve Irwin", 'email_address': 'rip@gmail.com'})
-            mock_post.assert_called_with(
-                ANY,
-                data={
-                    'subject': 'Notify feedback',
-                    'department_id': ANY,
-                    'agent_team_id': ANY,
-                    'message': 'Environment: http://localhost/\n\nSteve Irwin\nrip@gmail.com\nblah',
-                    'person_email': ANY},
-                headers=ANY)
+            assert mock_post.called
             mock_logger.assert_called_with(
                 "Deskpro create ticket request failed with {} '{}'".format(mock_post().status_code, mock_post().json()))

tests/app/main/views/test_service_settings.py

@@ -241,10 +241,9 @@ def test_should_redirect_after_request_to_go_live(
                     'subject': 'Request to go live',
                     'department_id': ANY,
                     'agent_team_id': ANY,
-                    'message': 'From Test User <test@user.gov.uk> on behalf of Test Service (http://localhost/services/6ce466d0-fd6a-11e5-82f5-e0accb9d11a6/dashboard)\n\nUsage estimate\n---\n\nOne million messages',  # noqa
-                # noqa
-                    # noqa
-                    'person_email': ANY
+                    'message': 'On behalf of Test Service (http://localhost/services/6ce466d0-fd6a-11e5-82f5-e0accb9d11a6/dashboard)\n\nUsage estimate\n---\n\nOne million messages',  # noqa
+                    'person_name': api_user_active.name,
+                    'person_email': api_user_active.email_address
                 },
                 headers=ANY
             )