Merge pull request #2321 from alphagov/auto-tag-request-to-go-live

Tag request to go live tickets automatically
This commit is contained in:
Chris Hill-Scott
2018-09-24 17:13:52 +01:00
committed by GitHub
7 changed files with 305 additions and 37 deletions

View File

@@ -180,30 +180,7 @@ def service_name_change_confirm(service_id):
@user_has_permissions('manage_service') @user_has_permissions('manage_service')
def request_to_go_live(service_id): def request_to_go_live(service_id):
return render_template( return render_template(
'views/service-settings/request-to-go-live.html', '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'},
) )
@@ -222,6 +199,7 @@ def submit_request_to_go_live(service_id):
'\n---' '\n---'
'\nOrganisation type: {organisation_type}' '\nOrganisation type: {organisation_type}'
'\nAgreement signed: {agreement}' '\nAgreement signed: {agreement}'
'\nChecklist completed: {checklist}'
'\nEmails in next year: {volume_email}' '\nEmails in next year: {volume_email}'
'\nText messages in next year: {volume_sms}' '\nText messages in next year: {volume_sms}'
'\nLetters in next year: {volume_letter}' '\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), service_dashboard=url_for('main.service_dashboard', service_id=current_service.id, _external=True),
organisation_type=str(current_service.organisation_type).title(), organisation_type=str(current_service.organisation_type).title(),
agreement=AgreementInfo.from_current_user().as_human_readable, 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_email=form.volume_email.data,
volume_sms=form.volume_sms.data, volume_sms=form.volume_sms.data,
volume_letter=form.volume_letter.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, ticket_type=zendesk_client.TYPE_QUESTION,
user_email=current_user.email_address, 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. Well get back to you within one working day.', 'default') flash('Thanks for your request to go live. Well get back to you within one working day.', 'default')
@@ -1042,6 +1022,7 @@ def branding_request(service_id):
ticket_type=zendesk_client.TYPE_QUESTION, ticket_type=zendesk_client.TYPE_QUESTION,
user_email=current_user.email_address, user_email=current_user.email_address,
user_name=current_user.name, user_name=current_user.name,
tags=['notify_action_add_branding'],
) )
flash(( flash((
@@ -1127,3 +1108,34 @@ def check_contact_details_type(contact_details):
return 'email_address' return 'email_address'
else: else:
return 'phone_number' 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

View File

@@ -3,6 +3,8 @@ from itertools import chain
from flask import request, session from flask import request, session
from flask_login import AnonymousUserMixin, UserMixin from flask_login import AnonymousUserMixin, UserMixin
from app.utils import get_default_sms_sender
roles = { roles = {
'send_messages': ['send_texts', 'send_emails', 'send_letters'], 'send_messages': ['send_texts', 'send_emails', 'send_letters'],
'manage_templates': ['manage_templates'], 'manage_templates': ['manage_templates'],
@@ -302,7 +304,79 @@ class Service(dict):
def has_permission(self, permission): def has_permission(self, permission):
return permission in self.permissions return permission in self.permissions
@property
def has_jobs(self): def has_jobs(self):
# Cant import at top-level because app isnt yet initialised # Cant import at top-level because app isnt yet initialised
from app import job_api_client from app import job_api_client
return job_api_client.has_jobs(self.id) 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'

View File

@@ -9,7 +9,7 @@
<li><a href="{{ url_for('.choose_template', service_id=current_service.id) }}" {{ main_navigation.is_selected('templates') }}>Templates</a></li> <li><a href="{{ url_for('.choose_template', service_id=current_service.id) }}" {{ main_navigation.is_selected('templates') }}>Templates</a></li>
{% if not current_user.has_permissions('view_activity') %} {% if not current_user.has_permissions('view_activity') %}
<li><a href="{{ url_for('.view_notifications', service_id=current_service.id, status='sending,delivered,failed') }}" {{ casework_navigation.is_selected('sent-messages') }}>Sent messages</a></li> <li><a href="{{ url_for('.view_notifications', service_id=current_service.id, status='sending,delivered,failed') }}" {{ casework_navigation.is_selected('sent-messages') }}>Sent messages</a></li>
{% if current_service.has_jobs() %} {% if current_service.has_jobs %}
<li><a href="{{ url_for('.view_jobs', service_id=current_service.id) }}" {{ casework_navigation.is_selected('uploaded-files') }}>Uploaded files</a></li> <li><a href="{{ url_for('.view_jobs', service_id=current_service.id) }}" {{ casework_navigation.is_selected('uploaded-files') }}>Uploaded files</a></li>
{% endif %} {% endif %}
{% endif %} {% endif %}

View File

@@ -16,30 +16,30 @@
<h1 class="heading-large">Before you request to go live</h1> <h1 class="heading-large">Before you request to go live</h1>
{% call task_list_wrapper() %} {% call task_list_wrapper() %}
{{ task_list_item( {{ task_list_item(
has_team_members, current_service.has_team_members,
'<a href="{}">Add a team member who can manage settings, team and usage</a> '<a href="{}">Add a team member who can manage settings, team and usage</a>
'.format( '.format(
url_for('main.manage_users', service_id=current_service.id) url_for('main.manage_users', service_id=current_service.id)
)|safe, )|safe,
) }} ) }}
{{ task_list_item( {{ task_list_item(
has_templates, current_service.has_templates,
'<a href="{}">Add templates with examples of the content you plan to send '<a href="{}">Add templates with examples of the content you plan to send
</a>'.format( </a>'.format(
url_for('main.choose_template', service_id=current_service.id) url_for('main.choose_template', service_id=current_service.id)
)|safe, )|safe,
) }} ) }}
{% if has_email_templates %} {% if current_service.has_email_templates %}
{{ task_list_item( {{ task_list_item(
has_email_reply_to_address, current_service.has_email_reply_to_address,
'<a href="{}">Add an email reply-to address</a>'.format( '<a href="{}">Add an email reply-to address</a>'.format(
url_for('main.service_email_reply_to', service_id=current_service.id) url_for('main.service_email_reply_to', service_id=current_service.id)
)|safe, )|safe,
) }} ) }}
{% endif %} {% 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( {{ task_list_item(
not sms_sender_is_govuk, not current_service.sms_sender_is_govuk,
'<a href="{}">Change your text message sender name</a>'.format( '<a href="{}">Change your text message sender name</a>'.format(
url_for('main.service_sms_senders', service_id=current_service.id) url_for('main.service_sms_senders', service_id=current_service.id)
)|safe )|safe

View File

@@ -21,4 +21,4 @@ notifications-python-client==5.1.0
# PaaS # PaaS
awscli-cwlogs>=1.4,<1.5 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

View File

@@ -23,7 +23,7 @@ notifications-python-client==5.1.0
# PaaS # PaaS
awscli-cwlogs>=1.4,<1.5 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: ## The following requirements were added by pip freeze:
awscli==1.16.19 awscli==1.16.19

View File

@@ -1,6 +1,6 @@
import uuid import uuid
from functools import partial from functools import partial
from unittest.mock import ANY, call from unittest.mock import ANY, PropertyMock, call
from urllib.parse import parse_qs, urlparse from urllib.parse import parse_qs, urlparse
import pytest import pytest
@@ -737,7 +737,9 @@ def test_should_redirect_after_request_to_go_live(
single_letter_contact_block, single_letter_contact_block,
mock_get_service_organisation, mock_get_service_organisation,
single_sms_sender, 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) mock_post = mocker.patch('app.main.views.service_settings.zendesk_client.create_ticket', autospec=True)
page = client_request.post( page = client_request.post(
@@ -756,7 +758,14 @@ def test_should_redirect_after_request_to_go_live(
message=ANY, message=ANY,
ticket_type=ZendeskClient.TYPE_QUESTION, ticket_type=ZendeskClient.TYPE_QUESTION,
user_name=active_user_with_permissions.name, 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'] == ( assert mock_post.call_args[1]['message'] == (
'Service: service one\n' 'Service: service one\n'
@@ -765,6 +774,7 @@ def test_should_redirect_after_request_to_go_live(
'---\n' '---\n'
'Organisation type: Central\n' 'Organisation type: Central\n'
'Agreement signed: Cant tell (domain is user.gov.uk)\n' 'Agreement signed: Cant tell (domain is user.gov.uk)\n'
'Checklist completed: No\n'
'Emails in next year: 111\n' 'Emails in next year: 111\n'
'Text messages in next year: 222\n' 'Text messages in next year: 222\n'
'Letters in next year: 333\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', [ @pytest.mark.parametrize('route', [
'main.service_settings', 'main.service_settings',
'main.service_name_change', 'main.service_name_change',
@@ -3022,6 +3203,7 @@ def test_submit_email_branding_request(
ticket_type='question', ticket_type='question',
user_email='test@user.gov.uk', user_email='test@user.gov.uk',
user_name='Test User', user_name='Test User',
tags=['notify_action_add_branding'],
) )
assert normalize_spaces(page.select_one('.banner-default').text) == ( assert normalize_spaces(page.select_one('.banner-default').text) == (
'Thanks for your branding request. Well get back to you ' 'Thanks for your branding request. Well get back to you '