diff --git a/app/assets/javascripts/fullscreenTable.js b/app/assets/javascripts/fullscreenTable.js new file mode 100644 index 000000000..27ff52636 --- /dev/null +++ b/app/assets/javascripts/fullscreenTable.js @@ -0,0 +1,107 @@ +(function(Modules) { + "use strict"; + + Modules.FullscreenTable = function() { + + this.start = function(component) { + + this.$component = $(component); + this.$table = this.$component.find('table'); + this.nativeHeight = this.$component.innerHeight() + 20; // 20px to allow room for scrollbar + this.topOffset = this.$component.offset().top; + + this.insertShims(); + this.maintainWidth(); + this.maintainHeight(); + this.toggleShadows(); + + $(window) + .on('scroll resize', this.maintainHeight) + .on('resize', this.maintainWidth); + + this.$scrollableTable + .on('scroll', this.toggleShadows) + .on('scroll', this.maintainHeight); + + if ( + window.GOVUK.stopScrollingAtFooter && + window.GOVUK.stopScrollingAtFooter.updateFooterTop + ) { + window.GOVUK.stopScrollingAtFooter.updateFooterTop(); + } + + }; + + this.insertShims = () => { + + this.$table.wrap('
'); + + this.$component + .append( + this.$component.find('.fullscreen-scrollable-table') + .clone() + .addClass('fullscreen-fixed-table') + .removeClass('fullscreen-scrollable-table') + .attr('role', 'presentation') + ) + .append( + '
' + ) + .after( + $("
").css({ + 'height': this.nativeHeight, + 'top': this.topOffset + }) + ); + + this.$scrollableTable = this.$component.find('.fullscreen-scrollable-table'); + this.$fixedTable = this.$component.find('.fullscreen-fixed-table'); + + }; + + this.maintainHeight = () => { + + let height = Math.min( + $(window).height() - this.topOffset + $('html, body').scrollTop(), + this.nativeHeight + ); + + this.$scrollableTable.outerHeight(height); + this.$fixedTable.outerHeight(height); + + }; + + this.maintainWidth = () => { + + let indexColumnWidth = this.$fixedTable.find('.table-field-index').outerWidth(); + + this.$scrollableTable + .css({ + 'width': this.$component.parent('main').width() - indexColumnWidth, + 'margin-left': indexColumnWidth + }); + + this.$fixedTable + .width(indexColumnWidth + 4); + + }; + + this.toggleShadows = () => { + + this.$fixedTable + .toggleClass( + 'fullscreen-scrolled-table', + this.$scrollableTable.scrollLeft() > 0 + ); + + this.$component.find('.fullscreen-right-shadow') + .toggleClass( + 'visible', + this.$scrollableTable.scrollLeft() < (this.$table.width() - this.$scrollableTable.width()) + ); + + }; + + }; + +})(window.GOVUK.Modules); diff --git a/app/assets/stylesheets/components/fullscreen-table.scss b/app/assets/stylesheets/components/fullscreen-table.scss new file mode 100644 index 000000000..b552fcbd8 --- /dev/null +++ b/app/assets/stylesheets/components/fullscreen-table.scss @@ -0,0 +1,134 @@ +.fullscreen { + + &-content { + + background: $white; + z-index: 10; + overflow-y: hidden; + box-sizing: border-box; + position: absolute; + margin: 5px 0 $gutter 0; + padding: 0 0 0 0; + overflow: hidden; + border-bottom: 1px solid $border-colour; + + .table { + + margin-bottom: 0; + + tr:last-child { + td { + border-bottom: 1px solid $white; + } + } + + } + + th, + .table-field-error-label, + .table-field-center-aligned { + white-space: nowrap; + } + + } + + &-right-shadow { + + position: absolute; + top: 0; + right: 0; + width: 4px; + height: 100%; + z-index: 200; + + &.visible { + transition: box-shadow 0.3s ease-in-out; + box-shadow: inset -1px 0 0 0 $border-colour, inset -3px 0 0 0 rgba($border-colour, 0.2); + } + + } + + &-scrollable-table { + + overflow-x: auto; + overflow-y: hidden; + + .table-field-heading-first, + .table-field-index { + display: none; + } + + .table-field-center-aligned { + position: relative; + z-index: 150; + background: $white; + } + + &::-webkit-scrollbar { + -webkit-appearance: none; + } + + &::-webkit-scrollbar:horizontal { + height: 11px; + background-color: $white; + } + + &::-webkit-scrollbar-thumb { + border-radius: 8px; + border: 2px solid $white; + background-color: rgba(0, 0, 0, .5); + } + + &::-webkit-scrollbar-track { + background-color: $white; + border-radius: 8px; + } + + } + + &-fixed-table { + + position: absolute; + top: 0; + overflow: hidden; + + .table-field-heading { + visibility: hidden; + } + + .table-field-center-aligned { + width: 0; + position: relative; + z-index: 100; + visibility: hidden; + } + + .table-field-heading-first, + .table-field-index { + transition: none; + position: relative; + z-index: 200; + background: $white; + } + + } + + &-scrolled-table { + + padding-bottom: 20px; + + .table-field-heading-first, + .table-field-index { + transition: box-shadow 0.3s ease-in-out; + box-shadow: 1px 0 0 0 $border-colour, 3px 0 0 0 rgba($border-colour, 0.2); + } + + } + + &-shim { + width: 100%; + position: relative; + z-index: 9; + } + +} diff --git a/app/assets/stylesheets/components/stick-at-top-when-scrolling.scss b/app/assets/stylesheets/components/stick-at-top-when-scrolling.scss index cd87884f1..77d076ff1 100644 --- a/app/assets/stylesheets/components/stick-at-top-when-scrolling.scss +++ b/app/assets/stylesheets/components/stick-at-top-when-scrolling.scss @@ -16,9 +16,24 @@ margin-bottom: 20px; } + .back-to-top-link { + + position: absolute; + top: $gutter; + right: $gutter-half; + opacity: 0; + transition: opacity 0.1s ease-in-out; + + @include ie-lte(8) { + display: none; + } + + } + } .content-fixed { + position: fixed; top: 0; background: $white; @@ -28,8 +43,15 @@ border-bottom: 1px solid $border-colour; box-shadow: 0 2px 0 0 rgba($border-colour, 0.2); transition: background 0.6s ease-in-out, margin-top 0.4s ease-out; + + .back-to-top-link { + opacity: 1; + transition: opacity 0.6s ease-in-out; + } + } .shim { display: block; + margin-bottom: 5px; } diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index c86d5cc12..5bd4c68c0 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -209,10 +209,9 @@ .table-show-more-link { @include core-16; color: $secondary-text-colour; - margin-top: -30px; margin-bottom: $gutter * 1.3333; border-bottom: 1px solid $border-colour; - padding: 0.75em 0 0.5625em 0; + padding: 10px 0 10px 0; text-align: center; } diff --git a/app/assets/stylesheets/main.scss b/app/assets/stylesheets/main.scss index ec57bf390..d4a2315df 100644 --- a/app/assets/stylesheets/main.scss +++ b/app/assets/stylesheets/main.scss @@ -59,6 +59,7 @@ $path: '/static/images/'; @import 'components/letter'; @import 'components/live-search'; @import 'components/stick-at-top-when-scrolling'; +@import 'components/fullscreen-table'; @import 'components/vendor/breadcrumbs'; @import 'components/vendor/responsive-embed'; diff --git a/app/main/views/send.py b/app/main/views/send.py index 6e8b81f44..f375c1645 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -2,7 +2,6 @@ import itertools from string import ascii_uppercase from orderedset import OrderedSet -from contextlib import suppress from zipfile import BadZipFile from xlrd.biffh import XLRDError from werkzeug.routing import RequestRedirect @@ -21,7 +20,6 @@ from flask import ( from flask_login import login_required, current_user from notifications_python_client.errors import HTTPError -from notifications_utils.columns import Columns from notifications_utils.recipients import ( RecipientCSV, first_column_headings, @@ -461,7 +459,7 @@ def send_test_preview(service_id, template_id, filetype): return TemplatePreview.from_utils_template(template, filetype, page=request.args.get('page')) -def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False): +def _check_messages(service_id, template_type, upload_id, preview_row, letters_as_pdf=False): if not session.get('upload_data'): # if we just return a `redirect` (302) object here, we'll get errors when we try and unpack in the @@ -497,6 +495,7 @@ def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False): template_type=template_type, upload_id=upload_id, filetype='png', + row_index=preview_row, ) if not letters_as_pdf else None, email_reply_to=email_reply_to, sms_sender=sms_sender @@ -522,23 +521,21 @@ def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False): back_link = url_for('.send_messages', service_id=service_id, template_id=template.id) choose_time_form = ChooseTimeForm() - with suppress(StopIteration): - first_recipient = None - template.values = next(recipients.rows) - first_recipient = template.values.get( - Columns.make_key(recipients.recipient_column_headers[0]), - '' - ) + count_of_recipients = len(list(recipients.rows)) - session['upload_data']['notification_count'] = len(list(recipients.rows)) + if preview_row < count_of_recipients: + template.values = recipients[preview_row] + elif preview_row > 0: + abort(404) + + session['upload_data']['notification_count'] = count_of_recipients session['upload_data']['valid'] = not recipients.has_errors return dict( recipients=recipients, - first_recipient=first_recipient, template=template, errors=recipients.has_errors, row_errors=get_errors_for_csv(recipients, template.template_type), - count_of_recipients=session['upload_data']['notification_count'], + count_of_recipients=count_of_recipients, count_of_displayed_recipients=( len(list(recipients.initial_annotated_rows_with_errors)) if any(recipients.rows_with_errors) and not recipients.missing_column_headers else @@ -561,11 +558,12 @@ def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False): @main.route("/services///check/", methods=['GET']) +@main.route("/services///check//row-", methods=['GET']) @login_required @user_has_permissions('send_texts', 'send_emails', 'send_letters') -def check_messages(service_id, template_type, upload_id): +def check_messages(service_id, template_type, upload_id, row_index=0): - data = _check_messages(service_id, template_type, upload_id) + data = _check_messages(service_id, template_type, upload_id, row_index) if ( data['recipients'].too_many_rows or @@ -588,22 +586,24 @@ def check_messages(service_id, template_type, upload_id): @main.route("/services///check/.", methods=['GET']) +@main.route("/services///check//row-.", methods=['GET']) @login_required @user_has_permissions('send_texts', 'send_emails', 'send_letters') -def check_messages_preview(service_id, template_type, upload_id, filetype): +def check_messages_preview(service_id, template_type, upload_id, filetype, row_index=0): if filetype not in ('pdf', 'png'): abort(404) template = _check_messages( - service_id, template_type, upload_id, letters_as_pdf=True + service_id, template_type, upload_id, row_index, letters_as_pdf=True )['template'] return TemplatePreview.from_utils_template(template, filetype) @main.route("/services///check/", methods=['POST']) +@main.route("/services///check//row-", methods=['POST']) @login_required @user_has_permissions('send_texts', 'send_emails', 'send_letters') -def recheck_messages(service_id, template_type, upload_id): +def recheck_messages(service_id, template_type, upload_id, row_index=0): if not session.get('upload_data'): return redirect(url_for('main.choose_template', service_id=service_id)) diff --git a/app/templates/views/check/column-errors.html b/app/templates/views/check/column-errors.html index 553449600..4fec14c21 100644 --- a/app/templates/views/check/column-errors.html +++ b/app/templates/views/check/column-errors.html @@ -38,12 +38,18 @@

Your file is missing some rows

-

- It needs at least one row of data, and {{ recipients.missing_column_headers | sort() | formatted_list( - prefix='a column called', - prefix_plural='columns called' - ) }}. -

+ {% if recipients.missing_column_headers %} +

+ It needs at least one row of data, and {{ recipients.missing_column_headers | sort() | formatted_list( + prefix='a column called', + prefix_plural='columns called' + ) }}. +

+ {% else %} +

+ It needs at least one row of data. +

+ {% endif %} {% elif not recipients.has_recipient_columns %} @@ -108,54 +114,58 @@ {% endcall %}
-
- {% if request.args.from_test %} - Back - {% else %} - {{file_upload(form.file, button_text='Re-upload your file')}} - {% endif %} + +
+
+ {% if request.args.from_test %} + Back + {% else %} + {{file_upload(form.file, button_text='Re-upload your file')}} + {% endif %} +
+ Back to top
{% if not request.args.from_test %}

{{ original_file_name }}

- {% call(item, row_number) list_table( - recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows, - caption=original_file_name, - caption_visible=False, - field_headings=[ - 'Row in file'|safe - ] + recipients.column_headers - ) %} - {% call index_field() %} - - {{ item.index + 2 }} - - {% endcall %} - {% for column in recipients.column_headers %} - {% if item['columns'][column].error and not recipients.missing_column_headers %} - {% call field() %} - - {{ item['columns'][column].error }} - {{ item['columns'][column].data if item['columns'][column].data != None }} - - {% endcall %} - {% elif item['columns'][column].ignore %} - {{ text_field(item['columns'][column].data or '', status='default') }} - {% else %} - {{ text_field(item['columns'][column].data or '') }} - {% endif %} - {% endfor %} - {% if item['columns'].get(None) %} - {% for column in item['columns'][None].data %} - {{ text_field(column, status='default') }} +
+ {% call(item, row_number) list_table( + recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows, + caption=original_file_name, + caption_visible=False, + field_headings=[ + 'Row in file'|safe + ] + recipients.column_headers + ) %} + {% call index_field() %} + + {{ item.index + 2 }} + + {% endcall %} + {% for column in recipients.column_headers %} + {% if item['columns'][column].error and not recipients.missing_column_headers %} + {% call field() %} + + {{ item['columns'][column].error }} + {{ item['columns'][column].data if item['columns'][column].data != None }} + + {% endcall %} + {% elif item['columns'][column].ignore %} + {{ text_field(item['columns'][column].data or '', status='default') }} + {% else %} + {{ text_field(item['columns'][column].data or '') }} + {% endif %} {% endfor %} - {% endif %} - {% endcall %} - - {% endif %} - + {% if item['columns'].get(None) %} + {% for column in item['columns'][None].data %} + {{ text_field(column, status='default') }} + {% endfor %} + {% endif %} + {% endcall %} + {% endif %} +
{% if recipients.too_many_rows %}

{{ original_file_name }}

- {% call(item, row_number) list_table( - recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows, - caption=original_file_name, - caption_visible=False, - field_headings=[ - 'Row in file'|safe - ] + recipients.column_headers - ) %} - {% call index_field() %} - - {{ item.index + 2 }} - - {% endcall %} - {% for column in recipients.column_headers %} - {% if item['columns'][column].ignore %} - {{ text_field(item['columns'][column].data or '', status='default') }} - {% else %} - {{ text_field(item['columns'][column].data or '') }} - {% endif %} - {% endfor %} - {% if item['columns'].get(None) %} - {% for column in item['columns'][None].data %} - {{ text_field(column, status='default') }} +
+ {% call(item, row_number) list_table( + recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows, + caption=original_file_name, + caption_visible=False, + field_headings=[ + 'Row in file'|safe + ] + recipients.column_headers + ) %} + {% call index_field() %} + + {{ item.index + 2 }} + + {% endcall %} + {% for column in recipients.column_headers %} + {% if item['columns'][column].ignore %} + {{ text_field(item['columns'][column].data or '', status='default') }} + {% else %} + {{ text_field(item['columns'][column].data or '') }} + {% endif %} {% endfor %} - {% endif %} - {% endcall %} + {% if item['columns'].get(None) %} + {% for column in item['columns'][None].data %} + {{ text_field(column, status='default') }} + {% endfor %} + {% endif %} + {% endcall %} +
{% endif %} diff --git a/app/templates/views/check/row-errors.html b/app/templates/views/check/row-errors.html index 537710516..e92647886 100644 --- a/app/templates/views/check/row-errors.html +++ b/app/templates/views/check/row-errors.html @@ -19,7 +19,7 @@ {% block maincolumn_content %} -
+
{% call banner_wrapper(type='dangerous') %} {% if row_errors|length == 1 %}

@@ -45,44 +45,48 @@ {% endcall %}

-
- {{ file_upload(form.file, button_text='Re-upload your file') }} +
+
+ {{ file_upload(form.file, button_text='Re-upload your file') }} +
+ Back to top
- {% call(item, row_number) list_table( - recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows, - caption=original_file_name, - caption_visible=False, - field_headings=[ - 'Row in file'|safe - ] + recipients.column_headers - ) %} - {% call index_field() %} - - {{ item.index + 2 }} - - {% endcall %} - {% for column in recipients.column_headers %} - {% if item['columns'][column].error and not recipients.missing_column_headers %} - {% call field() %} - - {{ item['columns'][column].error }} - {{ item['columns'][column].data if item['columns'][column].data != None }} - - {% endcall %} - {% elif item['columns'][column].ignore %} - {{ text_field(item['columns'][column].data or '', status='default') }} - {% else %} - {{ text_field(item['columns'][column].data or '') }} - {% endif %} - {% endfor %} - {% if item['columns'].get(None) %} - {% for column in item['columns'][None].data %} - {{ text_field(column, status='default') }} +
+ {% call(item, row_number) list_table( + recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows, + caption=original_file_name, + caption_visible=False, + field_headings=[ + 'Row in file'|safe + ] + recipients.column_headers + ) %} + {% call index_field() %} + + {{ item.index + 2 }} + + {% endcall %} + {% for column in recipients.column_headers %} + {% if item['columns'][column].error and not recipients.missing_column_headers %} + {% call field() %} + + {{ item['columns'][column].error }} + {{ item['columns'][column].data if item['columns'][column].data != None }} + + {% endcall %} + {% elif item['columns'][column].ignore %} + {{ text_field(item['columns'][column].data or '', status='default') }} + {% else %} + {{ text_field(item['columns'][column].data or '') }} + {% endif %} {% endfor %} - {% endif %} - {% endcall %} - + {% if item['columns'].get(None) %} + {% for column in item['columns'][None].data %} + {{ text_field(column, status='default') }} + {% endfor %} + {% endif %} + {% endcall %} +
{% if count_of_displayed_recipients < count_of_recipients %}
07700900986
', - '
Jo
', + '
07700900001
', + '
A
', ( ' ' '
' @@ -481,6 +533,43 @@ def test_upload_valid_csv_shows_file_contents( assert normalize_spaces(str(page.select('table tbody td')[index])) == cell +@pytest.mark.parametrize('row_index, expected_status', [ + (0, 200), + (2, 200), + (3, 404), +]) +def test_404_for_previewing_a_row_out_of_range( + client_request, + mocker, + mock_get_live_service, + mock_get_service_template_with_placeholders, + mock_get_users_by_service, + mock_get_detailed_service_for_today, + fake_uuid, + row_index, + expected_status, +): + + with client_request.session_transaction() as session: + session['upload_data'] = {'template_id': fake_uuid} + + mocker.patch('app.main.views.send.s3download', return_value=""" + phone number,name,thing,thing,thing + 07700900001, A, foo, foo, foo + 07700900002, B, foo, foo, foo + 07700900003, C, foo, foo, foo + """) + + client_request.get( + 'main.check_messages', + service_id=SERVICE_ONE_ID, + template_type='sms', + upload_id=fake_uuid, + row_index=row_index, + _expected_status=expected_status, + ) + + def test_send_test_doesnt_show_file_contents( logged_in_client, mocker, @@ -1373,6 +1462,20 @@ def test_can_start_letters_job( @pytest.mark.parametrize('filetype', ['pdf', 'png']) +@pytest.mark.parametrize('extra_args, expected_values', [ + ( + {}, + {'postcode': 'abc123', 'addressline1': '123 street'}, + ), + ( + {'row_index': 0}, + {'postcode': 'abc123', 'addressline1': '123 street'}, + ), + ( + {'row_index': 1}, + {'postcode': 'cba321', 'addressline1': '321 avenue'}, + ), +]) def test_should_show_preview_letter_message( filetype, logged_in_platform_admin_client, @@ -1382,6 +1485,8 @@ def test_should_show_preview_letter_message( service_one, fake_uuid, mocker, + extra_args, + expected_values, ): service_one['permissions'] = ['letter'] mocker.patch('app.service_api_client.get_service', return_value={"data": service_one}) @@ -1391,7 +1496,8 @@ def test_should_show_preview_letter_message( 'app.main.views.send.s3download', return_value='\n'.join( ['address line 1, postcode'] + - ['123 street, abc123'] + ['123 street, abc123'] + + ['321 avenue, cba321'] ) ) mocked_preview = mocker.patch( @@ -1414,7 +1520,8 @@ def test_should_show_preview_letter_message( service_id=service_id, template_type='letter', upload_id=fake_uuid, - filetype=filetype + filetype=filetype, + **extra_args ) ) @@ -1425,6 +1532,7 @@ def test_should_show_preview_letter_message( assert mocked_preview.call_args[0][0].id == template_id assert type(mocked_preview.call_args[0][0]) == LetterPreviewTemplate assert mocked_preview.call_args[0][1] == filetype + assert mocked_preview.call_args[0][0].values == expected_values def test_dont_show_preview_letter_templates_for_bad_filetype(