From 7e8c638865bddce0ba142541af3c386dc5a47476 Mon Sep 17 00:00:00 2001
From: Katie Smith <katie.smith@digital.cabinet-office.gov.uk>
Date: Tue, 29 Jun 2021 17:34:29 +0100
Subject: [PATCH] Change /broadcast/view-message.html page to work with both
 permissions

The buttons and links on this page now work with the original
permissions and the two new broadcast permissions. Since the new
broadcast permissions have the effect of splitting the `send_messages`
permission this means that additional sections of if/else logic were
required.
---
 .../views/broadcast/view-message.html         |  40 ++-
 tests/app/main/views/test_broadcast.py        | 250 ++++++++++++++++--
 2 files changed, 256 insertions(+), 34 deletions(-)

diff --git a/app/templates/views/broadcast/view-message.html b/app/templates/views/broadcast/view-message.html
index 1ba0edceb..e2fd2ec27 100644
--- a/app/templates/views/broadcast/view-message.html
+++ b/app/templates/views/broadcast/view-message.html
@@ -1,5 +1,4 @@
 {% from "components/back-link/macro.njk" import govukBackLink %}
-{% from "components/button/macro.njk" import govukButton %}
 {% from "components/details/macro.njk" import govukDetails %}
 {% from "components/form.html" import form_wrapper %}
 {% from "components/banner.html" import banner %}
@@ -19,9 +18,10 @@
 
 {% block service_page_title %}
   {% if broadcast_message.status == 'pending-approval' %}
-    {% if broadcast_message.created_by and broadcast_message.created_by == current_user and current_user.has_permissions('send_messages', restrict_admin_usage=True) %}
+    {% if broadcast_message.created_by and broadcast_message.created_by == current_user
+      and current_user.has_permissions('send_messages', 'create_broadcasts', 'approve_broadcasts', restrict_admin_usage=True) %}
       {{ broadcast_message.template.name }} is waiting for approval
-    {% elif current_user.has_permissions('send_messages', restrict_admin_usage=True) %}
+    {% elif current_user.has_permissions('send_messages', 'approve_broadcasts', restrict_admin_usage=True) %}
       {% if broadcast_message.created_by %}
         {{ broadcast_message.created_by.name }}
       {% else %}
@@ -42,7 +42,8 @@
   {{ govukBackLink({ "href": back_link }) }}
 
   {% if broadcast_message.status == 'pending-approval' %}
-    {% if broadcast_message.created_by and broadcast_message.created_by == current_user and current_user.has_permissions('send_messages', restrict_admin_usage=True) %}
+    {% if broadcast_message.created_by and broadcast_message.created_by == current_user
+      and current_user.has_permissions('send_messages', 'create_broadcasts', 'approve_broadcasts', restrict_admin_usage=True) %}
       <div class="banner govuk-!-margin-bottom-6">
         <h1 class="govuk-heading-m govuk-!-margin-bottom-3">
           {{ broadcast_message.template.name }} is waiting for approval
@@ -55,7 +56,7 @@
             delete_link=url_for('main.reject_broadcast_message', service_id=current_service.id, broadcast_message_id=broadcast_message.id),
             delete_link_text='Discard this alert'
           ) }}
-        {% else %}
+        {% elif current_user.has_permissions('send_messages', 'approve_broadcasts', restrict_admin_usage=True) %}
           <p class="govuk-body govuk-!-margin-bottom-3">
             When you use a live account you’ll need another member of
             your team to approve your alert.
@@ -79,9 +80,20 @@
               ) }}
             {% endcall %}
           </details>
+        {% elif current_user.has_permissions('create_broadcasts', restrict_admin_usage=True) %}
+          <p class="govuk-body">
+            You need another member of your team to approve this alert.
+          </p>
+          <p class="govuk-body">
+            This service is in training mode. No real alerts will be sent.
+          </p>
+          {{ page_footer(
+            delete_link=url_for('main.reject_broadcast_message', service_id=current_service.id, broadcast_message_id=broadcast_message.id),
+            delete_link_text='Discard this alert'
+          ) }}
         {% endif %}
       </div>
