mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-06-21 21:53:42 -04:00
Don’t show postage choice for international letters
International letters don’t have a choice of postage. Under the hood they are either `europe` or `rest-of-world`. So, for letters that we detect are international, this commit: - removes the radios buttons that give users the choice of postage - passes through either `europe` or `rest-of-world` to the API, depending on what address we find in the letter This will cause the API to 500 until it can accept `europe` or `rest-of-world` as postage types, but this is probably OK because it’s only our services that have international letters switched on at the moment.
This commit is contained in:
@@ -9,6 +9,7 @@ from flask_wtf import FlaskForm as Form
|
||||
from flask_wtf.file import FileAllowed
|
||||
from flask_wtf.file import FileField as FileField_wtf
|
||||
from notifications_utils.columns import Columns
|
||||
from notifications_utils.countries.data import Postage
|
||||
from notifications_utils.formatters import strip_whitespace
|
||||
from notifications_utils.postal_address import PostalAddress
|
||||
from notifications_utils.recipients import (
|
||||
@@ -850,6 +851,19 @@ class LetterTemplatePostageForm(StripWhitespaceForm):
|
||||
|
||||
|
||||
class LetterUploadPostageForm(StripWhitespaceForm):
|
||||
|
||||
def __init__(self, *args, postage_zone, **kwargs):
|
||||
|
||||
super().__init__(*args, **kwargs)
|
||||
|
||||
if postage_zone != Postage.UK:
|
||||
self.postage.choices = [(postage_zone, '')]
|
||||
self.postage.data = postage_zone
|
||||
|
||||
@property
|
||||
def show_postage(self):
|
||||
return len(self.postage.choices) > 1
|
||||
|
||||
postage = RadioField(
|
||||
'Choose the postage for this letter',
|
||||
choices=[
|
||||
|
||||
@@ -19,6 +19,7 @@ from flask import (
|
||||
)
|
||||
from notifications_utils.columns import Columns
|
||||
from notifications_utils.pdf import pdf_page_count
|
||||
from notifications_utils.postal_address import PostalAddress
|
||||
from notifications_utils.recipients import RecipientCSV
|
||||
from notifications_utils.sanitise_text import SanitiseASCII
|
||||
from PyPDF2.utils import PdfReadError
|
||||
@@ -279,7 +280,7 @@ def uploaded_letter_preview(service_id, file_id):
|
||||
status = metadata.get('status')
|
||||
error_shortcode = metadata.get('message')
|
||||
invalid_pages = metadata.get('invalid_pages')
|
||||
recipient = format_recipient(metadata.get('recipient', ''))
|
||||
postal_address = PostalAddress(metadata.get('recipient', ''))
|
||||
|
||||
if invalid_pages:
|
||||
invalid_pages = json.loads(invalid_pages)
|
||||
@@ -291,7 +292,9 @@ def uploaded_letter_preview(service_id, file_id):
|
||||
# a non null value of postage for letter templates
|
||||
template_dict['postage'] = None
|
||||
|
||||
form = LetterUploadPostageForm()
|
||||
form = LetterUploadPostageForm(
|
||||
postage_zone=postal_address.postage
|
||||
)
|
||||
|
||||
template = get_template(
|
||||
template_dict,
|
||||
@@ -313,7 +316,8 @@ def uploaded_letter_preview(service_id, file_id):
|
||||
message=error_message,
|
||||
error_code=error_shortcode,
|
||||
form=form,
|
||||
recipient=recipient,
|
||||
recipient=format_recipient(postal_address.raw_address),
|
||||
international=postal_address.international,
|
||||
re_upload_form=re_upload_form
|
||||
)
|
||||
|
||||
@@ -349,9 +353,11 @@ def send_uploaded_letter(service_id, file_id=None):
|
||||
if metadata.get('status') != 'valid':
|
||||
abort(403)
|
||||
|
||||
recipient_address = metadata.get('recipient')
|
||||
postal_address = PostalAddress(metadata.get('recipient'))
|
||||
|
||||
form = LetterUploadPostageForm()
|
||||
form = LetterUploadPostageForm(
|
||||
postage_zone=postal_address.postage
|
||||
)
|
||||
|
||||
if not form.validate_on_submit():
|
||||
return uploaded_letter_preview(service_id, file_id)
|
||||
@@ -361,7 +367,7 @@ def send_uploaded_letter(service_id, file_id=None):
|
||||
metadata.get('filename'),
|
||||
file_id,
|
||||
form.postage.data,
|
||||
recipient_address,
|
||||
postal_address.raw_address,
|
||||
)
|
||||
|
||||
return redirect(url_for(
|
||||
|
||||
@@ -52,12 +52,19 @@
|
||||
</p>
|
||||
|
||||
{% if current_service.live %}
|
||||
{% if international %}
|
||||
<p class="govuk-body">
|
||||
Postage: international
|
||||
</p>
|
||||
{% endif %}
|
||||
<form method="post" enctype="multipart/form-data" action="{{url_for(
|
||||
'main.send_uploaded_letter',
|
||||
service_id=current_service.id,
|
||||
file_id=file_id,
|
||||
)}}" class='page-footer'>
|
||||
{{ radios(form.postage, hide_legend=true, inline=True) }}
|
||||
{% if form.show_postage %}
|
||||
{{ radios(form.postage, hide_legend=true, inline=True) }}
|
||||
{% endif %}
|
||||
{{ page_footer("Send 1 letter") }}
|
||||
</form>
|
||||
{% endif %}
|
||||
|
||||
@@ -435,6 +435,64 @@ def test_post_upload_letter_shows_letter_preview_for_valid_file(
|
||||
page=page_no)
|
||||
|
||||
|
||||
def test_upload_international_letter_shows_preview_with_no_choice_of_postage(
|
||||
mocker,
|
||||
active_user_with_permissions,
|
||||
service_one,
|
||||
client_request,
|
||||
fake_uuid,
|
||||
):
|
||||
letter_template = {
|
||||
'template_type': 'letter',
|
||||
'reply_to_text': '',
|
||||
'postage': 'second',
|
||||
'subject': 'hi',
|
||||
'content': 'my letter',
|
||||
}
|
||||
|
||||
mocker.patch('uuid.uuid4', return_value=fake_uuid)
|
||||
mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True)
|
||||
mocker.patch('app.main.views.uploads.sanitise_letter', return_value=Mock(
|
||||
content='The sanitised content',
|
||||
json=lambda: {'file': 'VGhlIHNhbml0aXNlZCBjb250ZW50', 'recipient_address': 'The Queen'}
|
||||
))
|
||||
mocker.patch('app.main.views.uploads.upload_letter_to_s3')
|
||||
mocker.patch('app.main.views.uploads.pdf_page_count', return_value=3)
|
||||
mocker.patch('app.main.views.uploads.get_letter_metadata', return_value=LetterMetadata({
|
||||
'filename': 'tests/test_pdf_files/one_page_pdf.pdf',
|
||||
'page_count': '3',
|
||||
'status': 'valid',
|
||||
'recipient': (
|
||||
'123 Example Street\n'
|
||||
'Andorra la Vella\n'
|
||||
'Andorra'
|
||||
),
|
||||
}))
|
||||
mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template', return_value=letter_template)
|
||||
|
||||
service_one['restricted'] = False
|
||||
client_request.login(active_user_with_permissions, service=service_one)
|
||||
|
||||
with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file:
|
||||
page = client_request.post(
|
||||
'main.upload_letter',
|
||||
service_id=SERVICE_ONE_ID,
|
||||
_data={'file': file},
|
||||
_follow_redirects=True,
|
||||
)
|
||||
|
||||
assert page.find('h1').text == 'tests/test_pdf_files/one_page_pdf.pdf'
|
||||
assert not page.select('.letter-postage')
|
||||
assert not page.select('input[type=radio]')
|
||||
assert normalize_spaces(
|
||||
page.select_one('.js-stick-at-bottom-when-scrolling').text
|
||||
) == (
|
||||
'Recipient: 123 Example Street, Andorra la Vella, Andorra '
|
||||
'Postage: international '
|
||||
'Send 1 letter'
|
||||
)
|
||||
|
||||
|
||||
def test_post_upload_letter_shows_error_when_file_is_not_a_pdf(client_request):
|
||||
with open('tests/non_spreadsheet_files/actually_a_png.csv', 'rb') as file:
|
||||
page = client_request.post(
|
||||
@@ -766,13 +824,48 @@ def test_uploaded_letter_preview_image_400s_for_bad_page_type(
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize('address, post_data, expected_postage', (
|
||||
(
|
||||
'address',
|
||||
{'filename': 'my_file.pdf', 'postage': 'first'},
|
||||
'first',
|
||||
),
|
||||
(
|
||||
'address',
|
||||
{'filename': 'my_file.pdf'},
|
||||
'second',
|
||||
),
|
||||
(
|
||||
'123 Example Street\nLiechtenstein',
|
||||
{'filename': 'my_file.pdf', 'postage': 'first'},
|
||||
'europe',
|
||||
),
|
||||
(
|
||||
'123 Example Street\nLiechtenstein',
|
||||
{'filename': 'my_file.pdf'},
|
||||
'europe',
|
||||
),
|
||||
(
|
||||
'123 Example Street\nLesotho',
|
||||
{'filename': 'my_file.pdf'},
|
||||
'rest-of-world',
|
||||
),
|
||||
))
|
||||
def test_send_uploaded_letter_sends_letter_and_redirects_to_notification_page(
|
||||
mocker,
|
||||
service_one,
|
||||
client_request,
|
||||
fake_uuid,
|
||||
address,
|
||||
post_data,
|
||||
expected_postage,
|
||||
):
|
||||
metadata = LetterMetadata({'filename': 'my_file.pdf', 'page_count': '1', 'status': 'valid', 'recipient': 'address'})
|
||||
metadata = LetterMetadata({
|
||||
'filename': 'my_file.pdf',
|
||||
'page_count': '1',
|
||||
'status': 'valid',
|
||||
'recipient': address,
|
||||
})
|
||||
|
||||
mocker.patch('app.main.views.uploads.get_letter_pdf_and_metadata', return_value=('file', metadata))
|
||||
mock_send = mocker.patch('app.main.views.uploads.notification_api_client.send_precompiled_letter')
|
||||
@@ -784,7 +877,7 @@ def test_send_uploaded_letter_sends_letter_and_redirects_to_notification_page(
|
||||
'main.send_uploaded_letter',
|
||||
service_id=SERVICE_ONE_ID,
|
||||
file_id=fake_uuid,
|
||||
_data={'filename': 'my_file.pdf', 'postage': 'first'},
|
||||
_data=post_data,
|
||||
_expected_redirect=url_for(
|
||||
'main.view_notification',
|
||||
service_id=SERVICE_ONE_ID,
|
||||
@@ -792,7 +885,13 @@ def test_send_uploaded_letter_sends_letter_and_redirects_to_notification_page(
|
||||
_external=True
|
||||
)
|
||||
)
|
||||
mock_send.assert_called_once_with(SERVICE_ONE_ID, 'my_file.pdf', fake_uuid, 'first', 'address')
|
||||
mock_send.assert_called_once_with(
|
||||
SERVICE_ONE_ID,
|
||||
'my_file.pdf',
|
||||
fake_uuid,
|
||||
expected_postage,
|
||||
address,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize('permissions', [
|
||||
|
||||
Reference in New Issue
Block a user