From 8ff7fecf40d8b270b5a0ae28b8053d6ae2866336 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 9 Aug 2021 15:01:06 +0100 Subject: [PATCH 1/2] Expire test and operator alerts after 4 hours MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While testing alerts on these channels the MNOs sometimes need to restart their CBCs to make sure everything is failing over properly. If the CBC does not come back up, for whatever reason, then we are left in a state where the alert can’t be cancelled. To minimise the impact to the public in this scenario we should keep the expiry time at 4 hours for alerts sent on test channels. We recently increased it back up to 24 hours for all channels, so this in effect is reverting that change for channels that won’t be used in a real emergency. --- app/main/views/broadcast.py | 4 ++-- app/models/broadcast_message.py | 11 +++++++---- tests/app/main/views/test_broadcast.py | 15 ++++++++++++++- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/app/main/views/broadcast.py b/app/main/views/broadcast.py index e11a8fb30..052dc5399 100644 --- a/app/main/views/broadcast.py +++ b/app/main/views/broadcast.py @@ -449,14 +449,14 @@ def approve_broadcast_message(service_id, broadcast_message_id): )) if current_service.trial_mode: - broadcast_message.approve_broadcast() + broadcast_message.approve_broadcast(channel=current_service.broadcast_channel) return redirect(url_for( '.broadcast_tour', service_id=current_service.id, step_index=6, )) elif form.validate_on_submit(): - broadcast_message.approve_broadcast() + broadcast_message.approve_broadcast(channel=current_service.broadcast_channel) else: return render_template( 'views/broadcast/view-message.html', diff --git a/app/models/broadcast_message.py b/app/models/broadcast_message.py index a3ea36499..401ee444f 100644 --- a/app/models/broadcast_message.py +++ b/app/models/broadcast_message.py @@ -248,12 +248,15 @@ class BroadcastMessage(JSONModel): def request_approval(self): self._set_status_to('pending-approval') - def approve_broadcast(self): + def approve_broadcast(self, channel): + if channel in {'test', 'operator'}: + ttl = timedelta(hours=4, minutes=0) + else: + ttl = timedelta(hours=23, minutes=59) + self._update( starts_at=datetime.utcnow().isoformat(), - finishes_at=( - datetime.utcnow() + timedelta(hours=23, minutes=59) - ).isoformat(), + finishes_at=(datetime.utcnow() + ttl).isoformat(), ) self._set_status_to('broadcasting') diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index 6e480f0c9..6ddc7b137 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -2375,6 +2375,16 @@ def test_user_without_approve_permission_cant_approve_broadcast_they_created( ) +@pytest.mark.parametrize('channel, expected_finishes_at', ( + # 4 hours later + ('operator', '2020-02-23T02:22:22'), + ('test', '2020-02-23T02:22:22'), + + # 23 hours 59 minutes later + ('severe', '2020-02-23T22:21:22'), + ('government', '2020-02-23T22:21:22'), + (None, '2020-02-23T22:21:22'), # Training mode +)) @pytest.mark.parametrize( 'trial_mode, initial_status, post_data, expected_approval, expected_redirect', ( @@ -2424,6 +2434,8 @@ def test_confirm_approve_broadcast( expected_approval, trial_mode, expected_redirect, + channel, + expected_finishes_at, ): mocker.patch( 'app.broadcast_message_api_client.get_broadcast_message', @@ -2438,6 +2450,7 @@ def test_confirm_approve_broadcast( ) service_one['restricted'] = trial_mode service_one['permissions'] += ['broadcast'] + service_one['broadcast_channel'] = channel client_request.login(active_user_approve_broadcasts_permission) client_request.post( @@ -2457,7 +2470,7 @@ def test_confirm_approve_broadcast( broadcast_message_id=fake_uuid, data={ 'starts_at': '2020-02-22T22:22:22', - 'finishes_at': '2020-02-23T22:21:22', + 'finishes_at': expected_finishes_at, }, ) mock_update_broadcast_message_status.assert_called_once_with( From 196da2b1b7de60446ecbc8d527cd8338c6f8058c Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 10 Aug 2021 13:41:03 +0100 Subject: [PATCH 2/2] Reduce expiry time to 22 hours 30 minutes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Theoretically the maximum expiry time of a broadcast should be 24 hours. If it goes over 24 hours there can be problems. However we want to make it more conservative to mitigate two potential issues: 1. The CBC has a repetition period (eg 60 seconds) and a count (eg 1,440). If these were slightly innaccurate or generous it could take us over 24 hours. For this reason we should give ourselves half an hour of buffer. 2. It’s possibly that the CBC could interpret a UTC time as BST or vice versa. Until we’re sure that it’s using UTC everywhere, we need to remove another whole hour as buffer. In total this means we remove 1 hour 30 minutes from 24 hours, giving an expiry time of 22 hours 30 minutes. --- app/models/broadcast_message.py | 2 +- tests/app/main/views/test_broadcast.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/models/broadcast_message.py b/app/models/broadcast_message.py index 401ee444f..14c4c9525 100644 --- a/app/models/broadcast_message.py +++ b/app/models/broadcast_message.py @@ -252,7 +252,7 @@ class BroadcastMessage(JSONModel): if channel in {'test', 'operator'}: ttl = timedelta(hours=4, minutes=0) else: - ttl = timedelta(hours=23, minutes=59) + ttl = timedelta(hours=22, minutes=30) self._update( starts_at=datetime.utcnow().isoformat(), diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index 6ddc7b137..589bc9205 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -2380,10 +2380,10 @@ def test_user_without_approve_permission_cant_approve_broadcast_they_created( ('operator', '2020-02-23T02:22:22'), ('test', '2020-02-23T02:22:22'), - # 23 hours 59 minutes later - ('severe', '2020-02-23T22:21:22'), - ('government', '2020-02-23T22:21:22'), - (None, '2020-02-23T22:21:22'), # Training mode + # 22 hours 30 minutes later + ('severe', '2020-02-23T20:52:22'), + ('government', '2020-02-23T20:52:22'), + (None, '2020-02-23T20:52:22'), # Training mode )) @pytest.mark.parametrize( 'trial_mode, initial_status, post_data, expected_approval, expected_redirect',