Let people send one off letters

We didn’t used to allow this because it wasn’t really possible with the
old DVLA set up and we didn’t think there’s a need.

We think it’s possible now because, even though it’s cumbersome, it’s
better than the manual process.

app/main/views/send.py

@@ -317,16 +317,13 @@ def send_test(service_id, template_id):
 
 
 def get_notification_check_endpoint(service_id, template):
-    if template.template_type == 'letter':
-        return make_and_upload_csv_file(service_id, template)
-    else:
-        return redirect(url_for(
-            'main.check_notification',
-            service_id=service_id,
-            template_id=template.id,
-            # at check phase we should move to help stage 2 ("the template pulls in the data you provide")
-            help='2' if 'help' in request.args else None
-        ))
+    return redirect(url_for(
+        'main.check_notification',
+        service_id=service_id,
+        template_id=template.id,
+        # at check phase we should move to help stage 2 ("the template pulls in the data you provide")
+        help='2' if 'help' in request.args else None
+    ))
 
 
 @main.route(
@@ -669,6 +666,26 @@ def check_messages_preview(service_id, template_id, upload_id, filetype, row_ind
     return TemplatePreview.from_utils_template(template, filetype, page=page)
 
 
+@main.route(
+    "/services/<service_id>/<uuid:template_id>/check.<filetype>",
+    methods=['GET'],
+)
+@login_required
+@user_has_permissions('send_messages')
+def check_notification_preview(service_id, template_id, filetype):
+    if filetype == 'pdf':
+        page = None
+    elif filetype == 'png':
+        page = request.args.get('page', 1)
+    else:
+        abort(404)
+
+    template = _check_notification(
+        service_id, template_id,
+    )['template']
+    return TemplatePreview.from_utils_template(template, filetype, page=page)
+
+
 @main.route("/services/<service_id>/start-job/<upload_id>", methods=['POST'])
 @login_required
 @user_has_permissions('send_messages', restrict_admin_usage=True)
@@ -762,8 +779,6 @@ def all_placeholders_in_session(placeholders):
 def get_send_test_page_title(template_type, help_argument, entering_recipient, name=None):
     if help_argument:
         return 'Example text message'
-    if template_type == 'letter':
-        return 'Print a test letter'
     if entering_recipient:
         return 'Send ‘{}’'.format(name)
     return 'Personalise this message'
@@ -807,7 +822,10 @@ def get_back_link(service_id, template, step_index):
 @login_required
 @user_has_permissions('send_messages', restrict_admin_usage=True)
 def check_notification(service_id, template_id):
-    return _check_notification(service_id, template_id)
+    return render_template(
+        'views/notifications/check.html',
+        **_check_notification(service_id, template_id),
+    )
 
 
 def _check_notification(service_id, template_id, exception=None):
@@ -823,25 +841,33 @@ def _check_notification(service_id, template_id, exception=None):
         current_service,
         show_recipient=True,
         email_reply_to=email_reply_to,
-        sms_sender=sms_sender
+        sms_sender=sms_sender,
+        letter_preview_url=url_for(
+            '.check_notification_preview',
+            service_id=service_id,
+            template_id=template_id,
+            filetype='png',
+        ),
+        page_count=get_page_count_for_letter(db_template),
     )
 
     back_link = get_back_link(service_id, template, len(fields_to_fill_in(template)))
 
     if (
-        not session.get('recipient') or
-        not all_placeholders_in_session(template.placeholders)
+        (
+            not session.get('recipient')
+            and db_template['template_type'] != 'letter'
+        )
+        or not all_placeholders_in_session(template.placeholders)
     ):
-        return redirect(back_link)
+        raise RequestRedirect(back_link)
 
     template.values = get_recipient_and_placeholders_from_session(template.template_type)
-    return render_template(
-        'views/notifications/check.html',
+    return dict(
         template=template,
         back_link=back_link,
         help=get_help_argument(),
-
-        **(get_template_error_dict(exception) if exception else {})
+        **(get_template_error_dict(exception) if exception else {}),
     )
 
 
@@ -880,7 +906,7 @@ def send_notification(service_id, template_id):
         noti = notification_api_client.send_notification(
             service_id,
             template_id=template_id,
-            recipient=session['recipient'],
+            recipient=session['recipient'] or session['placeholders']['address line 1'],
             personalisation=session['placeholders'],
             sender_id=session['sender_id'] if 'sender_id' in session else None
         )
@@ -889,7 +915,10 @@ def send_notification(service_id, template_id):
             current_service.id,
             exception.message
         ))
