From 483221df7d446dadb4bf46431800d055c7fbfb66 Mon Sep 17 00:00:00 2001
From: Leo Hemsted <leo.hemsted@digital.cabinet-office.gov.uk>
Date: Thu, 16 Jul 2020 16:02:04 +0100
Subject: [PATCH] add broadcast message status transition map

be explicit about which transitions we allow. this is not necessarily an exhaustive list of everything we'll allow
---
 app/broadcast_message/rest.py            | 11 +++++---
 app/models.py                            | 11 ++++++++
 tests/app/broadcast_message/test_rest.py | 32 ++++++++++++++++++++++++
 3 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/app/broadcast_message/rest.py b/app/broadcast_message/rest.py
index addd41231..3d58a7aa6 100644
--- a/app/broadcast_message/rest.py
+++ b/app/broadcast_message/rest.py
@@ -41,16 +41,21 @@ def _update_broadcast_message(broadcast_message, new_status, updating_user):
     if updating_user not in broadcast_message.service.users:
         abort(
             400,
-            f'User {updating_user.id} cannot approve broadcast {broadcast_message.id} from other service'
+            f'User {updating_user.id} cannot approve broadcast_message {broadcast_message.id} from other service'
+        )
+
+    if new_status not in BroadcastStatusType.ALLOWED_STATUS_TRANSITIONS[broadcast_message.status]:
+        abort(
+            400,
+            f'Cannot move broadcast_message {broadcast_message.id} from {broadcast_message.status} to {new_status}'
         )
 
-    # TODO: Restrict status transitions
     if new_status == BroadcastStatusType.BROADCASTING:
         # TODO: Remove this platform admin shortcut when the feature goes live
         if updating_user == broadcast_message.created_by and not updating_user.platform_admin:
             abort(
                 400,
-                f'User {updating_user.id} cannot approve their own broadcast {broadcast_message.id}'
+                f'User {updating_user.id} cannot approve their own broadcast_message {broadcast_message.id}'
             )
         else:
             broadcast_message.approved_at = datetime.utcnow()
diff --git a/app/models.py b/app/models.py
index 6f5b9a2b7..ae3143b24 100644
--- a/app/models.py
+++ b/app/models.py
@@ -2176,6 +2176,17 @@ class BroadcastStatusType(db.Model):
     PRE_BROADCAST_STATUSES = [DRAFT, PENDING_APPROVAL, REJECTED]
     LIVE_STATUSES = [BROADCASTING, COMPLETED, CANCELLED]
 
+    # these are only the transitions we expect to administer via the API code.
+    ALLOWED_STATUS_TRANSITIONS = {
+        DRAFT: {PENDING_APPROVAL},
+        PENDING_APPROVAL: {REJECTED, DRAFT, BROADCASTING},
+        REJECTED: {DRAFT, PENDING_APPROVAL},
+        BROADCASTING: {COMPLETED, CANCELLED},
+        COMPLETED: {},
+        CANCELLED: {},
+        TECHNICAL_FAILURE: {},
+    }
+
     name = db.Column(db.String, primary_key=True)
 
 
diff --git a/tests/app/broadcast_message/test_rest.py b/tests/app/broadcast_message/test_rest.py
index a1c3eda49..bc3caa505 100644
--- a/tests/app/broadcast_message/test_rest.py
+++ b/tests/app/broadcast_message/test_rest.py
@@ -377,3 +377,35 @@ def test_update_broadcast_message_status_rejects_approval_from_user_not_on_that_
 
     assert mock_task.called is False
     assert f'cannot approve broadcast' in response['message']
+
+
+@pytest.mark.parametrize('current_status, new_status', [
+    (BroadcastStatusType.DRAFT, BroadcastStatusType.DRAFT),
+    (BroadcastStatusType.DRAFT, BroadcastStatusType.BROADCASTING),
+    (BroadcastStatusType.BROADCASTING, BroadcastStatusType.PENDING_APPROVAL),
+    (BroadcastStatusType.COMPLETED, BroadcastStatusType.BROADCASTING),
+    (BroadcastStatusType.CANCELLED, BroadcastStatusType.DRAFT),
+])
+def test_update_broadcast_message_status_restricts_status_transitions_to_explicit_list(
+    admin_request,
+    sample_service,
+    mocker,
+    current_status,
+    new_status
+):
+    t = create_template(sample_service, BROADCAST_TYPE)
+    bm = create_broadcast_message(t, status=current_status)
+    approver = create_user(email='approver@gov.uk')
+    sample_service.users.append(approver)
+    mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_message.apply_async')
+
+    response = admin_request.post(
+        'broadcast_message.update_broadcast_message_status',
+        _data={'status': new_status, 'created_by': str(approver.id)},
+        service_id=t.service_id,
+        broadcast_message_id=bm.id,
+        _expected_status=400
+    )
+
+    assert mock_task.called is False
+    assert f'from {current_status} to {new_status}' in response['message']