From 54fe8ee68da00f93bdc19214590dda2cd59bbbbd Mon Sep 17 00:00:00 2001
From: David McDonald <david.mcdonald@digital.cabinet-office.gov.uk>
Date: Tue, 15 Jun 2021 17:18:54 +0100
Subject: [PATCH 1/2] Remove old todo for support of draft to broadcasting
 transition

It looks like we were allowing broadcasts to transition from draft to
broadcasting in one go. This isn't valid now. It should go draft,
pending approval and then broadcasting.

It looks like this was a leftover bit of support in our code for when we
were building stuff out and is no longer needed.
---
 app/models.py                            | 5 +----
 tests/app/broadcast_message/test_rest.py | 2 +-
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/app/models.py b/app/models.py
index 7d3e1b7b8..8e59c01ca 100644
--- a/app/models.py
+++ b/app/models.py
@@ -2218,10 +2218,7 @@ class BroadcastStatusType(db.Model):
 
     # these are only the transitions we expect to administer via the API code.
     ALLOWED_STATUS_TRANSITIONS = {
-        DRAFT: {
-            PENDING_APPROVAL,
-            BROADCASTING,  # TODO: Remove me once we have pending approval flow put in properly
-        },
+        DRAFT: {PENDING_APPROVAL},
         PENDING_APPROVAL: {REJECTED, DRAFT, BROADCASTING},
         REJECTED: {DRAFT, PENDING_APPROVAL},
         BROADCASTING: {COMPLETED, CANCELLED},
diff --git a/tests/app/broadcast_message/test_rest.py b/tests/app/broadcast_message/test_rest.py
index 8e8802a2a..77185ba48 100644
--- a/tests/app/broadcast_message/test_rest.py
+++ b/tests/app/broadcast_message/test_rest.py
@@ -807,7 +807,7 @@ def test_update_broadcast_message_status_rejects_approval_from_user_not_on_that_
     (BroadcastStatusType.BROADCASTING, BroadcastStatusType.PENDING_APPROVAL),
     (BroadcastStatusType.COMPLETED, BroadcastStatusType.BROADCASTING),
     (BroadcastStatusType.CANCELLED, BroadcastStatusType.DRAFT),
-    pytest.param(BroadcastStatusType.DRAFT, BroadcastStatusType.BROADCASTING, marks=pytest.mark.xfail()),
+    (BroadcastStatusType.DRAFT, BroadcastStatusType.BROADCASTING),
 ])
 def test_update_broadcast_message_status_restricts_status_transitions_to_explicit_list(
     admin_request,

From 5b409bd3c3ecb7204c2cba64abcb4e31c0a446e1 Mon Sep 17 00:00:00 2001
From: David McDonald <david.mcdonald@digital.cabinet-office.gov.uk>
Date: Tue, 15 Jun 2021 17:27:21 +0100
Subject: [PATCH 2/2] Add test coverage for broadcast status transition

This was mentioned in an old pen test report that you could send a
request twice to set a broadcast message as broadcasting which would
trigger us to send two alerts.

It looks like this is now fixed and this test coverage backs that up.

Note, it's unlikely that it would have been an issue anyway as the CBC
would likely have rejected the message as it would notice it is a
duplicate.

Note, this test coverage is not supposed to be exhaustive of all the
potential transitions but covers the vast majority of ones that we care
about.

See `BroadcastStatusType.ALLOWED_STATUS_TRANSITIONS` for allowed
transitions.
---
 tests/app/broadcast_message/test_rest.py | 27 +++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/tests/app/broadcast_message/test_rest.py b/tests/app/broadcast_message/test_rest.py
index 77185ba48..492d808a0 100644
--- a/tests/app/broadcast_message/test_rest.py
+++ b/tests/app/broadcast_message/test_rest.py
@@ -804,10 +804,31 @@ def test_update_broadcast_message_status_rejects_approval_from_user_not_on_that_
 
 @pytest.mark.parametrize('current_status, new_status', [
     (BroadcastStatusType.DRAFT, BroadcastStatusType.DRAFT),
-    (BroadcastStatusType.BROADCASTING, BroadcastStatusType.PENDING_APPROVAL),
-    (BroadcastStatusType.COMPLETED, BroadcastStatusType.BROADCASTING),
-    (BroadcastStatusType.CANCELLED, BroadcastStatusType.DRAFT),
     (BroadcastStatusType.DRAFT, BroadcastStatusType.BROADCASTING),
+    (BroadcastStatusType.DRAFT, BroadcastStatusType.CANCELLED),
+
+    (BroadcastStatusType.PENDING_APPROVAL, BroadcastStatusType.PENDING_APPROVAL),
+    (BroadcastStatusType.PENDING_APPROVAL, BroadcastStatusType.CANCELLED),
+    (BroadcastStatusType.PENDING_APPROVAL, BroadcastStatusType.COMPLETED),
+
+    (BroadcastStatusType.REJECTED, BroadcastStatusType.REJECTED),
+    (BroadcastStatusType.REJECTED, BroadcastStatusType.BROADCASTING),
+    (BroadcastStatusType.REJECTED, BroadcastStatusType.CANCELLED),
+    (BroadcastStatusType.REJECTED, BroadcastStatusType.COMPLETED),
+
+    (BroadcastStatusType.BROADCASTING, BroadcastStatusType.DRAFT),
+    (BroadcastStatusType.BROADCASTING, BroadcastStatusType.PENDING_APPROVAL),
+    (BroadcastStatusType.BROADCASTING, BroadcastStatusType.BROADCASTING),
+
+    (BroadcastStatusType.COMPLETED, BroadcastStatusType.DRAFT),
+    (BroadcastStatusType.COMPLETED, BroadcastStatusType.PENDING_APPROVAL),
+    (BroadcastStatusType.COMPLETED, BroadcastStatusType.BROADCASTING),
+    (BroadcastStatusType.COMPLETED, BroadcastStatusType.CANCELLED),
+
+    (BroadcastStatusType.CANCELLED, BroadcastStatusType.DRAFT),
+    (BroadcastStatusType.CANCELLED, BroadcastStatusType.PENDING_APPROVAL),
+    (BroadcastStatusType.CANCELLED, BroadcastStatusType.BROADCASTING),
+    (BroadcastStatusType.CANCELLED, BroadcastStatusType.COMPLETED),
 ])
 def test_update_broadcast_message_status_restricts_status_transitions_to_explicit_list(
     admin_request,