-        return _check_notification(service_id, template_id, exception)
+        return render_template(
+            'views/notifications/check.html',
+            **_check_notification(service_id, template_id, exception),
+        )
 
     session.pop('placeholders')
     session.pop('recipient')

app/navigation.py

@@ -125,6 +125,7 @@ class HeaderNavigation(Navigation):
         'check_messages',
         'check_messages_preview',
         'check_notification',
+        'check_notification_preview',
         'choose_account',
         'choose_service',
         'choose_template',
@@ -395,6 +396,7 @@ class MainNavigation(Navigation):
         'check_and_resend_text_code',
         'check_and_resend_verification_code',
         'check_messages_preview',
+        'check_notification_preview',
         'choose_account',
         'choose_service',
         'confirm_edit_organisation_name',
@@ -567,6 +569,7 @@ class CaseworkNavigation(Navigation):
         'check_messages',
         'check_messages_preview',
         'check_notification',
+        'check_notification_preview',
         'choose_account',
         'choose_service',
         'choose_template_to_copy',
@@ -801,6 +804,7 @@ class OrgNavigation(Navigation):
         'check_messages',
         'check_messages_preview',
         'check_notification',
+        'check_notification_preview',
         'choose_account',
         'choose_service',
         'choose_template',

app/templates/views/templates/_template.html

@@ -16,14 +16,9 @@
       <div class="grid-row">
         {% if template.template_type == 'letter' %}
           {% if current_user.has_permissions('send_messages', restrict_admin_usage=True) %}
-            <div class="column-half">
-              <a href="{{ url_for(".send_messages", service_id=current_service.id, template_id=template.id) }}" class="pill-separate-item">
-                Upload a list of addresses
-              </a>
-            </div>
             <div class="column-half">
               <a href="{{ url_for(".set_sender", service_id=current_service.id, template_id=template.id) }}" class="pill-separate-item">
-                Print a test letter
+                Send
               </a>
             </div>
           {% endif %}

tests/app/main/views/test_send.py

@@ -1074,13 +1074,13 @@ def test_send_one_off_does_not_send_without_the_correct_permissions(
     (
         mock_get_service_letter_template,
         partial(url_for, 'main.send_test'),
-        'Print a test letter',
+        'Send ‘Two week reminder’',
         False,
     ),
     (
         mock_get_service_letter_template,
         partial(url_for, 'main.send_one_off'),
-        'Print a test letter',
+        'Send ‘Two week reminder’',
         False,
     ),
 ])
@@ -1595,10 +1595,9 @@ def test_send_test_letter_redirects_to_right_url(
 
     assert response.status_code == 302
     assert response.location.startswith(url_for(
-        'main.check_messages',
+        'main.check_notification',
         service_id=SERVICE_ONE_ID,
         template_id=fake_uuid,
-        upload_id=fake_uuid,
         _external=True,
     ))
 
@@ -2875,27 +2874,50 @@ def test_check_notification_shows_help(
     )
 
 
+@pytest.mark.parametrize('template, recipient, placeholders, expected_personalisation', (
+    (
+        mock_get_service_template,
+        '07700900001',
+        {'a': 'b'},
+        {'a': 'b'},
+    ),
+    (
+        mock_get_service_email_template,
+        'test@example.com',
+        {},
+        {},
+    ),
+    (
+        mock_get_service_letter_template,
+        'foo',
+        {},
+        {},
+    ),
+))
 def test_send_notification_submits_data(
     client_request,
-    service_one,
     fake_uuid,
     mock_send_notification,
+    template,
+    recipient,
+    placeholders,
+    expected_personalisation,
 ):
     with client_request.session_transaction() as session:
-        session['recipient'] = '07700900001'
-        session['placeholders'] = {'a': 'b'}
+        session['recipient'] = recipient
+        session['placeholders'] = placeholders
 
     client_request.post(
         'main.send_notification',
-        service_id=service_one['id'],
+        service_id=SERVICE_ONE_ID,
         template_id=fake_uuid
     )
 
     mock_send_notification.assert_called_once_with(
-        service_one['id'],
+        SERVICE_ONE_ID,
         template_id=fake_uuid,
-        recipient='07700900001',
-        personalisation={'a': 'b'},
+        recipient=recipient,
+        personalisation=expected_personalisation,
         sender_id=None
     )