From 324e1f9ef400f0129940c5c217d6b76eb4c38342 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 11 Oct 2016 14:11:10 +0100 Subject: [PATCH 01/56] Allow a job to be scheduled any time in next 96hrs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If you want to send a job on Monday morning, you should be able to schedule it on Friday. You shouldn’t need to work on the weekend. 96 hours is a full 4 days, so you can schedule a job at any time on Friday for any time on Monday. We’ve checked with the information assurance people, and they’re OK with us holding the data for this extra amount of time. This commit changes the choose time form from showing one radio button for each of the next 24 hours to one for each of the next 96 hours. It changes the labels from ‘9am’ to ‘Monday at 9am’ so it’s clear which day you’re choosing. --- app/main/forms.py | 40 +++++++++++++++++++++---- app/main/views/send.py | 2 +- tests/app/main/test_choose_time_form.py | 22 ++++++++++++-- 3 files changed, 55 insertions(+), 9 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 650f72891..ccd9fc14a 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -27,27 +27,55 @@ from app.main.validators import (Blacklist, CsvFileValidator, ValidGovEmail, NoC 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'))) + '{} at {}'.format( + get_human_day(future_time.astimezone(pytz.timezone('Europe/London'))), + get_human_time(future_time.astimezone(pytz.timezone('Europe/London'))) + ) ) def get_human_time(time): return { - '0': 'Midnight', - '12': 'Midday' + '0': 'midnight', + '12': 'midday' }.get( time.strftime('%-H'), time.strftime('%-I%p').lower() ) -def get_next_hours_from(now, hours=23): +def get_human_day(time): + # Add 1 hour to get ‘midnight today’ instead of ‘midnight tomorrow’ + time = (time - timedelta(hours=1)).strftime('%A') + if time == datetime.utcnow().strftime('%A'): + return 'Today' + if time == (datetime.utcnow() + timedelta(days=1)).strftime('%A'): + return 'Tomorrow' + return time + + +def get_furthest_possible_scheduled_time(): + return (datetime.utcnow() + timedelta(days=4)).replace(hour=0) + + +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) for i in range(1, hours + 1) ] +def get_next_days_until(until): + now = datetime.utcnow() + days = int((until - now).total_seconds() / (60 * 60 * 24)) + return [ + get_human_day((now + timedelta(days=i)).replace(tzinfo=pytz.utc)) + for i in range(0, days + 1) + ] + + def email_address(label='Email address', gov_user=True): validators = [ Length(min=5, max=255), @@ -310,7 +338,9 @@ 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()) + get_time_value_and_label(hour) for hour in get_next_hours_until( + get_furthest_possible_scheduled_time() + ) ] scheduled_for = RadioField( diff --git a/app/main/views/send.py b/app/main/views/send.py index 2ef85dd24..9766e7c7c 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -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, ChooseTimeForm +from app.main.forms import CsvUploadForm, ChooseTimeForm, get_next_days_until, get_furthest_possible_scheduled_time from app.main.uploader import ( s3upload, s3download diff --git a/tests/app/main/test_choose_time_form.py b/tests/app/main/test_choose_time_form.py index 70977908f..82b858ccb 100644 --- a/tests/app/main/test_choose_time_form.py +++ b/tests/app/main/test_choose_time_form.py @@ -9,12 +9,28 @@ def test_form_contains_next_24h(app_): choices = ChooseTimeForm().scheduled_for.choices + # Friday assert choices[0] == ('', 'Now') - assert choices[1] == ('2016-01-01T12:00:00.061258', 'Midday') - assert choices[23] == ('2016-01-02T10:00:00.061258', '10am') + assert choices[1] == ('2016-01-01T12:00:00.061258', 'Today at midday') + assert choices[13] == ('2016-01-02T00:00:00.061258', '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') + + # Sunday + assert choices[38] == ('2016-01-03T01:00:00.061258', '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') with pytest.raises(IndexError): - assert choices[24] + assert choices[ + 12 + # hours left in the day + (3 * 24) + # 3 days + 2 # magic number + ] @freeze_time("2016-01-01 11:09:00.061258") From a78d9d5048b7dfad6782ac039ad808882b4765f2 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 11 Oct 2016 14:17:29 +0100 Subject: [PATCH 02/56] Group choices for scheduling a job by day MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The options for scheduling a job by time should be grouped by day, because a long list of 96 options is not very usable. On the server side, this commit generates label for the next 4 days in a friendly format (ie today/tomorrow/Sunday/Monday) The Javascript component for choosing a time was built in a kind of old-school jQuery way, where it manipulated the elements on the page. The complexity of introducing groups of options was just too much for this pattern, because it involves storing a lot of state in the DOM. This commit completely rewrites the JS to: - read the initial options and groups from the HTML and store them in the object - use Hogan to completely re-render the UI from a series of Mustache templates, each of which represents a state of the UI and takes the inital options and groups - filter the choices to show when the today/tomorrow/… buttons are clicked --- app/assets/javascripts/radioSelect.js | 187 +++++++++++------- .../stylesheets/components/radio-select.scss | 29 +-- app/main/forms.py | 1 + app/templates/components/radios.html | 2 +- tests/app/main/test_choose_time_form.py | 7 + 5 files changed, 134 insertions(+), 92 deletions(-) diff --git a/app/assets/javascripts/radioSelect.js b/app/assets/javascripts/radioSelect.js index 8923376ef..35e1ebacf 100644 --- a/app/assets/javascripts/radioSelect.js +++ b/app/assets/javascripts/radioSelect.js @@ -2,89 +2,142 @@ "use strict"; - var render = ($options, $button) => ( - filterOptionVisibility($options) && setButtonState($options, $button) - ); + let states = { + 'initial': Hogan.compile(` +
+ +
+
+ {{#categories}} + + {{/categories}} +
+ `), + 'choose': Hogan.compile(` +
+ +
+
+ {{#choices}} + + {{/choices}} +
+ `), + 'chosen': Hogan.compile(` +
+ +
+
+ {{#choices}} + + {{/choices}} +
+
+ +
+ `) + }; - 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' + let focusSelected = function() { + setTimeout( + () => $('[type=radio]:checked').parent('label').blur().trigger('focus').addClass('selected'), + 10 ); - - // 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)); - + let render = (state, data) => $component.html(states[state].render(data)); + let choices = $('label', $component).toArray().map(function(element) { + let $element = $(element); + return { + 'id': $element.attr('for'), + 'label': $.trim($element.text()), + 'value': $element.find('input').attr('value') + }; }); + let categories = $component.data('categories').split(','); + let name = $component.find('input').eq(0).attr('name'); - $component.on('click', 'input[type=radio]', function(event) { + $component + .on('click', '.js-category-button', function(event) { - deselectUnchecked($options); + event.preventDefault(); + let day = $(this).attr('value'); + render('choose', { + 'choices': choices.filter( + element => element.label.indexOf(day) > -1 + ), + 'name': name + }); + $('.js-option').eq(0).parent('label').trigger('focus'); - // stop click being triggered by keyboard events - if (!event.pageX) return true; + }) + .on('click', '.js-option', function(event) { - render($options, $button); - refocus($(this)); + // stop click being triggered by keyboard events + if (!event.pageX) return true; + event.preventDefault(); + let value = $(this).attr('value'); + render('chosen', { + 'choices': choices.filter( + element => element.value == value + ), + 'name': name + }); + focusSelected(); + + }) + .on('keydown', 'input[type=radio]', function(event) { + + // intercept keypresses which aren’t enter or space + if (event.which !== 13 && event.which !== 32) { + return true; + } + + event.preventDefault(); + let value = $(this).attr('value'); + render('chosen', { + 'choices': choices.filter( + element => element.value == value + ), + 'name': name + }); + focusSelected(); + + }) + .on('click', '.js-reset-button', function(event) { + + event.preventDefault(); + render('initial', { + 'categories': categories, + 'name': name + }); + focusSelected(); + + }); + + render('initial', { + 'categories': categories, + 'name': name }); - render($options, $button); - }; }; diff --git a/app/assets/stylesheets/components/radio-select.scss b/app/assets/stylesheets/components/radio-select.scss index 7462eed1e..c33a985c9 100644 --- a/app/assets/stylesheets/components/radio-select.scss +++ b/app/assets/stylesheets/components/radio-select.scss @@ -6,7 +6,9 @@ vertical-align: top; .block-label { - margin-right: 10px; + margin-right: 5px; + padding-right: $gutter - 10px; + padding-left: 54px - 10px; } } @@ -15,33 +17,12 @@ display: inline-block; vertical-align: top; width: auto; - padding: 20px 30px 15px 30px; + padding: 20px $gutter-half 15px $gutter-half; + margin-right: 5px; } .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/main/forms.py b/app/main/forms.py index ccd9fc14a..58dd1ade4 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -342,6 +342,7 @@ class ChooseTimeForm(Form): get_furthest_possible_scheduled_time() ) ] + self.scheduled_for.categories = get_next_days_until(get_furthest_possible_scheduled_time()) scheduled_for = RadioField( 'When should Notify send these messages?', diff --git a/app/templates/components/radios.html b/app/templates/components/radios.html index 3a0f19a7c..c361c443e 100644 --- a/app/templates/components/radios.html +++ b/app/templates/components/radios.html @@ -38,7 +38,7 @@ {% endif %} -
+
{% for option in field %}