From 83156bd16e4be4a9450b156540dc756cb540cb86 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 16 Jul 2020 10:49:21 +0100 Subject: [PATCH] Let users choose when to end a broadcast Different emergencies will need broadcasts to last for a variable amount of time. We give users some control over this by letting them stop a broadcast early. But we should also let them set a maximum broadcast time, for: - when the duration of the danger is known - when the broadcast has been live long enough to alert everyone who needs to know about it This code re-uses the pattern for scheduling jobs, which has some constraints that are probably OK for now: - end time is limited to an hour - longest duration is 3 whole days (eg if you start broadcasting Friday you have the choice of Saturday, Sunday and all of Monday, up to midnight) --- app/assets/javascripts/radioSelect.js | 52 +++++--- app/main/forms.py | 20 ++- app/main/views/broadcast.py | 16 ++- app/models/broadcast_message.py | 7 +- app/templates/components/radios.html | 5 +- .../views/broadcast/preview-message.html | 9 +- tests/app/main/test_choose_time_form.py | 14 +-- tests/app/main/views/test_broadcast.py | 117 +++++++++++++++--- tests/javascripts/radioSelect.test.js | 2 +- 9 files changed, 187 insertions(+), 55 deletions(-) diff --git a/app/assets/javascripts/radioSelect.js b/app/assets/javascripts/radioSelect.js index 9426c0ab4..6edfd5392 100644 --- a/app/assets/javascripts/radioSelect.js +++ b/app/assets/javascripts/radioSelect.js @@ -7,12 +7,14 @@ let states = { 'initial': Hogan.compile(` -
-
- - + {{#showNowAsDefault}} +
+
+ + +
-
+ {{/showNowAsDefault}}
{{#categories}} @@ -20,12 +22,14 @@
`), 'choose': Hogan.compile(` -
-
- - + {{#showNowAsDefault}} +
+
+ + +
-
+ {{/showNowAsDefault}}
{{#choices}}
@@ -37,12 +41,14 @@
`), 'chosen': Hogan.compile(` -
-
- - + {{#showNowAsDefault}} +
+
+ + +
-
+ {{/showNowAsDefault}}
{{#choices}}
@@ -80,10 +86,15 @@ let categories = $component.data('categories').split(','); let name = $component.find('input').eq(0).attr('name'); let mousedownOption = null; + let showNowAsDefault = ( + $component.data('show-now-as-default').toLowerCase() === 'true' ? + {'name': name} : false + ); const reset = () => { render('initial', { 'categories': categories, - 'name': name + 'name': name, + 'showNowAsDefault': showNowAsDefault }); }; const selectOption = (value) => { @@ -91,7 +102,8 @@ 'choices': choices.filter( element => element.value == value ), - 'name': name + 'name': name, + 'showNowAsDefault': showNowAsDefault }); focusSelected(component); }; @@ -153,7 +165,8 @@ 'choices': choices.filter( element => element.value == $selection.eq(0).attr('value') ), - 'name': name + 'name': name, + 'showNowAsDefault': showNowAsDefault }); } else { @@ -174,7 +187,8 @@ render('initial', { 'categories': categories, - 'name': name + 'name': name, + 'showNowAsDefault': showNowAsDefault }); $component.css({'height': 'auto'}); diff --git a/app/main/forms.py b/app/main/forms.py index 957e7c11a..985f50bc6 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -100,7 +100,7 @@ def get_next_hours_until(until): now = datetime.utcnow() hours = int((until - now).total_seconds() / (60 * 60)) return [ - (now + timedelta(hours=i)).replace(minute=0, second=0).replace(tzinfo=pytz.utc) + (now + timedelta(hours=i)).replace(minute=0, second=0, microsecond=0).replace(tzinfo=pytz.utc) for i in range(1, hours + 1) ] @@ -979,6 +979,24 @@ class ChooseTimeForm(StripWhitespaceForm): ) +class ChooseBroadcastDurationForm(StripWhitespaceForm): + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.finishes_at.choices = [ + get_time_value_and_label(hour) for hour in get_next_hours_until( + get_furthest_possible_scheduled_time() + ) + ] + self.finishes_at.categories = get_next_days_until( + get_furthest_possible_scheduled_time() + ) + + finishes_at = RadioField( + 'When should this broadcast end?', + ) + + class CreateKeyForm(StripWhitespaceForm): def __init__(self, existing_keys, *args, **kwargs): self.existing_key_names = [ diff --git a/app/main/views/broadcast.py b/app/main/views/broadcast.py index 7dda048e8..be2f07ecc 100644 --- a/app/main/views/broadcast.py +++ b/app/main/views/broadcast.py @@ -1,8 +1,12 @@ -from flask import abort, jsonify, redirect, render_template, request, url_for +from flask import abort, jsonify, redirect, render_template, url_for from app import current_service from app.main import main -from app.main.forms import BroadcastAreaForm, SearchByNameForm +from app.main.forms import ( + BroadcastAreaForm, + ChooseBroadcastDurationForm, + SearchByNameForm, +) from app.models.broadcast_message import BroadcastMessage, BroadcastMessages from app.utils import service_has_permission, user_has_permissions @@ -141,15 +145,19 @@ def preview_broadcast_message(service_id, broadcast_message_id): broadcast_message_id, service_id=current_service.id, ) - if request.method == 'POST': - broadcast_message.request_approval() + form = ChooseBroadcastDurationForm() + + if form.validate_on_submit(): + broadcast_message.request_approval(until=form.finishes_at.data) return redirect(url_for( '.broadcast_dashboard', service_id=current_service.id, )) + return render_template( 'views/broadcast/preview-message.html', broadcast_message=broadcast_message, + form=form, ) diff --git a/app/models/broadcast_message.py b/app/models/broadcast_message.py index 8e7deb224..6703401c1 100644 --- a/app/models/broadcast_message.py +++ b/app/models/broadcast_message.py @@ -1,4 +1,4 @@ -from datetime import datetime, timedelta +from datetime import datetime from notifications_utils.broadcast_areas import broadcast_area_libraries from notifications_utils.template import BroadcastPreviewTemplate @@ -33,7 +33,6 @@ class BroadcastMessage(JSONModel): 'approved_by_id', 'cancelled_by_id', } - DEFAULT_TTL = timedelta(hours=72) libraries = broadcast_area_libraries @@ -132,9 +131,9 @@ class BroadcastMessage(JSONModel): data=kwargs, ) - def request_approval(self): + def request_approval(self, until): self._update( - finishes_at=(datetime.utcnow() + self.DEFAULT_TTL).isoformat(), + finishes_at=until, ) self._set_status_to('pending-approval') diff --git a/app/templates/components/radios.html b/app/templates/components/radios.html index e41e57af4..eb55217b0 100644 --- a/app/templates/components/radios.html +++ b/app/templates/components/radios.html @@ -23,7 +23,8 @@ {% macro radio_select( field, hint=None, - wrapping_class='form-group' + wrapping_class='form-group', + show_now_as_default=True ) %}
@@ -35,7 +36,7 @@ {% endif %} -
+
{% for option in field %}
diff --git a/app/templates/views/broadcast/preview-message.html b/app/templates/views/broadcast/preview-message.html index bed3cae07..8fa7a771c 100644 --- a/app/templates/views/broadcast/preview-message.html +++ b/app/templates/views/broadcast/preview-message.html @@ -1,7 +1,8 @@ {% from "components/button/macro.njk" import govukButton %} {% from "components/form.html" import form_wrapper %} {% from "components/page-header.html" import page_header %} -{% from "components/page-footer.html" import sticky_page_footer %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/radios.html" import radio_select %} {% extends "withnav_template.html" %} @@ -28,7 +29,11 @@ {{ broadcast_message.template|string }} {% call form_wrapper() %} - {{ sticky_page_footer('Submit for approval') }} + {{ radio_select( + form.finishes_at, + show_now_as_default=False + ) }} + {{ page_footer('Submit for approval') }} {% endcall %} {% endblock %} diff --git a/tests/app/main/test_choose_time_form.py b/tests/app/main/test_choose_time_form.py index a0f2618c4..41b32f90a 100644 --- a/tests/app/main/test_choose_time_form.py +++ b/tests/app/main/test_choose_time_form.py @@ -11,19 +11,19 @@ def test_form_contains_next_24h(app_): # Friday assert choices[0] == ('', 'Now') - assert choices[1] == ('2016-01-01T12:00:00.061258', 'Today at midday') - assert choices[13] == ('2016-01-02T00:00:00.061258', 'Today at midnight') + assert choices[1] == ('2016-01-01T12:00:00', 'Today at midday') + assert choices[13] == ('2016-01-02T00:00:00', 'Today at midnight') # Saturday - assert choices[14] == ('2016-01-02T01:00:00.061258', 'Tomorrow at 1am') - assert choices[37] == ('2016-01-03T00:00:00.061258', 'Tomorrow at midnight') + assert choices[14] == ('2016-01-02T01:00:00', 'Tomorrow at 1am') + assert choices[37] == ('2016-01-03T00:00:00', 'Tomorrow at midnight') # Sunday - assert choices[38] == ('2016-01-03T01:00:00.061258', 'Sunday at 1am') + assert choices[38] == ('2016-01-03T01:00:00', 'Sunday at 1am') # Monday - assert choices[84] == ('2016-01-04T23:00:00.061258', 'Monday at 11pm') - assert choices[85] == ('2016-01-05T00:00:00.061258', 'Monday at midnight') + assert choices[84] == ('2016-01-04T23:00:00', 'Monday at 11pm') + assert choices[85] == ('2016-01-05T00:00:00', 'Monday at midnight') with pytest.raises(IndexError): assert choices[ diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index edad43dd8..e9b2a761d 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -278,22 +278,79 @@ def test_remove_broadcast_area_page( ) +@pytest.mark.parametrize('end_time', ( + + # Before now + pytest.param('2020-02-02T02:00:00', marks=pytest.mark.xfail), + + # End of the current hour + pytest.param('2020-02-02T03:00:00'), + + # Midnight 3 days ahead + pytest.param('2020-02-06T00:00:00'), + + # 1am 4 days ahead + pytest.param('2020-02-06T01:00:00', marks=pytest.mark.xfail), + +)) +@freeze_time('2020-02-02 02:02:02') def test_preview_broadcast_message_page( client_request, service_one, mock_get_draft_broadcast_message, mock_get_broadcast_template, fake_uuid, + end_time, ): service_one['permissions'] += ['broadcast'] - client_request.get( + + page = client_request.get( '.preview_broadcast_message', service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, - ), + ) + + assert [ + normalize_spaces(area.text) + for area in page.select('.area-list-item.area-list-item--unremoveable') + ] == [ + 'England', + 'Scotland', + ] + + assert normalize_spaces( + page.select_one('.broadcast-message-wrapper').text + ) == ( + 'This is a test' + ) + + form = page.select_one('form') + assert form['method'] == 'post' + assert 'action' not in form + + radio_choices = [ + choice['value'] for choice in form.select('input[type=radio][name=finishes_at]') + ] + assert len(radio_choices) == 94 + assert end_time in radio_choices -@freeze_time('2020-02-02 02:02:02.222222') +@pytest.mark.parametrize('end_time', ( + + # Before now + pytest.param('2020-02-02T02:00:00', marks=pytest.mark.xfail), + + # End of the current hour + pytest.param('2020-02-02T03:00:00'), + + # Midnight 3 days ahead + pytest.param('2020-02-06T00:00:00'), + + # 1am 4 days ahead + pytest.param('2020-02-06T01:00:00', marks=pytest.mark.xfail), + +)) +@freeze_time('2020-02-02 02:02:02') def test_start_broadcasting( client_request, service_one, @@ -302,24 +359,54 @@ def test_start_broadcasting( mock_update_broadcast_message, mock_update_broadcast_message_status, fake_uuid, + end_time, +): + service_one['permissions'] += ['broadcast'] + client_request.post( + '.preview_broadcast_message', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + _data={ + 'finishes_at': end_time, + } + ), + mock_update_broadcast_message.assert_called_once_with( + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + data={ + 'finishes_at': end_time, + }, + ) + mock_update_broadcast_message_status.assert_called_once_with( + 'pending-approval', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + ) + + +def test_start_broadcasting_shows_validation_error( + client_request, + service_one, + mock_get_draft_broadcast_message, + mock_get_broadcast_template, + mock_update_broadcast_message, + mock_update_broadcast_message_status, + fake_uuid, ): service_one['permissions'] += ['broadcast'] - client_request.post( + page = client_request.post( '.preview_broadcast_message', service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, - ), - mock_update_broadcast_message.assert_called_once_with( - service_id=SERVICE_ONE_ID, - broadcast_message_id=fake_uuid, - data={ - 'finishes_at': '2020-02-05T02:02:02.222222', - }, + _data={}, + _expected_status=200, ) - mock_update_broadcast_message_status.assert_called_once_with( - 'pending-approval', - service_id=SERVICE_ONE_ID, - broadcast_message_id=fake_uuid, + assert mock_update_broadcast_message.called is False + assert mock_update_broadcast_message_status.called is False + assert normalize_spaces( + page.select_one('form fieldset legend .error-message').text + ) == ( + 'Select an option' ) diff --git a/tests/javascripts/radioSelect.test.js b/tests/javascripts/radioSelect.test.js index 31b8dba85..c41481208 100644 --- a/tests/javascripts/radioSelect.test.js +++ b/tests/javascripts/radioSelect.test.js @@ -99,7 +99,7 @@ describe('RadioSelect', () => { When should Notify send these messages? -
+