From a8deec1b9f2df26190473bf056c9e49722964796 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Thu, 16 Sep 2021 12:05:32 +0100 Subject: [PATCH 1/3] Bump utils to 46.1.0 --- requirements.in | 2 +- requirements.txt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements.in b/requirements.in index 560e0e5bb..f3868b299 100644 --- a/requirements.in +++ b/requirements.in @@ -26,7 +26,7 @@ fido2==0.9.1 awscli-cwlogs>=1.4,<1.5 itsdangerous==1.1.0 # pyup: <2 -git+https://github.com/alphagov/notifications-utils.git@46.0.0#egg=notifications-utils==46.0.0 +git+https://github.com/alphagov/notifications-utils.git@46.1.0#egg=notifications-utils==46.1.0 git+https://github.com/alphagov/govuk-frontend-jinja.git@v0.5.8-alpha#egg=govuk-frontend-jinja==0.5.8-alpha # cryptography 3.4+ incorporates Rust code, which isn't supported on PaaS diff --git a/requirements.txt b/requirements.txt index 2e25cffe4..941268c3e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -75,7 +75,7 @@ govuk-bank-holidays==0.8 # via # -r requirements.in # notifications-utils -git+https://github.com/alphagov/govuk-frontend-jinja.git@v0.5.8-alpha#egg=govuk-frontend-jinja==0.5.8-alpha +govuk-frontend-jinja @ git+https://github.com/alphagov/govuk-frontend-jinja.git@v0.5.8-alpha # via -r requirements.in greenlet==1.1.0 # via eventlet @@ -118,7 +118,7 @@ mistune==0.8.4 # via notifications-utils notifications-python-client==6.2.1 # via -r requirements.in -git+https://github.com/alphagov/notifications-utils.git@46.0.0#egg=notifications-utils==46.0.0 +notifications-utils @ git+https://github.com/alphagov/notifications-utils.git@46.1.0 # via -r requirements.in openpyxl==3.0.7 # via pyexcel-xlsx From 39c26f5bfbfd8977f21266df72b40da7aa413e16 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Thu, 16 Sep 2021 11:17:45 +0100 Subject: [PATCH 2/3] 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', From d41185db5f1642548c4715b977044c8157e686a8 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Thu, 16 Sep 2021 12:13:45 +0100 Subject: [PATCH 3/3] Delete unused template `delete.html` hasn't been used since e89f89bd11d0d54f29b3a5adea7fe00400f46055 --- .../views/service-settings/delete.html | 47 ------------------- 1 file changed, 47 deletions(-) delete mode 100644 app/templates/views/service-settings/delete.html diff --git a/app/templates/views/service-settings/delete.html b/app/templates/views/service-settings/delete.html deleted file mode 100644 index 016e6cc4d..000000000 --- a/app/templates/views/service-settings/delete.html +++ /dev/null @@ -1,47 +0,0 @@ -{% extends "withnav_template.html" %} -{% from "components/page-footer.html" import page_footer %} -{% from "components/form.html" import form_wrapper %} - -{% block service_page_title %} - Delete service -{% endblock %} - -{% block maincolumn_content %} - - - -
-
- -

Delete this service from GOV.UK Notify

- -

- This cannot be undone. You will lose: -

- -
    -
  • - any data you’ve uploaded messages -
    • -
  • - any templates you’ve created -
    • -
  • - the history of notifications you’ve sent -
    • -
  • - API keys -
    • -
- - {% call form_wrapper() %} - {{ page_footer( - 'Yes, delete ‘{}’'.format(current_service.name), - destructive=True, - back_link=url_for('.service_settings', service_id=current_service.id) - ) }} - {% endcall %} - -
-
-{% endblock %}