diff --git a/app/assets/stylesheets/components/radios.scss b/app/assets/stylesheets/components/radios.scss index ca6b343dc..0b755e10f 100644 --- a/app/assets/stylesheets/components/radios.scss +++ b/app/assets/stylesheets/components/radios.scss @@ -111,3 +111,9 @@ } } + +.inline { + .multiple-choice { + margin-right: 15px; + } +} diff --git a/app/main/forms.py b/app/main/forms.py index 2945f0347..d5c6acdce 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -765,6 +765,21 @@ class LetterTemplatePostageForm(StripWhitespaceForm): ) +class LetterUploadPostageForm(StripWhitespaceForm): + postage = RadioField( + 'Choose the postage for this letter', + choices=[ + ('first', 'First class post'), + ('second', 'Second class post'), + ], + default='second', + validators=[DataRequired()] + ) + file_id = HiddenField( + validators=[DataRequired()] + ) + + class ForgotPasswordForm(StripWhitespaceForm): email_address = email_address(gov_user=False) diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index 665fce5a5..2ed2ae114 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -18,7 +18,7 @@ from requests import RequestException from app import current_service, notification_api_client, service_api_client from app.extensions import antivirus_client from app.main import main -from app.main.forms import PDFUploadForm +from app.main.forms import LetterUploadPostageForm, PDFUploadForm from app.main.views.jobs import view_jobs from app.s3_client.s3_letter_upload_client import ( get_letter_metadata, @@ -153,6 +153,12 @@ def uploaded_letter_preview(service_id, file_id): error = get_letter_validation_error(error_message, invalid_pages, page_count) template_dict = service_api_client.get_precompiled_template(service_id) + # Override pre compiled letter template postage to none as it has not yet been picked even though + # the pre compiled letter template has its postage set as second class as the DB currently requires + # a non null value of postage for letter templates + template_dict['postage'] = None + + form = LetterUploadPostageForm() template = get_template( template_dict, @@ -172,6 +178,7 @@ def uploaded_letter_preview(service_id, file_id): status=status, file_id=file_id, error=error, + form=form, ) @@ -194,14 +201,20 @@ def send_uploaded_letter(service_id): if not (current_service.has_permission('letter') and current_service.has_permission('upload_letters')): abort(403) - file_id = request.form['file_id'] + form = LetterUploadPostageForm() + file_id = form.file_id.data + + if not form.validate_on_submit(): + return uploaded_letter_preview(service_id, file_id) + + postage = form.postage.data metadata = get_letter_metadata(service_id, file_id) filename = metadata.get('filename') if metadata.get('status') != 'valid': abort(403) - notification_api_client.send_precompiled_letter(service_id, filename, file_id) + notification_api_client.send_precompiled_letter(service_id, filename, file_id, postage) return redirect(url_for( '.view_notification', diff --git a/app/notify_client/notification_api_client.py b/app/notify_client/notification_api_client.py index 793af02ef..0ce251f6b 100644 --- a/app/notify_client/notification_api_client.py +++ b/app/notify_client/notification_api_client.py @@ -59,11 +59,11 @@ class NotificationApiClient(NotifyAdminAPIClient): data = _attach_current_user(data) return self.post(url='/service/{}/send-notification'.format(service_id), data=data) - def send_precompiled_letter(self, service_id, filename, file_id): + def send_precompiled_letter(self, service_id, filename, file_id, postage): data = { 'filename': filename, 'file_id': file_id, - 'postage': 'second', + 'postage': postage, } data = _attach_current_user(data) return self.post(url='/service/{}/send-pdf-letter'.format(service_id), data=data) diff --git a/app/templates/components/radios.html b/app/templates/components/radios.html index dd56baba0..e41e57af4 100644 --- a/app/templates/components/radios.html +++ b/app/templates/components/radios.html @@ -1,7 +1,7 @@ {% from "components/select-input.html" import select, select_list, select_nested, select_wrapper, select_input %} -{% macro radios(field, hint=None, disable=[], option_hints={}, hide_legend=False) %} - {{ select(field, hint, disable, option_hints, hide_legend, input="radio") }} +{% macro radios(field, hint=None, disable=[], option_hints={}, hide_legend=False, inline=False) %} + {{ select(field, hint, disable, option_hints, hide_legend, input="radio", inline=inline) }} {% endmacro %} diff --git a/app/templates/components/select-input.html b/app/templates/components/select-input.html index f65647647..f8387c68e 100644 --- a/app/templates/components/select-input.html +++ b/app/templates/components/select-input.html @@ -1,6 +1,6 @@ -{% macro select(field, hint=None, disable=[], option_hints={}, hide_legend=False, collapsible_opts={}, legend_style="text", input="radio") %} +{% macro select(field, hint=None, disable=[], option_hints={}, hide_legend=False, collapsible_opts={}, legend_style="text", input="radio", inline=False) %} {% call select_wrapper( - field, hint, disable, option_hints, hide_legend, collapsible_opts, legend_style + field, hint, disable, option_hints, hide_legend, collapsible_opts, legend_style, inline=inline ) %} {% for option in field %} {{ select_input(option, disable, option_hints, input=input) }} @@ -35,13 +35,13 @@ {% endmacro %} -{% macro select_wrapper(field, hint=None, disable=[], option_hints={}, hide_legend=False, collapsible_opts={}, legend_style="text") %} +{% macro select_wrapper(field, hint=None, disable=[], option_hints={}, hide_legend=False, collapsible_opts={}, legend_style="text", inline=False) %} {% set is_collapsible = collapsible_opts|length %}
{% if is_collapsible %}
{% endif %} -
+
{% if hide_legend %}{% endif %} {{ field.label.text|safe }} diff --git a/app/templates/views/uploads/preview.html b/app/templates/views/uploads/preview.html index c448b3b4d..f088cee0f 100644 --- a/app/templates/views/uploads/preview.html +++ b/app/templates/views/uploads/preview.html @@ -1,6 +1,8 @@ {% extends "withnav_template.html" %} {% from "components/banner.html" import banner_wrapper %} {% from "components/page-header.html" import page_header %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/radios.html" import radios %} {% block service_page_title %} {{ original_filename }} @@ -39,9 +41,9 @@ 'main.send_uploaded_letter', service_id=current_service.id, )}}" class='page-footer'> - - - + {{ radios(form.postage, hide_legend=true, inline=True) }} + {{ form.file_id(value=file_id) }} + {{ page_footer("Send 1 letter") }}
{% endif %} diff --git a/requirements-app.txt b/requirements-app.txt index 55df8fd15..b6ecda39b 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -23,5 +23,5 @@ awscli-cwlogs>=1.4,<1.5 # Putting upgrade on hold due to v1.0.0 using sha512 instead of sha1 by default itsdangerous==0.24 # pyup: <1.0.0 -git+https://github.com/alphagov/notifications-utils.git@35.0.0#egg=notifications-utils==35.0.0 +git+https://github.com/alphagov/notifications-utils.git@36.0.0#egg=notifications-utils==36.0.0 git+https://github.com/alphagov/govuk-frontend-jinja.git@v0.3.0-alpha#egg=govuk-frontend-jinja==0.3.0-alpha diff --git a/requirements.txt b/requirements.txt index b05f94dc2..8418aa9e4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -25,14 +25,14 @@ awscli-cwlogs>=1.4,<1.5 # Putting upgrade on hold due to v1.0.0 using sha512 instead of sha1 by default itsdangerous==0.24 # pyup: <1.0.0 -git+https://github.com/alphagov/notifications-utils.git@35.0.0#egg=notifications-utils==35.0.0 +git+https://github.com/alphagov/notifications-utils.git@36.0.0#egg=notifications-utils==36.0.0 git+https://github.com/alphagov/govuk-frontend-jinja.git@v0.3.0-alpha#egg=govuk-frontend-jinja==0.3.0-alpha ## The following requirements were added by pip freeze: -awscli==1.16.255 +awscli==1.16.266 bleach==3.1.0 boto3==1.9.221 -botocore==1.12.245 +botocore==1.13.2 certifi==2019.9.11 chardet==3.0.4 Click==7.0 @@ -42,7 +42,7 @@ docopt==0.6.2 docutils==0.15.2 et-xmlfile==1.0.1 flask-redis==0.4.0 -future==0.17.1 +future==0.18.1 greenlet==0.4.15 idna==2.8 jdcal==1.4.1 @@ -63,7 +63,7 @@ PyPDF2==1.26.0 python-dateutil==2.8.0 python-json-logger==0.1.11 PyYAML==5.1.2 -redis==3.3.8 +redis==3.3.11 requests==2.22.0 rsa==3.4.2 s3transfer==0.2.1 diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index a45daaafc..23d481f85 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -125,9 +125,10 @@ def test_post_upload_letter_shows_letter_preview_for_valid_file( ) assert page.find('h1').text == 'tests/test_pdf_files/one_page_pdf.pdf' - assert len(page.select('.letter-postage')) == 1 - assert normalize_spaces(page.select_one('.letter-postage').text) == ('Postage: second class') - assert page.select_one('.letter-postage')['class'] == ['letter-postage', 'letter-postage-second'] + assert len(page.select('.letter-postage')) == 0 + # Check postage radios exists and second class is checked by default + assert page.find('input', id="postage-0", value="first") + assert page.find('input', id="postage-1", value="second").has_attr('checked') letter_images = page.select('main img') assert len(letter_images) == 3 @@ -283,9 +284,7 @@ def test_post_upload_letter_shows_letter_preview_for_invalid_file(mocker, client _follow_redirects=True, ) - assert len(page.select('.letter-postage')) == 1 - assert normalize_spaces(page.select_one('.letter-postage').text) == ('Postage: first class') - assert page.select_one('.letter-postage')['class'] == ['letter-postage', 'letter-postage-first'] + assert len(page.select('.letter-postage')) == 0 letter_images = page.select('main img') assert len(letter_images) == 1 @@ -421,7 +420,7 @@ def test_send_uploaded_letter_sends_letter_and_redirects_to_notification_page(mo client_request.post( 'main.send_uploaded_letter', service_id=SERVICE_ONE_ID, - _data={'filename': 'my_file.pdf', 'file_id': file_id}, + _data={'filename': 'my_file.pdf', 'file_id': file_id, 'postage': 'first'}, _expected_redirect=url_for( 'main.view_notification', service_id=SERVICE_ONE_ID, @@ -429,7 +428,7 @@ def test_send_uploaded_letter_sends_letter_and_redirects_to_notification_page(mo _external=True ) ) - mock_send.assert_called_once_with(SERVICE_ONE_ID, 'my_file.pdf', file_id) + mock_send.assert_called_once_with(SERVICE_ONE_ID, 'my_file.pdf', file_id, 'first') @pytest.mark.parametrize('permissions', [ @@ -452,7 +451,7 @@ def test_send_uploaded_letter_when_service_does_not_have_correct_permissions( client_request.post( 'main.send_uploaded_letter', service_id=SERVICE_ONE_ID, - _data={'filename': 'my_file.pdf', 'file_id': file_id}, + _data={'filename': 'my_file.pdf', 'file_id': file_id, 'postage': 'first'}, _expected_status=403 ) assert not mock_send.called diff --git a/tests/app/notify_client/test_notification_client.py b/tests/app/notify_client/test_notification_client.py index eee76a981..5df1a97b1 100644 --- a/tests/app/notify_client/test_notification_client.py +++ b/tests/app/notify_client/test_notification_client.py @@ -64,7 +64,8 @@ def test_send_precompiled_letter(mocker, logged_in_client, active_user_with_perm NotificationApiClient().send_precompiled_letter( 'abcd-1234', 'my_file.pdf', - 'file-ID' + 'file-ID', + 'second' ) mock_post.assert_called_once_with( url='/service/abcd-1234/send-pdf-letter',