diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index d25160b6d..58b84b442 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -180,30 +180,7 @@ def service_name_change_confirm(service_id): @user_has_permissions('manage_service') def request_to_go_live(service_id): return render_template( - 'views/service-settings/request-to-go-live.html', - has_team_members=( - user_api_client.get_count_of_users_with_permission( - service_id, 'manage_service' - ) > 1 - ), - has_templates=( - service_api_client.count_service_templates(service_id) > 0 - ), - has_email_templates=( - service_api_client.count_service_templates(service_id, template_type='email') > 0 - ), - has_sms_templates=( - service_api_client.count_service_templates(service_id, template_type='sms') > 0 - ), - has_email_reply_to_address=bool( - service_api_client.get_reply_to_email_addresses(service_id) - ), - shouldnt_use_govuk_as_sms_sender=( - current_service.organisation_type in {'local', 'nhs'} - ), - sms_sender_is_govuk=get_default_sms_sender( - service_api_client.get_sms_senders(service_id) - ) in {'GOVUK', 'None'}, + 'views/service-settings/request-to-go-live.html' ) @@ -222,6 +199,7 @@ def submit_request_to_go_live(service_id): '\n---' '\nOrganisation type: {organisation_type}' '\nAgreement signed: {agreement}' + '\nChecklist completed: {checklist}' '\nEmails in next year: {volume_email}' '\nText messages in next year: {volume_sms}' '\nLetters in next year: {volume_letter}' @@ -244,6 +222,7 @@ def submit_request_to_go_live(service_id): service_dashboard=url_for('main.service_dashboard', service_id=current_service.id, _external=True), 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=form.volume_email.data, volume_sms=form.volume_sms.data, volume_letter=form.volume_letter.data, @@ -256,7 +235,8 @@ def submit_request_to_go_live(service_id): ), ticket_type=zendesk_client.TYPE_QUESTION, user_email=current_user.email_address, - user_name=current_user.name + user_name=current_user.name, + tags=get_request_to_go_live_tags(current_service, current_user), ) flash('Thanks for your request to go live. We’ll get back to you within one working day.', 'default') @@ -1042,6 +1022,7 @@ def branding_request(service_id): ticket_type=zendesk_client.TYPE_QUESTION, user_email=current_user.email_address, user_name=current_user.name, + tags=['notify_action_add_branding'], ) flash(( @@ -1127,3 +1108,34 @@ def check_contact_details_type(contact_details): return 'email_address' else: return 'phone_number' + + +def get_request_to_go_live_tags(service, user): + return list(_get_request_to_go_live_tags( + service, + AgreementInfo.from_user(user).agreement_signed, + )) + + +def _get_request_to_go_live_tags(service, agreement_signed): + + BASE = 'notify_request_to_go_live' + COMPLETE = BASE + '_complete' + INCOMPLETE = BASE + '_incomplete' + + yield BASE + + if service.go_live_checklist_completed and agreement_signed: + return COMPLETE + + for test, tag in ( + (True, ''), + (not service.go_live_checklist_completed, '_checklist'), + (not agreement_signed, '_mou'), + (service.needs_to_add_email_reply_to_address, '_email_reply_to'), + (not service.has_team_members, '_team_member'), + (not service.has_templates, '_template_content'), + (service.needs_to_change_sms_sender, '_sms_sender'), + ): + if test: + yield INCOMPLETE + tag diff --git a/app/notify_client/models.py b/app/notify_client/models.py index 03a97851c..356a00395 100644 --- a/app/notify_client/models.py +++ b/app/notify_client/models.py @@ -3,6 +3,8 @@ from itertools import chain from flask import request, session from flask_login import AnonymousUserMixin, UserMixin +from app.utils import get_default_sms_sender + roles = { 'send_messages': ['send_texts', 'send_emails', 'send_letters'], 'manage_templates': ['manage_templates'], @@ -302,7 +304,79 @@ class Service(dict): def has_permission(self, permission): return permission in self.permissions + @property def has_jobs(self): # Can’t import at top-level because app isn’t yet initialised from app import job_api_client return job_api_client.has_jobs(self.id) + + @property + def has_team_members(self): + from app import user_api_client + return user_api_client.get_count_of_users_with_permission( + self.id, 'manage_service' + ) > 1 + + @property + def has_templates(self): + from app import service_api_client + return service_api_client.count_service_templates( + self.id + ) > 0 + + @property + def has_email_templates(self): + from app import service_api_client + return service_api_client.count_service_templates( + self.id, template_type='email' + ) > 0 + + @property + def has_sms_templates(self): + from app import service_api_client + return service_api_client.count_service_templates( + self.id, template_type='sms' + ) > 0 + + @property + def has_email_reply_to_address(self): + from app import service_api_client + return bool(service_api_client.get_reply_to_email_addresses( + self.id + )) + + @property + def needs_to_add_email_reply_to_address(self): + return self.has_email_templates and not self.has_email_reply_to_address + + @property + def shouldnt_use_govuk_as_sms_sender(self): + return self.organisation_type in {'local', 'nhs'} + + @property + def sms_sender_is_govuk(self): + from app import service_api_client + return get_default_sms_sender( + service_api_client.get_sms_senders(self.id) + ) in {'GOVUK', 'None'} + + @property + def needs_to_change_sms_sender(self): + return all(( + self.has_sms_templates, + self.shouldnt_use_govuk_as_sms_sender, + self.sms_sender_is_govuk, + )) + + @property + def go_live_checklist_completed(self): + return all(( + self.has_team_members, + self.has_templates, + not self.needs_to_add_email_reply_to_address, + not self.needs_to_change_sms_sender, + )) + + @property + def go_live_checklist_completed_as_yes_no(self): + return 'Yes' if self.go_live_checklist_completed else 'No' diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 2259a982d..543828537 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -9,7 +9,7 @@
  • Templates
    • {% if not current_user.has_permissions('view_activity') %}
  • Sent messages
    • - {% if current_service.has_jobs() %} + {% if current_service.has_jobs %}
  • Uploaded files
    • {% endif %} {% endif %} diff --git a/app/templates/views/service-settings/request-to-go-live.html b/app/templates/views/service-settings/request-to-go-live.html index 28cf3c503..0b92f1d48 100644 --- a/app/templates/views/service-settings/request-to-go-live.html +++ b/app/templates/views/service-settings/request-to-go-live.html @@ -16,30 +16,30 @@

    Before you request to go live

    {% call task_list_wrapper() %} {{ task_list_item( - has_team_members, + current_service.has_team_members, 'Add a team member who can manage settings, team and usage '.format( url_for('main.manage_users', service_id=current_service.id) )|safe, ) }} {{ task_list_item( - has_templates, + current_service.has_templates, 'Add templates with examples of the content you plan to send '.format( url_for('main.choose_template', service_id=current_service.id) )|safe, ) }} - {% if has_email_templates %} + {% if current_service.has_email_templates %} {{ task_list_item( - has_email_reply_to_address, + current_service.has_email_reply_to_address, 'Add an email reply-to address'.format( url_for('main.service_email_reply_to', service_id=current_service.id) )|safe, ) }} {% endif %} - {% if has_sms_templates and shouldnt_use_govuk_as_sms_sender %} + {% if current_service.has_sms_templates and current_service.shouldnt_use_govuk_as_sms_sender %} {{ task_list_item( - not sms_sender_is_govuk, + not current_service.sms_sender_is_govuk, 'Change your text message sender name'.format( url_for('main.service_sms_senders', service_id=current_service.id) )|safe diff --git a/requirements-app.txt b/requirements-app.txt index 2b33e908e..f3a59c738 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -21,4 +21,4 @@ notifications-python-client==5.1.0 # PaaS awscli-cwlogs>=1.4,<1.5 -git+https://github.com/alphagov/notifications-utils.git@30.1.2#egg=notifications-utils==30.1.2 +git+https://github.com/alphagov/notifications-utils.git@30.3.0#egg=notifications-utils==30.3.0 diff --git a/requirements.txt b/requirements.txt index 224f8f34a..3afa9cc7f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -23,7 +23,7 @@ notifications-python-client==5.1.0 # PaaS awscli-cwlogs>=1.4,<1.5 -git+https://github.com/alphagov/notifications-utils.git@30.1.2#egg=notifications-utils==30.1.2 +git+https://github.com/alphagov/notifications-utils.git@30.3.0#egg=notifications-utils==30.3.0 ## The following requirements were added by pip freeze: awscli==1.16.19 diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index f35ca07ce..9052439ce 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1,6 +1,6 @@ import uuid from functools import partial -from unittest.mock import ANY, call +from unittest.mock import ANY, PropertyMock, call from urllib.parse import parse_qs, urlparse import pytest @@ -737,7 +737,9 @@ def test_should_redirect_after_request_to_go_live( single_letter_contact_block, mock_get_service_organisation, single_sms_sender, - mock_get_service_settings_page_common + mock_get_service_settings_page_common, + mock_get_service_templates, + mock_get_users_by_service, ): mock_post = mocker.patch('app.main.views.service_settings.zendesk_client.create_ticket', autospec=True) page = client_request.post( @@ -756,7 +758,14 @@ def test_should_redirect_after_request_to_go_live( message=ANY, ticket_type=ZendeskClient.TYPE_QUESTION, user_name=active_user_with_permissions.name, - user_email=active_user_with_permissions.email_address + user_email=active_user_with_permissions.email_address, + tags=[ + 'notify_request_to_go_live', + 'notify_request_to_go_live_incomplete', + 'notify_request_to_go_live_incomplete_checklist', + 'notify_request_to_go_live_incomplete_mou', + 'notify_request_to_go_live_incomplete_team_member', + ], ) assert mock_post.call_args[1]['message'] == ( 'Service: service one\n' @@ -765,6 +774,7 @@ def test_should_redirect_after_request_to_go_live( '---\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\n' 'Text messages in next year: 222\n' 'Letters in next year: 333\n' @@ -782,6 +792,177 @@ def test_should_redirect_after_request_to_go_live( ) +@pytest.mark.parametrize( + ( + 'has_team_members,' + 'has_templates,' + 'has_email_templates,' + 'has_sms_templates,' + 'has_email_reply_to_address,' + 'shouldnt_use_govuk_as_sms_sender,' + 'sms_sender_is_govuk,' + 'expected_readyness,' + 'agreement_signed,' + 'expected_tags,' + ), + ( + ( # Just sending email + True, + True, + True, + False, + True, + True, + True, + 'Yes', + True, + [ + 'notify_request_to_go_live', + ], + ), + ( # Needs to set reply to address + True, + True, + True, + False, + False, + True, + True, + 'No', + True, + [ + 'notify_request_to_go_live', + 'notify_request_to_go_live_incomplete', + 'notify_request_to_go_live_incomplete_checklist', + 'notify_request_to_go_live_incomplete_email_reply_to', + ], + ), + ( # Just sending SMS + True, + True, + False, + True, + True, + True, + False, + 'Yes', + True, + [ + 'notify_request_to_go_live', + ], + ), + ( # Needs to change SMS sender + True, + True, + False, + True, + True, + True, + True, + 'No', + True, + [ + 'notify_request_to_go_live', + 'notify_request_to_go_live_incomplete', + 'notify_request_to_go_live_incomplete_checklist', + 'notify_request_to_go_live_incomplete_sms_sender', + ], + ), + ( # Needs team members + False, + True, + False, + True, + True, + True, + False, + 'No', + True, + [ + 'notify_request_to_go_live', + 'notify_request_to_go_live_incomplete', + 'notify_request_to_go_live_incomplete_checklist', + 'notify_request_to_go_live_incomplete_team_member', + ], + ), + ( # Needs templates + True, + False, + False, + True, + True, + True, + False, + 'No', + True, + [ + 'notify_request_to_go_live', + 'notify_request_to_go_live_incomplete', + 'notify_request_to_go_live_incomplete_checklist', + 'notify_request_to_go_live_incomplete_template_content', + ], + ), + ( # Everything is wrong + False, + False, + True, + True, + False, + True, + True, + 'No', + False, + [ + 'notify_request_to_go_live', + 'notify_request_to_go_live_incomplete', + 'notify_request_to_go_live_incomplete_checklist', + 'notify_request_to_go_live_incomplete_mou', + 'notify_request_to_go_live_incomplete_email_reply_to', + 'notify_request_to_go_live_incomplete_team_member', + 'notify_request_to_go_live_incomplete_template_content', + 'notify_request_to_go_live_incomplete_sms_sender', + ], + ), + ), +) +def test_ready_to_go_live( + client_request, + mocker, + has_team_members, + has_templates, + has_email_templates, + has_sms_templates, + has_email_reply_to_address, + shouldnt_use_govuk_as_sms_sender, + sms_sender_is_govuk, + expected_readyness, + agreement_signed, + expected_tags, +): + for prop in { + 'has_team_members', + 'has_templates', + 'has_email_templates', + 'has_sms_templates', + 'has_email_reply_to_address', + 'shouldnt_use_govuk_as_sms_sender', + 'sms_sender_is_govuk', + }: + mocker.patch( + 'app.notify_client.models.Service.{}'.format(prop), + new_callable=PropertyMock + ).return_value = locals()[prop] + + assert app.notify_client.models.Service({ + 'id': fake_uuid() + }).go_live_checklist_completed_as_yes_no == expected_readyness + + assert list(app.main.views.service_settings._get_request_to_go_live_tags( + app.notify_client.models.Service({'id': fake_uuid()}), + agreement_signed, + )) == expected_tags + + @pytest.mark.parametrize('route', [ 'main.service_settings', 'main.service_name_change', @@ -3022,6 +3203,7 @@ def test_submit_email_branding_request( ticket_type='question', user_email='test@user.gov.uk', user_name='Test User', + tags=['notify_action_add_branding'], ) assert normalize_spaces(page.select_one('.banner-default').text) == ( 'Thanks for your branding request. We’ll get back to you '