From 39c26f5bfbfd8977f21266df72b40da7aa413e16 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Thu, 16 Sep 2021 11:17:45 +0100 Subject: [PATCH] Create go-live support tickets using the new way The new way of creating support tickets can be seen in [notifications-utils](https://github.com/alphagov/notifications-utils/pull/899). This changes tickets created when making a request to go live to use the new way, while other tickets stay the same for now. The go live tags have been removed. Some of these had become unneccessary since you can't make the request to go live unless they are true (e.g. `notify_go_live_email_reply_to`). Others will always get added by a Zendesk macro when the ticket is replied to, so we don't need to add them here. --- app/main/views/service_settings.py | 23 ++-- app/models/service.py | 27 ---- tests/app/main/views/test_service_settings.py | 128 +++++++----------- 3 files changed, 67 insertions(+), 111 deletions(-) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 34bd5dd2f..a90fbb9f3 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -14,6 +14,9 @@ from flask import ( ) from flask_login import current_user from notifications_python_client.errors import HTTPError +from notifications_utils.clients.zendesk.zendesk_client import ( + NotifySupportTicket, +) from notifications_utils.timezones import utc_string_to_aware_gmt_datetime from app import ( @@ -207,10 +210,7 @@ def request_to_go_live(service_id): @user_has_permissions('manage_service') @user_is_gov_user def submit_request_to_go_live(service_id): - - zendesk_client.create_ticket( - subject='Request to go live - {}'.format(current_service.name), - message=( + ticket_message = ( 'Service: {service_name}\n' '{service_dashboard}\n' '\n---' @@ -243,13 +243,20 @@ def submit_request_to_go_live(service_id): existing_live='Yes' if current_user.live_services else 'No', email_address=current_user.email_address, email_reply_to=current_service.default_email_reply_to_address or 'not set', - ), - ticket_type=zendesk_client.TYPE_QUESTION, - user_email=current_user.email_address, + ) + + ticket = NotifySupportTicket( + subject=f'Request to go live - {current_service.name}', + message=ticket_message, + ticket_type=NotifySupportTicket.TYPE_QUESTION, user_name=current_user.name, - tags=current_service.request_to_go_live_tags, + user_email=current_user.email_address, requester_sees_message_content=False, + org_id=current_service.organisation_id, + org_type=current_service.organisation_type, + service_id=current_service.id, ) + zendesk_client.send_ticket_to_zendesk(ticket) current_service.update(go_live_user=current_user.id) diff --git a/app/models/service.py b/app/models/service.py index c37b19aec..b6012c5ad 100644 --- a/app/models/service.py +++ b/app/models/service.py @@ -684,33 +684,6 @@ class Service(JSONModel): } ) - @property - def request_to_go_live_tags(self): - return list(self._get_request_to_go_live_tags()) - - def _get_request_to_go_live_tags(self): - - BASE = 'notify_go_live' - - yield 'notify_action' - yield BASE - - if self.go_live_checklist_completed and self.organisation.agreement_signed: - yield BASE + '_complete' - return - - for test, tag in ( - (not self.volumes, '_volumes'), - (not self.go_live_checklist_completed, '_checklist'), - (not self.organisation.agreement_signed, '_mou'), - (self.needs_to_add_email_reply_to_address, '_email_reply_to'), - (not self.has_team_members, '_team_member'), - (not self.has_templates, '_template_content'), - (self.needs_to_change_sms_sender, '_sms_sender'), - ): - if test: - yield BASE + '_incomplete' + tag - @cached_property def returned_letter_statistics(self): return service_api_client.get_returned_letter_statistics(self.id) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 12b3f2917..2b9e7e65f 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1,7 +1,7 @@ import re from datetime import datetime from functools import partial -from unittest.mock import ANY, Mock, PropertyMock, call +from unittest.mock import Mock, PropertyMock, call from urllib.parse import parse_qs, urlparse from uuid import UUID, uuid4 @@ -10,7 +10,6 @@ from bs4 import BeautifulSoup from flask import url_for from freezegun import freeze_time from notifications_python_client.errors import HTTPError -from notifications_utils.clients.zendesk.zendesk_client import ZendeskClient import app from app.formatters import email_safe @@ -1673,7 +1672,7 @@ def test_non_gov_users_cant_request_to_go_live( ) -@pytest.mark.parametrize('volumes, displayed_volumes, formatted_displayed_volumes, extra_tags', ( +@pytest.mark.parametrize('volumes, displayed_volumes, formatted_displayed_volumes', ( ( (('email', None), ('sms', None), ('letter', None)), ', , ', @@ -1682,7 +1681,6 @@ def test_non_gov_users_cant_request_to_go_live( 'Text messages in next year: \n' 'Letters in next year: \n' ), - ['notify_go_live_incomplete_volumes'] ), ( (('email', 1234), ('sms', 0), ('letter', 999)), @@ -1692,7 +1690,6 @@ def test_non_gov_users_cant_request_to_go_live( 'Text messages in next year: 0\n' 'Letters in next year: 999\n' ), - [], ), )) @freeze_time("2012-12-21 13:12:12.12354") @@ -1712,7 +1709,6 @@ def test_should_redirect_after_request_to_go_live( volumes, displayed_volumes, formatted_displayed_volumes, - extra_tags, ): for channel, volume in volumes: mocker.patch( @@ -1721,29 +1717,22 @@ def test_should_redirect_after_request_to_go_live( new_callable=PropertyMock, return_value=volume, ) - mock_post = mocker.patch('app.main.views.service_settings.zendesk_client.create_ticket', autospec=True) + mock_ticket = mocker.patch( + 'app.main.views.service_settings.NotifySupportTicket', + return_value='go_live_ticket', + ) + mock_ticket.TYPE_QUESTION = 'question' + mock_send_ticket_to_zendesk = mocker.patch( + 'app.main.views.service_settings.zendesk_client.send_ticket_to_zendesk', + autospec=True, + ) page = client_request.post( 'main.request_to_go_live', service_id=SERVICE_ONE_ID, _follow_redirects=True ) - mock_post.assert_called_with( - subject='Request to go live - service one', - message=ANY, - ticket_type=ZendeskClient.TYPE_QUESTION, - user_name=active_user_with_permissions['name'], - user_email=active_user_with_permissions['email_address'], - tags=[ - 'notify_action', - 'notify_go_live', - ] + extra_tags + [ - 'notify_go_live_incomplete_checklist', - 'notify_go_live_incomplete_mou', - 'notify_go_live_incomplete_team_member', - ], - requester_sees_message_content=False, - ) - assert mock_post.call_args[1]['message'] == ( + + expected_message = ( 'Service: service one\n' 'http://localhost/services/{service_id}\n' '\n' @@ -1764,6 +1753,18 @@ def test_should_redirect_after_request_to_go_live( service_id=SERVICE_ONE_ID, formatted_displayed_volumes=formatted_displayed_volumes, ) + mock_ticket.assert_called_once_with( + subject='Request to go live - service one', + message=expected_message, + ticket_type='question', + user_name=active_user_with_permissions['name'], + user_email=active_user_with_permissions['email_address'], + requester_sees_message_content=False, + org_id=None, + org_type='central', + service_id=SERVICE_ONE_ID, + ) + mock_send_ticket_to_zendesk.assert_called_once_with('go_live_ticket') 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.' @@ -1802,14 +1803,22 @@ def test_request_to_go_live_displays_go_live_notes_in_zendesk_ticket( request_to_go_live_notes=go_live_note, ) ) - mock_post = mocker.patch('app.main.views.service_settings.zendesk_client.create_ticket', autospec=True) + mock_ticket = mocker.patch( + 'app.main.views.service_settings.NotifySupportTicket', + return_value='go_live_ticket', + ) + mock_ticket.TYPE_QUESTION = 'question' + mock_send_ticket_to_zendesk = mocker.patch( + 'app.main.views.service_settings.zendesk_client.send_ticket_to_zendesk', + autospec=True, + ) client_request.post( 'main.request_to_go_live', service_id=SERVICE_ONE_ID, _follow_redirects=True ) - assert mock_post.call_args[1]['message'] == ( + expected_message = ( 'Service: service one\n' 'http://localhost/services/{service_id}\n' '\n' @@ -1833,6 +1842,19 @@ def test_request_to_go_live_displays_go_live_notes_in_zendesk_ticket( go_live_note=go_live_note ) + mock_ticket.assert_called_once_with( + subject='Request to go live - service one', + message=expected_message, + ticket_type='question', + user_name=active_user_with_permissions['name'], + user_email=active_user_with_permissions['email_address'], + requester_sees_message_content=False, + org_id=ORGANISATION_ID, + org_type='central', + service_id=SERVICE_ONE_ID + ) + mock_send_ticket_to_zendesk.assert_called_once_with('go_live_ticket') + def test_should_be_able_to_request_to_go_live_with_no_organisation( client_request, @@ -1854,7 +1876,10 @@ def test_should_be_able_to_request_to_go_live_with_no_organisation( new_callable=PropertyMock, return_value=1, ) - 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.send_ticket_to_zendesk', + autospec=True + ) client_request.post( 'main.request_to_go_live', @@ -1879,7 +1904,6 @@ def test_should_be_able_to_request_to_go_live_with_no_organisation( 'volume_letter,' 'expected_readyness,' 'agreement_signed,' - 'expected_tags,' ), ( ( # Just sending email @@ -1893,11 +1917,6 @@ def test_should_be_able_to_request_to_go_live_with_no_organisation( 1, 0, 0, 'Yes', True, - [ - 'notify_action', - 'notify_go_live', - 'notify_go_live_complete', - ], ), ( # Needs to set reply to address True, @@ -1910,12 +1929,6 @@ def test_should_be_able_to_request_to_go_live_with_no_organisation( 1, 0, 1, 'No', True, - [ - 'notify_action', - 'notify_go_live', - 'notify_go_live_incomplete_checklist', - 'notify_go_live_incomplete_email_reply_to', - ], ), ( # Just sending SMS True, @@ -1928,11 +1941,6 @@ def test_should_be_able_to_request_to_go_live_with_no_organisation( 0, 1, 0, 'Yes', True, - [ - 'notify_action', - 'notify_go_live', - 'notify_go_live_complete', - ], ), ( # Needs to change SMS sender True, @@ -1945,12 +1953,6 @@ def test_should_be_able_to_request_to_go_live_with_no_organisation( 0, 1, 0, 'No', True, - [ - 'notify_action', - 'notify_go_live', - 'notify_go_live_incomplete_checklist', - 'notify_go_live_incomplete_sms_sender', - ], ), ( # Needs team members False, @@ -1963,12 +1965,6 @@ def test_should_be_able_to_request_to_go_live_with_no_organisation( 1, 0, 0, 'No', True, - [ - 'notify_action', - 'notify_go_live', - 'notify_go_live_incomplete_checklist', - 'notify_go_live_incomplete_team_member', - ], ), ( # Needs templates True, @@ -1981,12 +1977,6 @@ def test_should_be_able_to_request_to_go_live_with_no_organisation( 0, 1, 0, 'No', True, - [ - 'notify_action', - 'notify_go_live', - 'notify_go_live_incomplete_checklist', - 'notify_go_live_incomplete_template_content', - ], ), ( # Not done anything yet False, @@ -1999,15 +1989,6 @@ def test_should_be_able_to_request_to_go_live_with_no_organisation( None, None, None, 'No', False, - [ - 'notify_action', - 'notify_go_live', - 'notify_go_live_incomplete_volumes', - 'notify_go_live_incomplete_checklist', - 'notify_go_live_incomplete_mou', - 'notify_go_live_incomplete_team_member', - 'notify_go_live_incomplete_template_content', - ], ), ), ) @@ -2027,7 +2008,6 @@ def test_ready_to_go_live( volume_letter, expected_readyness, agreement_signed, - expected_tags, ): mocker.patch( 'app.organisations_client.get_organisation', @@ -2064,10 +2044,6 @@ def test_ready_to_go_live( 'id': SERVICE_ONE_ID }).go_live_checklist_completed_as_yes_no == expected_readyness - assert app.models.service.Service( - {'id': SERVICE_ONE_ID} - ).request_to_go_live_tags == expected_tags - @pytest.mark.parametrize('route', [ 'main.service_settings',