Merge pull request #3552 from alphagov/cant-self-approve

Don’t let users self-approve broadcasts

app/models/broadcast_message.py

@@ -3,6 +3,7 @@ from datetime import datetime
 from notifications_utils.broadcast_areas import broadcast_area_libraries
 from notifications_utils.template import BroadcastPreviewTemplate
 from orderedset import OrderedSet
+from werkzeug.utils import cached_property
 
 from app.models import JSONModel, ModelList
 from app.models.user import User
@@ -94,15 +95,15 @@ class BroadcastMessage(JSONModel):
             return 'completed'
         return self._dict['status']
 
-    @property
+    @cached_property
     def created_by(self):
         return User.from_id(self.created_by_id)
 
-    @property
+    @cached_property
     def approved_by(self):
         return User.from_id(self.approved_by_id)
 
-    @property
+    @cached_property
     def cancelled_by(self):
         return User.from_id(self.cancelled_by_id)
 

app/templates/views/broadcast/view-message.html

@@ -1,5 +1,6 @@
 {% from "components/button/macro.njk" import govukButton %}
 {% from "components/form.html" import form_wrapper %}
+{% from "components/banner.html" import banner %}
 {% from "components/page-header.html" import page_header %}
 {% from "components/page-footer.html" import page_footer %}
 
@@ -17,17 +18,30 @@
   ) }}
 
   {% if broadcast_message.status == 'pending-approval' %}
-    {% call form_wrapper(class="banner govuk-!-margin-bottom-6") %}
-      <p class="govuk-body govuk-!-margin-top-0 govuk-!-margin-bottom-3">
-        {{ broadcast_message.created_by.name }} wants to broadcast this
-        message until {{ broadcast_message.finishes_at|format_datetime_relative }}.
-      </p>
-      {{ page_footer(
-        "Start broadcasting now",
-        delete_link=url_for('main.reject_broadcast_message', service_id=current_service.id, broadcast_message_id=broadcast_message.id),
-        delete_link_text='Reject this broadcast'
-      ) }}
-    {% endcall %}
+    {% if broadcast_message.created_by == current_user %}
+      <div class="banner govuk-!-margin-bottom-6">
+        <h2 class="govuk-heading-s govuk-!-margin-bottom-3">Your broadcast is waiting for approval from another member of your team</h2>
+        <p class="govuk-body">Once approved it will be live until
+          {{ broadcast_message.finishes_at|format_datetime_relative }}
+        </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='Withdraw this broadcast'
+        ) }}
+      </div>
+    {% else %}
+      {% call form_wrapper(class="banner govuk-!-margin-bottom-6") %}
+        <p class="govuk-body govuk-!-margin-top-0 govuk-!-margin-bottom-3">
+          {{ broadcast_message.created_by.name }} wants to broadcast this
+          message until {{ broadcast_message.finishes_at|format_datetime_relative }}.
+        </p>
+        {{ page_footer(
+          "Start broadcasting now",
+          delete_link=url_for('main.reject_broadcast_message', service_id=current_service.id, broadcast_message_id=broadcast_message.id),
+          delete_link_text='Reject this broadcast'
+        ) }}
+      {% endcall %}
+    {% endif %}
   {% else %}
     <p class="govuk-body govuk-!-margin-bottom-3">
       Created by {{ broadcast_message.created_by.name }} and approved by

tests/app/main/views/test_broadcast.py

@@ -1,4 +1,5 @@
 import json
+import uuid
 
 import pytest
 from flask import url_for
@@ -542,6 +543,7 @@ def test_view_pending_broadcast(
     mocker,
     client_request,
     service_one,
+    active_user_with_permissions,
     mock_get_broadcast_template,
     fake_uuid,
 ):
@@ -556,6 +558,10 @@ def test_view_pending_broadcast(
             status='pending-approval',
         ),
     )
+    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
+    ])
     service_one['permissions'] += ['broadcast']
 
     page = client_request.get(
@@ -585,6 +591,57 @@ def test_view_pending_broadcast(
     )
 
 
+@freeze_time('2020-02-22T22:22:22.000000')
+def test_cant_approve_own_broadcast(
+    mocker,
+    client_request,
+    service_one,
+    active_user_with_permissions,
+    mock_get_broadcast_template,
+    fake_uuid,
+):
+    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',
+        ),
+    )
+    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)
+    ])
+    service_one['permissions'] += ['broadcast']
+
+    page = client_request.get(
+        '.view_broadcast_message',
+        service_id=SERVICE_ONE_ID,
+        broadcast_message_id=fake_uuid,
+    )
+
+    assert (
+        normalize_spaces(page.select_one('.banner').text)
+    ) == (
+        'Your broadcast is waiting for approval from another member of your team '
+        'Once approved it will be live until tomorrow at 11:23pm '
+        'Withdraw this broadcast'
+    )
+
+    assert not page.select_one('form')
+
+    link = page.select_one('.banner a.govuk-link.govuk-link--destructive')
+    assert link.text == 'Withdraw this broadcast'
+    assert link['href'] == url_for(
+        '.reject_broadcast_message',
+        service_id=SERVICE_ONE_ID,
+        broadcast_message_id=fake_uuid,
+    )
+
+
 @pytest.mark.parametrize('initial_status, expected_approval', (
     ('draft', False,),
     ('pending-approval', True),