-    {% elif current_user.has_permissions('send_messages', restrict_admin_usage=True) %}
+    {% elif current_user.has_permissions('send_messages', 'approve_broadcasts', restrict_admin_usage=True) %}
       {% call form_wrapper(class="banner govuk-!-margin-bottom-6") %}
         <h1 class="govuk-heading-m govuk-!-margin-top-0 govuk-!-margin-bottom-3">
           {% if broadcast_message.created_by %}
@@ -109,6 +121,22 @@
           delete_link_text='Reject this alert'
         ) }}
       {% endcall %}
+    {% elif current_user.has_permissions('create_broadcasts', restrict_admin_usage=True) %}
+      <div class="banner govuk-!-margin-bottom-6">
+        <h1 class="govuk-heading-m govuk-!-margin-bottom-3">This alert is waiting for approval</h1>
+        <p class="govuk-body">
+          Another member of your team needs to approve this alert.
+        </p>
+        {% if not current_service.live %}
+          <p class="govuk-body">
+            This service is in training mode. No real alerts will be sent.
+          </p>
+        {% endif %}
+        {{ page_footer(
+          delete_link=url_for('main.reject_broadcast_message', service_id=current_service.id, broadcast_message_id=broadcast_message.id),
+          delete_link_text='Reject this alert'
+          ) }}
+      </div>
     {% else %}
       <div class="banner govuk-!-margin-bottom-6">
         <h1 class="govuk-heading-m govuk-!-margin-bottom-3">This alert is waiting for approval</h1>
diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py
index 609b2e21a..4ac8de360 100644
--- a/tests/app/main/views/test_broadcast.py
+++ b/tests/app/main/views/test_broadcast.py
@@ -172,20 +172,18 @@ def test_broadcast_pages_403_for_user_without_permission(
     )
 
 
-@pytest.mark.parametrize('user_is_platform_admin', [True, False])
+@pytest.mark.parametrize('user', [
+    create_active_user_view_permissions(),
+    create_platform_admin_user(),
+    create_active_user_create_broadcasts_permissions(),
+])
 def test_user_cannot_accept_broadcast_without_permission(
-    mocker,
     client_request,
     service_one,
-    active_user_view_permissions,
-    platform_admin_user_no_service_permissions,
-    user_is_platform_admin
+    user,
 ):
     service_one['permissions'] += ['broadcast']
