From 309de074c3e442ba90a0c9c2d1d0fdb2554b9b4d Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Tue, 15 Jan 2019 12:06:56 +0000
Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20give=20postage=20choice=20on=20?=
 =?UTF-8?q?service=20and=20template?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We are moving from the postage being set on the service to being set on
the template. Once a service has been migrated to have the new
permission they should no longer be able to set the postage at a service
level, only at the template level.
---
 app/main/views/service_settings.py            |  2 +-
 app/templates/views/service-settings.html     | 22 ++++++++--------
 tests/app/main/views/test_service_settings.py | 25 +++++++++++++++++++
 3 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py
index 92f1bedbf..548b4d9de 100644
--- a/app/main/views/service_settings.py
+++ b/app/main/views/service_settings.py
@@ -571,7 +571,7 @@ def service_set_letters(service_id):
 @user_has_permissions('manage_service')
 def service_set_postage(service_id):
 
-    if not current_service.has_permission('letter'):
+    if (not current_service.has_permission('letter')) or current_service.has_permission('choose_postage'):
         abort(404)
 
     form = ServicePostageForm(postage=current_service.postage)
diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html
index 576ab201f..15e6ddb4b 100644
--- a/app/templates/views/service-settings.html
+++ b/app/templates/views/service-settings.html
@@ -234,18 +234,20 @@
           )}}
         {% endcall %}
 
-        {% call settings_row(if_has_permission='letter') %}
-          {{ text_field('Postage') }}
+        {% if current_service.has_permission('letter') and not current_service.has_permission('choose_postage') %}
+          {% call settings_row() %}
+            {{ text_field('Postage') }}
             {% set postage = {'first': 'First class only', 'second': 'Second class only'} %}
             {{ text_field(postage[current_service.postage]) }}
-          {{ edit_field(
-              'Change',
-              url_for('.service_set_postage',
-              service_id=current_service.id),
-              permissions=['manage_service']
-          )
-          }}
-        {% endcall %}
+            {{ edit_field(
+                'Change',
+                url_for('.service_set_postage',
+                service_id=current_service.id),
+                permissions=['manage_service']
+            )
+            }}
+          {% endcall %}
+        {% endif %}
 
       {% endcall %}
     </div>
diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py
index 590463623..91b7f60d9 100644
--- a/tests/app/main/views/test_service_settings.py
+++ b/tests/app/main/views/test_service_settings.py
@@ -233,6 +233,31 @@ def test_if_cant_send_letters_then_cant_see_postage(
     assert 'Postage' not in letter_table
 
 
+def test_if_can_choose_postage_on_template_cant_choose_on_service(
+    client_request,
+    service_one,
+    single_reply_to_email_address,
+    single_letter_contact_block,
+    mock_get_service_organisation,
+    single_sms_sender,
+    mock_get_service_settings_page_common,
+):
+    service_one['permissions'] = ['letter', 'choose_postage']
+    page = client_request.get('main.service_settings', service_id=SERVICE_ONE_ID)
+
+    letter_table = page.find_all('table')[3]
+    rows = letter_table.select('tbody tr')
+
+    assert len(rows) == 3
+    assert 'Postage' not in letter_table
+
+    client_request.get(
+        'main.service_set_postage',
+        service_id=SERVICE_ONE_ID,
+        _expected_status=404,
+    )
+
+
 def test_if_cant_send_letters_then_cant_see_letter_contact_block(
         client_request,
         service_one,