Merge pull request #4024 from alphagov/new-zendesk-form

Use Zendesk form for go live requests

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):
-
+    ticket_message = (
-    zendesk_client.create_ticket(
-        subject='Request to go live - {}'.format(current_service.name),
-        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)
 

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)

app/templates/views/service-settings/delete.html

@@ -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 %}
-
-
-
-  <div class="govuk-grid-row">
-    <div class="govuk-grid-column-three-quarters">
-
-      <h1 class="heading-large">Delete this service from GOV.UK Notify</h1>
-
-      <p class="govuk-body">
-        This cannot be undone. You will lose:
-      </p>
-
-      <ul class="list list-bullet">
-        <li>
-          any data you’ve uploaded messages
-        </li>
-        <li>
-          any templates you’ve created
-        </li>
-        <li>
-          the history of notifications you’ve sent
-        </li>
-        <li>
-          API keys
-        </li>
-      </ul>
-
-      {% 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 %}
-
-    </div>
-  </div>
-{% endblock %}

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

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

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',
+    expected_message = (
-        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'] == (
         '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',