-    if user_is_platform_admin:
-        client_request.login(platform_admin_user_no_service_permissions)
-    else:
-        client_request.login(active_user_view_permissions)
+    client_request.login(user)
 
     client_request.post(
         '.approve_broadcast_message',
@@ -1836,28 +1834,34 @@ def test_view_broadcast_message_shows_correct_highlighted_navigation(
 
 
 @freeze_time('2020-02-22T22:22:22.000000')
+@pytest.mark.parametrize('user', [
+    create_active_user_with_permissions(),
+    create_active_user_approve_broadcasts_permissions(),
+])
 def test_view_pending_broadcast(
     mocker,
     client_request,
     service_one,
-    active_user_with_permissions,
     mock_get_broadcast_template,
     fake_uuid,
+    user,
 ):
+    broadcast_creator = create_active_user_create_broadcasts_permissions(with_unique_id=True)
     mocker.patch(
         'app.broadcast_message_api_client.get_broadcast_message',
         return_value=broadcast_message_json(
             id_=fake_uuid,
             service_id=SERVICE_ONE_ID,
             template_id=fake_uuid,
-            created_by_id=fake_uuid,
+            created_by_id=broadcast_creator['id'],
             finishes_at=None,
             status='pending-approval',
         ),
     )
+    client_request.login(user)
     mocker.patch('app.user_api_client.get_user', side_effect=[
-        active_user_with_permissions,  # Current user
-        user_json(id_=uuid.uuid4()),  # User who created broadcast
+        user,  # Current user
+        broadcast_creator,  # User who created broadcast
     ])
     service_one['permissions'] += ['broadcast']
 
@@ -1870,7 +1874,7 @@ def test_view_pending_broadcast(
     assert (
         normalize_spaces(page.select_one('.banner').text)
     ) == (
-        'Test User wants to broadcast Example template '
+        'Test User Create Broadcasts Permission wants to broadcast Example template '
         'No phones will get this alert. '
         'Start broadcasting now Reject this alert'
     )
@@ -1891,29 +1895,35 @@ def test_view_pending_broadcast(
 
 
 @freeze_time('2020-02-22T22:22:22.000000')
+@pytest.mark.parametrize('user', [
+    create_active_user_with_permissions(),
+    create_active_user_approve_broadcasts_permissions(),
+])
 def test_view_pending_broadcast_without_template(
     mocker,
     client_request,
     service_one,
-    active_user_with_permissions,
     fake_uuid,
+    user,
 ):
+    broadcast_creator = create_active_user_create_broadcasts_permissions(with_unique_id=True)
     mocker.patch(
         'app.broadcast_message_api_client.get_broadcast_message',
         return_value=broadcast_message_json(
             id_=fake_uuid,
             service_id=SERVICE_ONE_ID,
             template_id=None,
-            created_by_id=fake_uuid,
+            created_by_id=broadcast_creator['id'],
             finishes_at=None,
             status='pending-approval',
             reference='No template test',
             content='Uh-oh',
         ),
     )
+    client_request.login(user)
     mocker.patch('app.user_api_client.get_user', side_effect=[
-        active_user_with_permissions,  # Current user
-        user_json(id_=uuid.uuid4()),  # User who created broadcast
+        user,  # Current user
+        broadcast_creator,  # User who created broadcast
     ])
     service_one['permissions'] += ['broadcast']
 
@@ -1926,7 +1936,7 @@ def test_view_pending_broadcast_without_template(
     assert (
         normalize_spaces(page.select_one('.banner').text)
     ) == (
-        'Test User wants to broadcast No template test '
+        'Test User Create Broadcasts Permission wants to broadcast No template test '
         'No phones will get this alert. '
         'Start broadcasting now Reject this alert'
     )
@@ -1939,12 +1949,16 @@ def test_view_pending_broadcast_without_template(
 
 
 @freeze_time('2020-02-22T22:22:22.000000')
+@pytest.mark.parametrize('user', [
+    create_active_user_with_permissions(),
+    create_active_user_approve_broadcasts_permissions(),
+])
 def test_view_pending_broadcast_from_api_call(
     mocker,
     client_request,
     service_one,
-    active_user_with_permissions,
     fake_uuid,
+    user,
 ):
     mocker.patch(
         'app.broadcast_message_api_client.get_broadcast_message',
@@ -1961,6 +1975,7 @@ def test_view_pending_broadcast_from_api_call(
     )
     service_one['permissions'] += ['broadcast']
 
+    client_request.login(user)
     page = client_request.get(
         '.view_current_broadcast',
         service_id=SERVICE_ONE_ID,
@@ -1982,6 +1997,60 @@ def test_view_pending_broadcast_from_api_call(
     )
 
 
+def test_view_pending_broadcast_created_by_user_without_approve_permission(
+    mocker,
+    client_request,
+    service_one,
+    fake_uuid,
+):
+    current_user = create_active_user_create_broadcasts_permissions()
+    mocker.patch(
+        'app.broadcast_message_api_client.get_broadcast_message',
+        return_value=broadcast_message_json(
+            id_=fake_uuid,
+            service_id=SERVICE_ONE_ID,
+            template_id=fake_uuid,
+            created_by_id=current_user['id'],
+            finishes_at=None,
+            status='pending-approval',
+        ),
+    )
+    client_request.login(current_user)
+    mocker.patch('app.user_api_client.get_user', side_effect=[
+        current_user,  # Current user
+        current_user,  # Same created the broadcast
+    ])
+    service_one['permissions'] += ['broadcast']
+
+    page = client_request.get(
+        '.view_current_broadcast',
+        service_id=SERVICE_ONE_ID,
+        broadcast_message_id=fake_uuid,
+    )
+
+    assert (
+        normalize_spaces(page.select_one('.banner').text)
+    ) == (
+        'Example template is waiting for approval '
+        'You need another member of your team to approve this alert. '
+        'This service is in training mode. No real alerts will be sent. '
+        'Discard this alert'
+    )
+    assert not page.select('.banner input[type=checkbox]')
+
+    link = page.select_one('a.govuk-link.govuk-link--destructive')
+    assert link.text == 'Discard this alert'
+    assert link['href'] == url_for(
+        '.reject_broadcast_message',
+        service_id=SERVICE_ONE_ID,
+        broadcast_message_id=fake_uuid,
+    )
+
+
+@pytest.mark.parametrize('user', [
+    create_active_user_with_permissions(),
+    create_active_user_approve_broadcasts_permissions(),
+])
 @pytest.mark.parametrize('channel, expected_label_text', (
     ('test', (
         'I understand this will alert anyone who has switched on the test channel'
@@ -2001,8 +2070,8 @@ def test_checkbox_to_confirm_non_training_broadcasts(
     mocker,
     client_request,
     service_one,
-    active_user_with_permissions,
     fake_uuid,
+    user,
     channel,
     expected_label_text,
 ):
@@ -2021,6 +2090,7 @@ def test_checkbox_to_confirm_non_training_broadcasts(
     service_one['allowed_broadcast_provider'] = 'all'
     service_one['broadcast_channel'] = channel
 
+    client_request.login(user)
     page = client_request.get(
         '.view_current_broadcast',
         service_id=SERVICE_ONE_ID,
@@ -2038,14 +2108,18 @@ def test_checkbox_to_confirm_non_training_broadcasts(
 
 
 @freeze_time('2020-02-22T22:22:22.000000')
+@pytest.mark.parametrize('user', [
+    create_active_user_with_permissions(),
+    create_active_user_approve_broadcasts_permissions(),
+])
 def test_confirm_approve_non_training_broadcasts_errors_if_not_ticked(
     mocker,
     client_request,
     service_one,
-    active_user_with_permissions,
     fake_uuid,
     mock_update_broadcast_message,
     mock_update_broadcast_message_status,
+    user,
 ):
     mocker.patch(
         'app.broadcast_message_api_client.get_broadcast_message',
@@ -2062,6 +2136,7 @@ def test_confirm_approve_non_training_broadcasts_errors_if_not_ticked(
     service_one['allowed_broadcast_provider'] = 'all'
     service_one['broadcast_channel'] = 'severe'
 
+    client_request.login(user)
     page = client_request.post(
         '.view_current_broadcast',
         service_id=SERVICE_ONE_ID,
@@ -2081,13 +2156,18 @@ def test_confirm_approve_non_training_broadcasts_errors_if_not_ticked(
 
 
 @freeze_time('2020-02-22T22:22:22.000000')
+@pytest.mark.parametrize('user', [
+    create_active_user_with_permissions(),
+    create_active_user_approve_broadcasts_permissions(),
+    create_active_user_create_broadcasts_permissions(),
+])
 def test_cant_approve_own_broadcast(
     mocker,
     client_request,
     service_one,
-    active_user_with_permissions,
     mock_get_broadcast_template,
     fake_uuid,
+    user,
 ):
     service_one['restricted'] = False
     mocker.patch(
@@ -2101,9 +2181,10 @@ def test_cant_approve_own_broadcast(
             status='pending-approval',
         ),
     )
+    client_request.login(user)
     mocker.patch('app.user_api_client.get_user', side_effect=[
-        active_user_with_permissions,  # Current user
-        active_user_with_permissions,  # User who created broadcast (the same)
+        user,  # Current user
+        user,  # User who created broadcast (the same)
     ])
     service_one['permissions'] += ['broadcast']
 
@@ -2135,13 +2216,17 @@ def test_cant_approve_own_broadcast(
 
 
 @freeze_time('2020-02-22T22:22:22.000000')
+@pytest.mark.parametrize('user', (
+    create_active_user_with_permissions(),
+    create_active_user_approve_broadcasts_permissions(),
+))
 def test_can_approve_own_broadcast_in_trial_mode(
     mocker,
     client_request,
     service_one,
-    active_user_with_permissions,
     mock_get_broadcast_template,
     fake_uuid,
+    user,
 ):
     mocker.patch(
         'app.broadcast_message_api_client.get_broadcast_message',
@@ -2154,9 +2239,10 @@ def test_can_approve_own_broadcast_in_trial_mode(
             status='pending-approval',
         ),
     )
+    client_request.login(user)
     mocker.patch('app.user_api_client.get_user', side_effect=[
-        active_user_with_permissions,  # Current user
-        active_user_with_permissions,  # User who created broadcast (the same)
+        user,  # Current user
+        user,  # User who created broadcast (the same)
     ])
     service_one['permissions'] += ['broadcast']
 
@@ -2260,6 +2346,114 @@ def test_view_only_user_cant_approve_broadcast(
     assert not page.select_one('.banner a')
 
 
+def test_view_only_user_cant_approve_broadcasts_they_created(
+    mocker,
+    client_request,
+    service_one,
+    fake_uuid,
+    active_user_broadcast_permissions,
+    active_user_view_permissions,
+):
+    mocker.patch(
+        'app.broadcast_message_api_client.get_broadcast_message',
+        return_value=broadcast_message_json(
+            id_=fake_uuid,
+            service_id=SERVICE_ONE_ID,
+            template_id=fake_uuid,
+            created_by_id=fake_uuid,
+            finishes_at='2020-02-23T23:23:23.000000',
+            status='pending-approval',
+        ),
+    )
+    client_request.login(active_user_view_permissions)
+
+    # active_user_view_permissions and active_user_broadcast_permissions have the same
+    # id. This mocks the same user being returned, but with different permissions each time.
+    mocker.patch('app.user_api_client.get_user', side_effect=[
+        active_user_view_permissions,  # Current user
+        active_user_broadcast_permissions,  # User who created broadcast
+    ])
+    service_one['permissions'] += ['broadcast']
+    service_one['restriced'] = False
+
+    page = client_request.get(
+        '.view_current_broadcast',
+        service_id=SERVICE_ONE_ID,
+        broadcast_message_id=fake_uuid,
+    )
+
+    assert (
+        normalize_spaces(page.select_one('.banner').text)
+    ) == (
+        'This alert is waiting for approval '
+        'You don’t have permission to approve alerts.'
+    )
+
+    assert not page.select_one('form')
+    assert not page.select_one('.banner a')
+
+
+@pytest.mark.parametrize('is_service_training_mode,banner_text', [
+    (
+        True,
+        ('This alert is waiting for approval '
+         'Another member of your team needs to approve this alert. '
+         'This service is in training mode. No real alerts will be sent. '
+         'Reject this alert')
+    ),
+    (
+        False,
+        ('This alert is waiting for approval '
+         'Another member of your team needs to approve this alert. '
+         'Reject this alert')
+    )
+])
+def test_user_without_approve_permission_cant_approve_broadcast_created_by_someone_else(
+    mocker,
+    client_request,
+    service_one,
+    active_user_broadcast_permissions,
+    mock_get_broadcast_template,
+    fake_uuid,
+    is_service_training_mode,
+    banner_text,
+):
+    current_user = create_active_user_create_broadcasts_permissions(with_unique_id=True)
+    mocker.patch(
+        'app.broadcast_message_api_client.get_broadcast_message',
+        return_value=broadcast_message_json(
+            id_=fake_uuid,
+            service_id=SERVICE_ONE_ID,
+            template_id=fake_uuid,
+            created_by_id=fake_uuid,
+            finishes_at='2020-02-23T23:23:23.000000',
+            status='pending-approval',
+        ),
+    )
+    client_request.login(current_user)
+    mocker.patch('app.user_api_client.get_user', side_effect=[
+        current_user,  # Current user
+        active_user_broadcast_permissions,  # User who created broadcast
+    ])
+    service_one['permissions'] += ['broadcast']
+    service_one['restricted'] = is_service_training_mode
+
+    page = client_request.get(
+        '.view_current_broadcast',
+        service_id=SERVICE_ONE_ID,
+        broadcast_message_id=fake_uuid,
+    )
+
+    assert (
+        normalize_spaces(page.select_one('.banner').text)
+    ) == banner_text
+    assert not page.select_one('form')
+    link = page.select_one('.banner a')
+    assert link['href'] == url_for('.reject_broadcast_message',
+                                   service_id=SERVICE_ONE_ID,
+                                   broadcast_message_id=fake_uuid)
+
+
 @pytest.mark.parametrize(
     'trial_mode, initial_status, post_data, expected_approval, expected_redirect',
     (