From c26d308a32b509cd7d4d262e78a2526442991e9f Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 14 Sep 2017 12:21:40 +0100 Subject: [PATCH 1/9] Show row errors before trial mode errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s useful to be able to play with the CSV upload thing and find out how it works before you’re interrupted by the error telling you that you’ve got as far as you can in trial mode. --- app/main/views/send.py | 8 +++++--- tests/app/main/views/test_send.py | 32 +++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index 870fdcb48..757f8cbe0 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -459,15 +459,17 @@ def check_messages(service_id, template_type, upload_id): data['recipients'].too_many_rows or not data['count_of_recipients'] or not data['recipients'].has_recipient_columns or - data['recipients'].missing_column_headers or - data['trying_to_send_letters_in_trial_mode'] + data['recipients'].missing_column_headers ): return render_template('views/check/column-errors.html', **data) if data['row_errors']: return render_template('views/check/row-errors.html', **data) - if data['errors']: + if ( + data['errors'] or + data['trying_to_send_letters_in_trial_mode'] + ): return render_template('views/check/column-errors.html', **data) return render_template('views/check/ok.html', **data) diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index d80548416..45e6b4b44 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -1631,6 +1631,38 @@ def test_check_messages_shows_trial_mode_error_for_letters( assert not error +def test_check_messages_shows_data_errors_before_trial_mode_errors_for_letters( + mocker, + client_request, + mock_get_service_letter_template, + mock_has_permissions, + mock_get_users_by_service, + mock_get_detailed_service_for_today, +): + + mocker.patch('app.main.views.send.s3download', return_value='\n'.join( + ['address_line_1,address_line_2,postcode,'] + + [' , ,11SW1 1AA'] + )) + + with client_request.session_transaction() as session: + session['upload_data'] = {'template_id': '', 'original_file_name': 'example.xlsx'} + + page = client_request.get( + 'main.check_messages', + service_id=SERVICE_ONE_ID, + template_type='letter', + upload_id=uuid.uuid4(), + _test_page_title=False, + ) + + assert normalize_spaces(page.select_one('.banner-dangerous').text) == ( + 'There is a problem with example.xlsx ' + 'You need to enter missing data in 1 row ' + 'Skip to file contents' + ) + + def test_generate_test_letter_doesnt_block_in_trial_mode( client_request, mocker, From 2a84d14a73d134e9f55991676ddb9a39a1f36bd8 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 14 Sep 2017 13:10:56 +0100 Subject: [PATCH 2/9] Fix overlapping text on settings page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Think this broke when we split the setting page up into three sections. This forces the text to wrap onto multiple lines even if it doesn’t contain spaces (for example an email address). --- app/assets/stylesheets/app.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/app/assets/stylesheets/app.scss b/app/assets/stylesheets/app.scss index 1e9d32b95..c3a1ba93b 100644 --- a/app/assets/stylesheets/app.scss +++ b/app/assets/stylesheets/app.scss @@ -180,6 +180,7 @@ details summary { th, td { @include core-19; + word-wrap: break-word; } thead { From d7370c181692303bf668572c84092f6c7e2da770 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 14 Sep 2017 13:03:27 +0100 Subject: [PATCH 3/9] Rename manage templates permission MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ‘Add and edit’ is the language we use next to the checkbox. Also helps disambiguate it from the ‘manage service’ permission. --- app/templates/views/manage-users.html | 4 ++-- tests/app/main/views/test_manage_users.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/templates/views/manage-users.html b/app/templates/views/manage-users.html index fb79b3b8a..373c5b3a6 100644 --- a/app/templates/views/manage-users.html +++ b/app/templates/views/manage-users.html @@ -53,7 +53,7 @@ ) }} {{ tick_cross( user.has_permissions(permissions=['manage_templates']), - 'Manage templates' + 'Add and edit templates' ) }} {{ tick_cross( user.has_permissions(permissions=['manage_users', 'manage_settings']), @@ -94,7 +94,7 @@ ) }} {{ tick_cross( user.has_permissions(permissions=['manage_templates']), - 'Edit templates' + 'Add and edit templates' ) }} {{ tick_cross( user.has_permissions(permissions=['manage_users', 'manage_settings']), diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index ce97036f0..2766cd032 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -19,21 +19,21 @@ from tests.conftest import ( active_user_with_permissions, ( 'Test User (you) ' - 'Can Send messages Can Manage templates Can Manage service Can Access API keys' + 'Can Send messages Can Add and edit templates Can Manage service Can Access API keys' ), ), ( active_user_view_permissions, ( 'Test User With Permissions (you) ' - 'Can’t Send messages Can’t Manage templates Can’t Manage service Can’t Access API keys' + 'Can’t Send messages Can’t Add and edit templates Can’t Manage service Can’t Access API keys' ), ), ( active_user_manage_template_permission, ( 'Test User With Permissions (you) ' - 'Can’t Send messages Can Manage templates Can’t Manage service Can’t Access API keys' + 'Can’t Send messages Can Add and edit templates Can’t Manage service Can’t Access API keys' ), ), ]) @@ -255,7 +255,7 @@ def test_manage_users_shows_invited_user( assert page.h1.string.strip() == 'Team members' assert normalize_spaces(page.select('.user-list')[1].text) == ( 'invited_user@test.gov.uk ' - 'Can’t Send messages Can’t Edit templates Can’t Manage service Can Access API keys ' + 'Can’t Send messages Can’t Add and edit templates Can’t Manage service Can Access API keys ' 'Cancel invitation' ) From 4ac90c066a91bcbda6a1ba6133fc69d0d9a44c51 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 15 Sep 2017 14:10:21 +0100 Subject: [PATCH 4/9] Prevent doubling clicking form submissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Have seen users complaining that they got an invitation email twice. This is probably because they clicked the button twice even though they think they only clicked it once. Double form submission is a common issue on web pages, and there are a number of different ways to prevent it. I’ve chosen to do it this way because: - temporarily, not permanently disabling the button means that this addresses the double clicking issue without breaking things if the user did, really want to click the button again deliberately (for whatever reason) - doing it with a `data` attribute, rather than the `disabled` attribute means that the interaction behaviour of the button doesn’t change ( `disabled` buttons can’t be focused, for example) --- .../preventDuplicateFormSubmissions.js | 30 +++++++++++++++++++ gulpfile.babel.js | 1 + 2 files changed, 31 insertions(+) create mode 100644 app/assets/javascripts/preventDuplicateFormSubmissions.js diff --git a/app/assets/javascripts/preventDuplicateFormSubmissions.js b/app/assets/javascripts/preventDuplicateFormSubmissions.js new file mode 100644 index 000000000..0eca28039 --- /dev/null +++ b/app/assets/javascripts/preventDuplicateFormSubmissions.js @@ -0,0 +1,30 @@ +(function() { + + "use strict"; + + let disableSubmitButtons = function(event) { + + $submitButton = $(this).find(':submit'); + + if ($submitButton.data('clicked') == 'true') { + + event.preventDefault(); + + } else { + + $submitButton.data('clicked', 'true'); + setTimeout(renableSubmitButton($submitButton), 1500); + + } + + }; + + let renableSubmitButton = $submitButton => () => { + + $submitButton.data('clicked', ''); + + }; + + $('form').on('submit', disableSubmitButtons); + +})(); diff --git a/gulpfile.babel.js b/gulpfile.babel.js index 26dd6c96b..9ecc9c15d 100644 --- a/gulpfile.babel.js +++ b/gulpfile.babel.js @@ -67,6 +67,7 @@ gulp.task('javascripts', () => gulp paths.src + 'javascripts/listEntry.js', paths.src + 'javascripts/liveSearch.js', paths.src + 'javascripts/errorTracking.js', + paths.src + 'javascripts/preventDuplicateFormSubmissions.js', paths.src + 'javascripts/main.js' ]) .pipe(plugins.prettyerror()) From d6545ad10484bc4bde2a896b4412742b6a792632 Mon Sep 17 00:00:00 2001 From: pyup-bot Date: Sat, 16 Sep 2017 01:22:27 +0100 Subject: [PATCH 5/9] Update pytest-mock from 1.6.2 to 1.6.3 --- requirements_for_test.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements_for_test.txt b/requirements_for_test.txt index 9abf5197e..ea72849ef 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -1,7 +1,7 @@ -r requirements.txt pycodestyle==2.3.1 pytest==3.2.2 -pytest-mock==1.6.2 +pytest-mock==1.6.3 pytest-cov==2.5.1 pytest-xdist==1.20.0 coveralls==1.2.0 From 8fca1345c7b94405b578a2c85d7350ede5fc9ef7 Mon Sep 17 00:00:00 2001 From: pyup-bot Date: Sat, 16 Sep 2017 16:47:27 +0100 Subject: [PATCH 6/9] Update cffi from 1.10.0 to 1.11.0 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 0b1bd52a6..81f7898ae 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ ago==0.0.9 -cffi==1.10.0 +cffi==1.11.0 Flask==0.12.2 Flask-Script==2.0.5 Flask-WTF==0.14.2 From a49c30a92dd6f161a8a9ee8d2ccdd9448d33e3d4 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 18 Sep 2017 09:34:31 +0100 Subject: [PATCH 7/9] Remove six dependency six is used for Python 2/3 compatibility. We never run under Python 2. --- requirements.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 0b1bd52a6..a13eb7234 100644 --- a/requirements.txt +++ b/requirements.txt @@ -15,7 +15,6 @@ pyexcel-xls==0.5.0 pyexcel-xlsx==0.5.0.1 pyexcel-ods3==0.5.0 pytz==2017.2 -six==1.10.0 gunicorn==19.7.1 whitenoise==3.3.0 #manages static assets From 0789c37d4830932db81e9dc5382f1ed4bfa559d2 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 18 Sep 2017 09:54:40 +0100 Subject: [PATCH 8/9] Remove lxml package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don’t use lxml directly. Our dependencies (eg html5lib) do, but we should just install whatever version they specify. --- requirements.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 0b1bd52a6..c32952e6d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,7 +8,6 @@ Flask-Login==0.4.0 boto3==1.4.7 py-gfm==0.1.3 blinker==1.4 -lxml==3.8.0 pyexcel==0.5.3 pyexcel-io==0.5.1 pyexcel-xls==0.5.0 From 4d1c10c48ce5dc8eb27c88497da93591b39e62e7 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 18 Sep 2017 10:15:07 +0100 Subject: [PATCH 9/9] Remove Github-Flavored Markdown dependency This was used when we published the documentation in the app. --- requirements.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 0b1bd52a6..707176e0e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,7 +6,6 @@ Flask-WTF==0.14.2 Flask-Login==0.4.0 boto3==1.4.7 -py-gfm==0.1.3 blinker==1.4 lxml==3.8.0 pyexcel==0.5.3