Merge pull request #2216 from alphagov/fix-back-link-template-skip

Fix ‘Back’ link from first step of one off

app/main/views/send.py

@@ -49,6 +49,7 @@ from app.utils import (
     get_errors_for_csv,
     get_help_argument,
     get_template,
+    should_skip_template_page,
     unicode_truncate,
     user_has_permissions,
 )
@@ -428,7 +429,7 @@ def send_test_step(service_id, template_id, step_index):
             help=get_help_argument(),
         ))
 
-    back_link = get_back_link(service_id, template_id, step_index)
+    back_link = get_back_link(service_id, template, step_index)
 
     template.values = get_recipient_and_placeholders_from_session(template.template_type)
     template.values[current_placeholder] = None
@@ -762,35 +763,36 @@ def get_send_test_page_title(template_type, help_argument, entering_recipient, n
     return 'Personalise this message'
 
 
-def get_back_link(service_id, template_id, step_index):
+def get_back_link(service_id, template, step_index):
     if get_help_argument():
         # if we're on the check page, redirect back to the beginning. anywhere else, don't return the back link
         if request.endpoint == 'main.check_notification':
             return url_for(
                 'main.send_test',
                 service_id=service_id,
-                template_id=template_id,
+                template_id=template.id,
                 help=get_help_argument()
             )
         else:
             return None
     elif step_index == 0:
-        if current_user.has_permissions('view_activity'):
+        if should_skip_template_page(template.template_type):
-            return url_for(
-                '.view_template',
-                service_id=service_id,
-                template_id=template_id,
-            )
-        else:
             return url_for(
                 '.choose_template',
                 service_id=service_id,
             )
+        else:
+            return url_for(
+                '.view_template',
+                service_id=service_id,
+                template_id=template.id,
+            )
+
     else:
         return url_for(
             'main.send_one_off_step',
             service_id=service_id,
-            template_id=template_id,
+            template_id=template.id,
             step_index=step_index - 1,
         )
 
@@ -818,7 +820,7 @@ def _check_notification(service_id, template_id, exception=None):
         sms_sender=sms_sender
     )
 
-    back_link = get_back_link(service_id, template_id, len(fields_to_fill_in(template)))
+    back_link = get_back_link(service_id, template, len(fields_to_fill_in(template)))
 
     if (
         not session.get('recipient') or

app/main/views/templates.py

@@ -29,6 +29,7 @@ from app.template_previews import TemplatePreview, get_page_count_for_letter
 from app.utils import (
     email_or_sms_not_enabled,
     get_template,
+    should_skip_template_page,
     user_has_permissions,
 )
 
@@ -49,11 +50,7 @@ page_headings = {
 @user_has_permissions()
 def view_template(service_id, template_id):
     template = service_api_client.get_service_template(service_id, str(template_id))['data']
-    if (
+    if should_skip_template_page(template['template_type']):
-        current_user.has_permissions('send_messages') and
-        not current_user.has_permissions('manage_templates', 'manage_api_keys') and
-        template['template_type'] != 'letter'
-    ):
         return redirect(url_for(
             '.send_one_off', service_id=service_id, template_id=template_id
         ))

app/utils.py

@@ -655,3 +655,11 @@ def guess_name_from_email_address(email_address):
     ).then(
         normalize_spaces
     )
+
+
+def should_skip_template_page(template_type):
+    return (
+        current_user.has_permissions('send_messages') and
+        not current_user.has_permissions('manage_templates', 'manage_api_keys') and
+        template_type != 'letter'
+    )

tests/app/main/views/test_send.py

@@ -1482,47 +1482,55 @@ def test_send_test_email_message_without_placeholders_redirects_to_check_page(
     assert page.select('h1')[0].text.strip() == 'Preview of ‘Two week reminder’'
 
 
-@pytest.mark.parametrize('user, expected_back_link_endpoint, extra_args', (
+@pytest.mark.parametrize('permissions, expected_back_link_endpoint, extra_args', (
-    (active_user_with_permissions, 'main.view_template', {'template_id': unchanging_fake_uuid}),
+    (
-    (active_caseworking_user, 'main.choose_template', {}),
+        {'send_messages', 'manage_templates'},
+        'main.view_template',
+        {'template_id': unchanging_fake_uuid}
+    ),
+    (
+        {'send_messages'},
+        'main.choose_template',
+        {},
+    ),
+    (
+        {'send_messages', 'view_activity'},
+        'main.choose_template',
+        {},
+    ),
 ))
 def test_send_test_sms_message_with_placeholders_shows_first_field(
-    logged_in_client,
+    client_request,
-    mocker,
+    active_user_with_permissions,
-    service_one,
     mock_login,
     mock_get_service,
     mock_get_service_template_with_placeholders,
     mock_has_no_jobs,
-    fake_uuid,
+    permissions,
-    user,
     expected_back_link_endpoint,
     extra_args,
 ):
-    mocker.patch('app.user_api_client.get_user', return_value=user(fake_uuid))
+    active_user_with_permissions._permissions[SERVICE_ONE_ID] = permissions
+    client_request.login(active_user_with_permissions)
 
-    with logged_in_client.session_transaction() as session:
+    with client_request.session_transaction() as session:
         assert 'placeholders' not in session
 
-    response = logged_in_client.get(
+    page = client_request.get(
-        url_for(
+        'main.send_test',
-            'main.send_test',
+        service_id=SERVICE_ONE_ID,
-            service_id=service_one['id'],
+        template_id=unchanging_fake_uuid,
-            template_id=unchanging_fake_uuid,
+        _follow_redirects=True,
-        ),
-        follow_redirects=True,
     )
-    assert response.status_code == 200
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
 
     assert page.select('label')[0].text.strip() == 'name'
     assert page.select('input')[0]['name'] == 'placeholder_value'
     assert page.select('.page-footer-back-link')[0]['href'] == url_for(
         expected_back_link_endpoint,
-        service_id=service_one['id'],
+        service_id=SERVICE_ONE_ID,
         **extra_args
     )
-    with logged_in_client.session_transaction() as session:
+    with client_request.session_transaction() as session:
         assert session['recipient'] == '07700 900762'