From 225a61ddd31721296146d56421770ac0fb4c3b7d Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Sun, 7 Aug 2016 09:17:49 +0100 Subject: [PATCH] Add a component for picking the time to send a job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Users need to pick a time in the next 24hrs, or send a file immediately. Rationale for this is a bit lost in time-before-holiday, but generally: ‘Now’ and ‘later’ as the inital choices makes it really clear what this feature is about conceptually. The choice of times is absolute, eg ‘1pm’ not ‘in 3 hours’ --- app/assets/javascripts/radioSelect.js | 92 +++++++++++++++++++ .../stylesheets/components/radio-select.scss | 47 ++++++++++ .../stylesheets/components/sms-message.scss | 2 +- app/assets/stylesheets/main.scss | 1 + app/main/forms.py | 43 +++++++++ app/main/views/index.py | 2 +- app/main/views/send.py | 8 +- app/notify_client/job_api_client.py | 4 +- app/templates/components/radio-select.html | 0 app/templates/components/radios.html | 34 +++++++ app/templates/views/check.html | 5 + app/templates/views/trial-mode.html | 1 - gulpfile.babel.js | 3 +- tests/app/main/test_choose_time_form.py | 22 +++++ tests/app/main/views/test_send.py | 19 +++- tests/app/notify_client/test_job_client.py | 15 +++ tests/conftest.py | 2 +- 17 files changed, 288 insertions(+), 12 deletions(-) create mode 100644 app/assets/javascripts/radioSelect.js create mode 100644 app/assets/stylesheets/components/radio-select.scss create mode 100644 app/templates/components/radio-select.html create mode 100644 tests/app/main/test_choose_time_form.py diff --git a/app/assets/javascripts/radioSelect.js b/app/assets/javascripts/radioSelect.js new file mode 100644 index 000000000..8923376ef --- /dev/null +++ b/app/assets/javascripts/radioSelect.js @@ -0,0 +1,92 @@ +(function(Modules) { + + "use strict"; + + var render = ($options, $button) => ( + filterOptionVisibility($options) && setButtonState($options, $button) + ); + + var filterOptionVisibility = $options => $options + .removeClass('js-visible') + .filter( + (index, element) => (index === 0 || $(element).has(':checked').length) + ) + .addClass('js-visible'); + + var setButtonState = ($options, $button) => $button + .addClass('js-visible') + .prop( + 'value', + $options.has(':checked').find('input').attr('id') === $options.eq(0).find('input').attr('id') ? + 'Later' : 'Choose a different time' + ); + + // Workaround because GOV.UK SelectionButtons doesn’t deselect in this case + var deselectUnchecked = $options => $options + .filter( + (index, element) => $(element).not(':has(:checked)') + ).removeClass('selected'); + + var refocus = $element => setTimeout( + () => $element.blur().trigger('focus'), + 10 + ); + + var renderIfComponentLosesFocus = ($options, $button, $focused) => () => + ($focused.attr('type') !== 'radio') && + render($options, $button) && + refocus($focused); // Make sure that window scrolls to focused element + + Modules.RadioSelect = function() { + + this.start = function(component) { + + let $component = $(component); + let $options = $('label', $component); + + $component.append( + $button = $('') + ); + + $button.on('click', () => + $options.addClass('js-visible').has(':checked').focus() && + $button.removeClass('js-visible') + ); + + $component.on('keydown', 'input[type=radio]', function() { + + // intercept keypresses which aren’t enter or space + if (event.which !== 13 && event.which !== 32) { + setTimeout( + renderIfComponentLosesFocus($options, $button, $(document.activeElement)), + 200 + ); + return true; + } + + event.preventDefault(); + + render($options, $button); + refocus($(this)); + + }); + + $component.on('click', 'input[type=radio]', function(event) { + + deselectUnchecked($options); + + // stop click being triggered by keyboard events + if (!event.pageX) return true; + + render($options, $button); + refocus($(this)); + + }); + + render($options, $button); + + }; + + }; + +})(window.GOVUK.Modules); diff --git a/app/assets/stylesheets/components/radio-select.scss b/app/assets/stylesheets/components/radio-select.scss new file mode 100644 index 000000000..7462eed1e --- /dev/null +++ b/app/assets/stylesheets/components/radio-select.scss @@ -0,0 +1,47 @@ +.radio-select { + + &-column { + + display: inline-block; + vertical-align: top; + + .block-label { + margin-right: 10px; + } + + } + + .tertiary-button { + display: inline-block; + vertical-align: top; + width: auto; + padding: 20px 30px 15px 30px; + } + + .js-enabled & { + + + .block-label { + &:last-child { + margin-bottom: 10px; + } + } + + .block-label, + .tertiary-button { + display: none; + } + + .js-visible { + + display: block; + + &.tertiary-button { + display: inline-block; + } + + } + + } + +} diff --git a/app/assets/stylesheets/components/sms-message.scss b/app/assets/stylesheets/components/sms-message.scss index 29696bd04..9a60ecd5a 100644 --- a/app/assets/stylesheets/components/sms-message.scss +++ b/app/assets/stylesheets/components/sms-message.scss @@ -32,7 +32,7 @@ .sms-message-recipient { @include copy-19; color: $secondary-text-colour; - margin: 10px 0 5px 0; + margin: 10px 0 0 0; } .sms-message-name { diff --git a/app/assets/stylesheets/main.scss b/app/assets/stylesheets/main.scss index 7f6450c7b..98294ae48 100644 --- a/app/assets/stylesheets/main.scss +++ b/app/assets/stylesheets/main.scss @@ -49,6 +49,7 @@ $path: '/static/images/'; @import 'components/email-message'; @import 'components/api-key'; @import 'components/vendor/previous-next-navigation'; +@import 'components/radio-select'; @import 'components/pill'; @import 'components/secondary-button'; @import 'components/show-more'; diff --git a/app/main/forms.py b/app/main/forms.py index 343deb062..1cfb20a68 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1,4 +1,6 @@ +import pytz from flask_wtf import Form +from datetime import datetime, timedelta from notifications_utils.recipients import ( validate_phone_number, InvalidPhoneError @@ -22,6 +24,30 @@ from app.main.validators import (Blacklist, CsvFileValidator, ValidEmailDomainRe from app.notify_client.api_key_api_client import KEY_TYPE_NORMAL, KEY_TYPE_TEST, KEY_TYPE_TEAM +def get_time_value_and_label(future_time): + return ( + future_time.replace(tzinfo=None).isoformat(), + get_human_time(future_time.astimezone(pytz.timezone('Europe/London'))) + ) + + +def get_human_time(time): + return { + '0': 'Midnight', + '12': 'Midday' + }.get( + time.strftime('%-H'), + time.strftime('%-I%p').lower() + ) + + +def get_next_hours_from(now, hours=23): + return [ + (now + timedelta(hours=i)).replace(minute=0, second=0).replace(tzinfo=pytz.utc) + for i in range(1, hours + 1) + ] + + def email_address(label='Email address'): return EmailField(label, validators=[ Length(min=5, max=255), @@ -288,6 +314,23 @@ class ConfirmMobileNumberForm(Form): raise ValidationError(msg) +class ChooseTimeForm(Form): + + def __init__(self, *args, **kwargs): + super(ChooseTimeForm, self).__init__(*args, **kwargs) + self.scheduled_for.choices = [('', 'Now')] + [ + get_time_value_and_label(hour) for hour in get_next_hours_from(datetime.utcnow()) + ] + + scheduled_for = RadioField( + 'When should Notify send these messages?', + default='', + validators=[ + DataRequired() + ] + ) + + class CreateKeyForm(Form): def __init__(self, existing_key_names=[], *args, **kwargs): self.existing_key_names = [x.lower() for x in existing_key_names] diff --git a/app/main/views/index.py b/app/main/views/index.py index 569d716c8..bd350fe12 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -31,7 +31,7 @@ def cookies(): @main.route('/trial-mode') def trial_mode(): - return render_template('views/trial-mode.html') + return render_template('views/trial-mode.html', hours=hours) @main.route('/pricing') diff --git a/app/main/views/send.py b/app/main/views/send.py index 851042b7b..73c91ded2 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -1,6 +1,6 @@ import json import itertools -from datetime import datetime +from datetime import datetime, timedelta from string import ascii_uppercase from contextlib import suppress @@ -23,7 +23,7 @@ from notifications_utils.template import Template from notifications_utils.recipients import RecipientCSV, first_column_heading, validate_and_format_phone_number from app.main import main -from app.main.forms import CsvUploadForm +from app.main.forms import CsvUploadForm, ChooseTimeForm from app.main.uploader import ( s3upload, s3download @@ -276,6 +276,7 @@ def check_messages(service_id, template_type, upload_id): upload_id=upload_id, form=CsvUploadForm(), remaining_messages=remaining_messages, + choose_time_form=ChooseTimeForm(), back_link=back_link, help=get_help_argument() ) @@ -310,7 +311,8 @@ def start_job(service_id, upload_id): service_id, upload_data.get('template_id'), upload_data.get('original_file_name'), - upload_data.get('notification_count') + upload_data.get('notification_count'), + scheduled_for=request.form.get('scheduled_for', '') ) return redirect( diff --git a/app/notify_client/job_api_client.py b/app/notify_client/job_api_client.py index bd9f06d57..3db4c6631 100644 --- a/app/notify_client/job_api_client.py +++ b/app/notify_client/job_api_client.py @@ -58,13 +58,15 @@ class JobApiClient(BaseAPIClient): return jobs - def create_job(self, job_id, service_id, template_id, original_file_name, notification_count): + def create_job(self, job_id, service_id, template_id, original_file_name, notification_count, scheduled_for=None): data = { "id": job_id, "template": template_id, "original_file_name": original_file_name, "notification_count": notification_count } + if scheduled_for: + data.update({'scheduled_for': scheduled_for}) data = _attach_current_user(data) job = self.post(url='/service/{}/job'.format(service_id), data=data) diff --git a/app/templates/components/radio-select.html b/app/templates/components/radio-select.html new file mode 100644 index 000000000..e69de29bb diff --git a/app/templates/components/radios.html b/app/templates/components/radios.html index 2fba0f566..3a0f19a7c 100644 --- a/app/templates/components/radios.html +++ b/app/templates/components/radios.html @@ -23,6 +23,40 @@ {% endmacro %} +{% macro radio_select( + field, + hint=None, + wrapping_class='form-group' +) %} +
+
+ + {{ field.label }} + {% if field.errors %} + + {{ field.errors[0] }} + + {% endif %} + +
+
+ {% for option in field %} + + {% if loop.first %} +
+
+ {% endif %} + {% endfor %} +
+
+
+
+{% endmacro %} + + {% macro branding_radios( field, hint=None, diff --git a/app/templates/views/check.html b/app/templates/views/check.html index cb073ef72..998547874 100644 --- a/app/templates/views/check.html +++ b/app/templates/views/check.html @@ -1,6 +1,7 @@ {% extends "withnav_template.html" %} {% from "components/banner.html" import banner_wrapper %} {% from "components/email-message.html" import email_message %} +{% from "components/radios.html" import radio_select %} {% from "components/sms-message.html" import sms_message %} {% from "components/table.html" import list_table, field, text_field, index_field, hidden_field_heading %} {% from "components/file-upload.html" import file_upload %} @@ -144,6 +145,10 @@
+ {{ radio_select( + choose_time_form.scheduled_for, + wrapping_class='bottom-gutter-2-3' + ) }} Back
diff --git a/app/templates/views/trial-mode.html b/app/templates/views/trial-mode.html index 4bc76d36d..a7de3fb7c 100644 --- a/app/templates/views/trial-mode.html +++ b/app/templates/views/trial-mode.html @@ -9,7 +9,6 @@

Trial mode

-

All new accounts on Notify start off in trial mode.

diff --git a/gulpfile.babel.js b/gulpfile.babel.js index 51bc795d5..b4452b5da 100644 --- a/gulpfile.babel.js +++ b/gulpfile.babel.js @@ -60,8 +60,9 @@ gulp.task('javascripts', () => gulp paths.src + 'javascripts/autofocus.js', paths.src + 'javascripts/highlightTags.js', paths.src + 'javascripts/fileUpload.js', - paths.src + 'javascripts/updateContent.js', paths.src + 'javascripts/expandCollapse.js', + paths.src + 'javascripts/radioSelect.js', + paths.src + 'javascripts/updateContent.js', paths.src + 'javascripts/main.js' ]) .pipe(plugins.babel({ diff --git a/tests/app/main/test_choose_time_form.py b/tests/app/main/test_choose_time_form.py new file mode 100644 index 000000000..70977908f --- /dev/null +++ b/tests/app/main/test_choose_time_form.py @@ -0,0 +1,22 @@ +import pytest + +from app.main.forms import ChooseTimeForm +from freezegun import freeze_time + + +@freeze_time("2016-01-01 11:09:00.061258") +def test_form_contains_next_24h(app_): + + choices = ChooseTimeForm().scheduled_for.choices + + assert choices[0] == ('', 'Now') + assert choices[1] == ('2016-01-01T12:00:00.061258', 'Midday') + assert choices[23] == ('2016-01-02T10:00:00.061258', '10am') + + with pytest.raises(IndexError): + assert choices[24] + + +@freeze_time("2016-01-01 11:09:00.061258") +def test_form_defaults_to_now(app_): + assert ChooseTimeForm().scheduled_for.data == '' diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index a7aed4e44..892556f42 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -322,6 +322,11 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers( mock_get_detailed_service_for_today.assert_called_once_with(fake_uuid) +@pytest.mark.parametrize( + 'when', [ + '', '2016-08-25T13:04:21.767198' + ] +) def test_create_job_should_call_api( app_, service_one, @@ -331,7 +336,8 @@ def test_create_job_should_call_api( mock_get_notifications, mock_get_service_template, mocker, - fake_uuid + fake_uuid, + when ): service_id = service_one['id'] data = mock_get_job(service_one['id'], fake_uuid)['data'] @@ -348,11 +354,18 @@ def test_create_job_should_call_api( 'notification_count': notification_count, 'valid': True} url = url_for('main.start_job', service_id=service_one['id'], upload_id=job_id) - response = client.post(url, data={}, follow_redirects=True) + response = client.post(url, data={'scheduled_for': when}, follow_redirects=True) assert response.status_code == 200 assert original_file_name in response.get_data(as_text=True) - mock_create_job.assert_called_with(job_id, service_id, template_id, original_file_name, notification_count) + mock_create_job.assert_called_with( + job_id, + service_id, + template_id, + original_file_name, + notification_count, + scheduled_for=when + ) def test_check_messages_should_revalidate_file_when_uploading_file( diff --git a/tests/app/notify_client/test_job_client.py b/tests/app/notify_client/test_job_client.py index 5009c7c6a..f2ff70560 100644 --- a/tests/app/notify_client/test_job_client.py +++ b/tests/app/notify_client/test_job_client.py @@ -27,6 +27,21 @@ def test_client_creates_job_data_correctly(mocker, fake_uuid): mock_post.assert_called_once_with(url=expected_url, data=expected_data) +def test_client_schedules_job(mocker, fake_uuid): + + mocker.patch('app.notify_client.current_user', id='1') + + mock_post = mocker.patch('app.notify_client.job_api_client.JobApiClient.post') + + when = '2016-08-25T13:04:21.767198' + + JobApiClient().create_job( + fake_uuid, fake_uuid, fake_uuid, fake_uuid, 1, scheduled_for=when + ) + + assert mock_post.call_args[1]['data']['scheduled_for'] == when + + def test_client_gets_job_by_service_and_job(mocker): mocker.patch('app.notify_client.current_user', id='1') diff --git a/tests/conftest.py b/tests/conftest.py index 854b44baf..012f40150 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -837,7 +837,7 @@ def mock_check_verify_code_code_expired(mocker): @pytest.fixture(scope='function') def mock_create_job(mocker, api_user_active): - def _create(job_id, service_id, template_id, file_name, notification_count): + def _create(job_id, service_id, template_id, file_name, notification_count, scheduled_for=None): return job_json( service_id, api_user_active,