From 5f4280cf81a7ea2848af3a2e67584f9471ce1d21 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 27 Feb 2019 17:05:02 +0000 Subject: [PATCH] Let people go live without filling the volumes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At the moment it 500s because it can’t format the `None` values as numbers. In the future we will stop people requesting to go live until they’ve provided this info. For now it has to be optional. --- app/main/views/service_settings.py | 23 +++++++--- tests/app/main/views/test_service_settings.py | 45 ++++++++++++++++--- 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 4d0333e51..5f581f5cf 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -201,9 +201,9 @@ def submit_request_to_go_live(service_id): '\nOrganisation type: {organisation_type}' '\nAgreement signed: {agreement}' '\nChecklist completed: {checklist}' - '\nEmails in next year: {volume_email:,.0f}' - '\nText messages in next year: {volume_sms:,.0f}' - '\nLetters in next year: {volume_letter:,.0f}' + '\nEmails in next year: {volume_email_formatted}' + '\nText messages in next year: {volume_sms_formatted}' + '\nLetters in next year: {volume_letter_formatted}' '\nConsent to research: {research_consent}' '\nOther live services: {existing_live}' '\n' @@ -225,9 +225,12 @@ def submit_request_to_go_live(service_id): organisation_type=str(current_service.organisation_type).title(), agreement=AgreementInfo.from_current_user().as_human_readable, checklist=current_service.go_live_checklist_completed_as_yes_no, - volume_email=current_service.volume_email, - volume_sms=current_service.volume_sms, - volume_letter=current_service.volume_letter, + volume_email=print_if_number(current_service.volume_email), + volume_email_formatted=format_if_number(current_service.volume_email), + volume_sms=print_if_number(current_service.volume_sms), + volume_sms_formatted=format_if_number(current_service.volume_sms), + volume_letter=print_if_number(current_service.volume_letter), + volume_letter_formatted=format_if_number(current_service.volume_letter), research_consent='Yes' if current_service.consent_to_research else 'No', existing_live='Yes' if user_api_client.user_has_live_services(current_user) else 'No', service_id=current_service.id, @@ -1070,3 +1073,11 @@ def _get_request_to_go_live_tags(service, agreement_signed): ): if test: yield BASE + '_incomplete' + tag + + +def print_if_number(value): + return value if isinstance(value, int) else '' + + +def format_if_number(value): + return '{:,.0f}'.format(value) if isinstance(value, int) else '' diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 2e85005aa..3974a6bea 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1017,6 +1017,26 @@ def test_non_gov_users_cant_request_to_go_live( ) +@pytest.mark.parametrize('volumes, displayed_volumes, formatted_displayed_volumes', ( + ( + (('email', None), ('sms', None), ('letter', None)), + ', , ', + ( + 'Emails in next year: \n' + 'Text messages in next year: \n' + 'Letters in next year: \n' + ), + ), + ( + (('email', 1234), ('sms', 0), ('letter', 999)), + '0, 1234, 999', # This is a different order to match the spreadsheet + ( + 'Emails in next year: 1,234\n' + 'Text messages in next year: 0\n' + 'Letters in next year: 999\n' + ), + ), +)) @freeze_time("2012-12-21") def test_should_redirect_after_request_to_go_live( client_request, @@ -1030,7 +1050,17 @@ def test_should_redirect_after_request_to_go_live( mock_get_service_settings_page_common, mock_get_service_templates, mock_get_users_by_service, + volumes, + displayed_volumes, + formatted_displayed_volumes, ): + for channel, volume in volumes: + mocker.patch( + 'app.models.service.Service.volume_{}'.format(channel), + create=True, + new_callable=PropertyMock, + return_value=volume, + ) mock_post = mocker.patch('app.main.views.service_settings.zendesk_client.create_ticket', autospec=True) page = client_request.post( 'main.request_to_go_live', @@ -1053,21 +1083,24 @@ def test_should_redirect_after_request_to_go_live( ) assert mock_post.call_args[1]['message'] == ( 'Service: service one\n' - 'http://localhost/services/{}\n' + 'http://localhost/services/{service_id}\n' '\n' '---\n' 'Organisation type: Central\n' 'Agreement signed: Can’t tell (domain is user.gov.uk)\n' 'Checklist completed: No\n' - 'Emails in next year: 111,111\n' - 'Text messages in next year: 222,222\n' - 'Letters in next year: 333,333\n' + '{formatted_displayed_volumes}' 'Consent to research: Yes\n' 'Other live services: No\n' '\n' '---\n' - '{}, None, service one, Test User, test@user.gov.uk, -, 21/12/2012, 222222, 111111, 333333' - ).format(SERVICE_ONE_ID, SERVICE_ONE_ID) + '{service_id}, None, service one, Test User, test@user.gov.uk, -, 21/12/2012, ' + '{displayed_volumes}' + ).format( + service_id=SERVICE_ONE_ID, + displayed_volumes=displayed_volumes, + formatted_displayed_volumes=formatted_displayed_volumes, + ) assert normalize_spaces(page.select_one('.banner-default').text) == ( 'Thanks for your request to go live. We’ll get back to you within one working day.'