From bf981c308aa7d099237998f2c4a87ba96cf7ef21 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Thu, 7 Nov 2019 13:31:02 +0000
Subject: [PATCH] Require `return_to` param in `action_blocked`
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The `action_blocked` endpoint needed a variation of the URL without
a `template_id` parameter, because `None` is no longer a valid
`template_id` (because it’s not a UUID).

This change was made in 265931d21746918c4ddfc19c4ad3f8cb5683c1bf, which
also removed the `return_to` parameter, because the back link on the
`action blocked` page only ever goes to `add_new_template` if there’s
no `template_id` provided.

However this was conflating the two things, so I’ve wound it back a bit
so that:
- there’s still a new route, whose URL doesn’t include `template_id`
  as a parameter
- `return_to` is always required

I’ve also refactored the code a bit to move the looking up of the back
link from the Jinja into the view layer, so that the related code is in
one place and easier to reason about.
---
 app/main/views/templates.py                   | 24 ++++++++++++-------
 .../views/templates/action_blocked.html       | 18 ++++----------
 2 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/app/main/views/templates.py b/app/main/views/templates.py
index e42aba388..a3827a4b5 100644
--- a/app/main/views/templates.py
+++ b/app/main/views/templates.py
@@ -1,4 +1,5 @@
 from datetime import datetime, timedelta
+from functools import partial
 from string import ascii_uppercase
 
 from dateutil.parser import parse
@@ -406,25 +407,32 @@ def _get_template_copy_name(template, existing_templates):
 
 @main.route((
     '/services/<uuid:service_id>/templates/action-blocked/'
-    '<template_type:notification_type>'
+    '<template_type:notification_type>/<return_to>'
 ))
 @main.route((
     '/services/<uuid:service_id>/templates/action-blocked/'
     '<template_type:notification_type>/<return_to>/<uuid:template_id>'
 ))
 @user_has_permissions('manage_templates')
-def action_blocked(service_id, notification_type, return_to='add_new_template', template_id=None):
-    if notification_type == 'sms':
-        notification_type = 'text messages'
-    elif notification_type == 'email':
-        notification_type = 'emails'
+def action_blocked(service_id, notification_type, return_to, template_id=None):
+
+    back_link = {
+        'add_new_template': partial(
+            url_for, '.choose_template', service_id=current_service.id
+        ),
+        'templates': partial(
+            url_for, '.choose_template', service_id=current_service.id
+        ),
+        'view_template': partial(
+            url_for, '.view_template', service_id=current_service.id, template_id=template_id
+        ),
+    }.get(return_to)
 
     return render_template(
         'views/templates/action_blocked.html',
         service_id=service_id,
         notification_type=notification_type,
-        return_to=return_to,
-        template_id=template_id,
+        back_link=back_link(),
     )
 
 
diff --git a/app/templates/views/templates/action_blocked.html b/app/templates/views/templates/action_blocked.html
index d7575fe41..84bb59b8e 100644
--- a/app/templates/views/templates/action_blocked.html
+++ b/app/templates/views/templates/action_blocked.html
@@ -2,6 +2,7 @@
 {% from "components/textbox.html" import textbox %}
 {% from "components/page-header.html" import page_header %}
 {% from "components/page-footer.html" import page_footer %}
+{% from "components/message-count-label.html" import message_count_label %}
 
 {% block service_page_title %}
   {{ notification_type.capitalize() }} are disabled
@@ -11,26 +12,15 @@
 
   <div class="grid-row">
     <div class="column-five-sixths">
-      {% set
-        back_link_dict = {
-          'add_new_template': '.choose_template',
-          'templates': '.choose_template',
-          'view_template': '.view_template'
-        }
-      %}
       {{ page_header(
         '{} are disabled'.format(notification_type.capitalize()),
-        back_link=url_for(
-          back_link_dict[return_to],
-          service_id=current_service.id,
-          template_id=template_id
-        )
+        back_link=back_link,
       ) }}
       <p>
-        Sending {{ notification_type }} has been disabled for your service.
+        Sending {{ message_count_label(999, notification_type, suffix='') -}} has been disabled for your service.
       </p>
       <p>
-        If you need to send {{ notification_type }}
+        If you need to send {{ message_count_label(999, notification_type, suffix='') }}
         <a href="{{ url_for('.support') }}">get in touch with the GOV.UK Notify team</a>.
       </p>