Merge branch 'master' into master

app/__init__.py

@@ -25,6 +25,7 @@ from functools import partial
 
 from notifications_python_client.errors import HTTPError
 from notifications_utils import logging, request_helper, formatters
+from notifications_utils.clients import DeskproClient
 from notifications_utils.clients.statsd.statsd_client import StatsdClient
 from notifications_utils.recipients import (
     validate_phone_number,
@@ -73,6 +74,7 @@ provider_client = ProviderClient()
 organisations_client = OrganisationsClient()
 asset_fingerprinter = AssetFingerprinter()
 statsd_client = StatsdClient()
+deskpro_client = DeskproClient()
 letter_jobs_client = LetterJobsClient()
 inbound_number_client = InboundNumberClient()
 billing_api_client = BillingAPIClient()
@@ -90,6 +92,7 @@ def create_app(application):
 
     init_app(application)
     statsd_client.init_app(application)
+    deskpro_client.init_app(application)
     logging.init_app(application, statsd_client)
     csrf.init_app(application)
     request_helper.init_app(application)

app/main/views/feedback.py

@@ -1,12 +1,14 @@
-import requests
 import pytz
-from flask import render_template, url_for, redirect, current_app, abort, request, session
+from flask import render_template, url_for, redirect, abort, request, session
 from flask_login import current_user
-from app import convert_to_boolean, current_service, service_api_client
+from notifications_utils.clients import DeskproError
+
+from app import convert_to_boolean, current_service, service_api_client, deskpro_client
 from app.main import main
 from app.main.forms import SupportType, Feedback, Problem, Triage
 from datetime import datetime
 
+
 QUESTION_TICKET_TYPE = 'ask-question-give-feedback'
 PROBLEM_TICKET_TYPE = "report-problem"
 
@@ -113,31 +115,17 @@ def feedback(ticket_type):
             '' if user_email else '{} (no email address supplied)'.format(form.name.data),
             form.feedback.data
         )
-        data = {
+
-            'person_email': user_email or current_app.config.get('DESKPRO_PERSON_EMAIL'),
+        try:
-            'person_name': user_name,
+            deskpro_client.create_ticket(
-            'department_id': current_app.config.get('DESKPRO_DEPT_ID'),
+                subject='Notify feedback {}'.format(user_name),
-            'agent_team_id': current_app.config.get('DESKPRO_ASSIGNED_AGENT_TEAM_ID'),
+                message=feedback_msg,
-            'subject': 'Notify feedback {}'.format(user_name),
+                ticket_type=ticket_type,
-            'message': feedback_msg,
+                urgency=10 if urgent else 1,
-            'label': ticket_type,
+                user_email=user_email,
-            'urgency': 10 if urgent else 1,
+                user_name=user_name
-        }
-        headers = {
-            "X-DeskPRO-API-Key": current_app.config.get('DESKPRO_API_KEY'),
-            'Content-Type': "application/x-www-form-urlencoded"
-        }
-        resp = requests.post(
-            current_app.config.get('DESKPRO_API_HOST') + '/api/tickets',
-            data=data,
-            headers=headers)
-        if resp.status_code != 201:
-            current_app.logger.error(
-                "Deskpro create ticket request failed with {} '{}'".format(
-                    resp.status_code,
-                    resp.json()
-                )
             )
+        except DeskproError:
             abort(500, "Feedback submission failed")
         return redirect(url_for('.thanks', urgent=urgent, anonymous=anonymous))
 
@@ -191,25 +179,40 @@ def is_weekend(time):
 
 
 def is_bank_holiday(time):
-    return time.strftime('%d/%m/%Y') in {
+    return time.strftime('%Y-%m-%d') in {
-        # taken from
+        # taken from https://www.gov.uk/bank-holidays.json
-        # https://github.com/alphagov/calendars/blob/7f6512b0a95d77aa22accef105860074c19f1ec0/lib/data/bank-holidays.json
+        "2016-01-01",
-        "01/01/2016",
+        "2016-03-25",
-        "25/03/2016",
+        "2016-03-28",
-        "28/03/2016",
+        "2016-05-02",
-        "02/05/2016",
+        "2016-05-30",
-        "30/05/2016",
+        "2016-08-29",
-        "29/08/2016",
+        "2016-12-26",
-        "26/12/2016",
+        "2016-12-27",
-        "27/12/2016",
+        "2017-01-02",
-        "02/01/2017",
+        "2017-04-14",
-        "14/04/2017",
+        "2017-04-17",
-        "17/04/2017",
+        "2017-05-01",
-        "01/05/2017",
+        "2017-05-29",
-        "29/05/2017",
+        "2017-08-28",
-        "28/08/2017",
+        "2017-12-25",
-        "25/12/2017",
+        "2017-12-26",
-        "26/12/2017",
+        "2018-01-01",
+        "2018-03-30",
+        "2018-04-02",
+        "2018-05-07",
+        "2018-05-28",
+        "2018-08-27",
+        "2018-12-25",
+        "2018-12-26",
+        "2019-01-01",
+        "2019-04-19",
+        "2019-04-22",
+        "2019-05-06",
+        "2019-05-27",
+        "2019-08-26",
+        "2019-12-25",
+        "2019-12-26",
     }
 
 

app/main/views/service_settings.py

@@ -1,4 +1,3 @@
-import requests
 from flask import (
     render_template,
     redirect,
@@ -7,7 +6,6 @@ from flask import (
     session,
     flash,
     abort,
-    current_app
 )
 
 from flask_login import (
@@ -16,9 +14,10 @@ from flask_login import (
 )
 
 from notifications_utils.field import Field
+from notifications_utils.clients import DeskproError
 from notifications_python_client.errors import HTTPError
 
-from app import service_api_client
+from app import service_api_client, deskpro_client
 from app.main import main
 from app.utils import user_has_permissions, email_safe, get_cdn_domain
 from app.main.forms import (
@@ -153,56 +152,40 @@ def service_request_to_go_live(service_id):
     form = RequestToGoLiveForm()
 
     if form.validate_on_submit():
-        data = {
+        try:
-            'person_email': current_user.email_address,
+            deskpro_client.create_ticket(
-            'person_name': current_user.name,
+                subject='Request to go live - {}'.format(current_service['name']),
-            'department_id': current_app.config.get('DESKPRO_DEPT_ID'),
+                message=(
-            'agent_team_id': current_app.config.get('DESKPRO_ASSIGNED_AGENT_TEAM_ID'),
+                    'On behalf of {} ({})\n'
-            'subject': 'Request to go live - {}'.format(current_service['name']),
+                    '\n---'
-            'message': (
+                    '\nOrganisation type: {}'
-                'On behalf of {} ({})\n'
+                    '\nMOU in place: {}'
-                '\n---'
+                    '\nChannel: {}\nStart date: {}\nStart volume: {}'
-                '\nOrganisation type: {}'
+                    '\nPeak volume: {}'
-                '\nMOU in place: {}'
+                    '\nFeatures: {}'
-                '\nChannel: {}\nStart date: {}\nStart volume: {}'
+                ).format(
-                '\nPeak volume: {}'
+                    current_service['name'],
-                '\nFeatures: {}'
+                    url_for('main.service_dashboard', service_id=current_service['id'], _external=True),
-            ).format(
+                    current_service['organisation_type'],
-                current_service['name'],
+                    form.mou.data,
-                url_for('main.service_dashboard', service_id=current_service['id'], _external=True),
+                    formatted_list(filter(None, (
-                current_service['organisation_type'],
+                        'email' if form.channel_email.data else None,
-                form.mou.data,
+                        'text messages' if form.channel_sms.data else None,
-                formatted_list(filter(None, (
+                        'letters' if form.channel_letter.data else None,
-                    'email' if form.channel_email.data else None,
+                    )), before_each='', after_each=''),
-                    'text messages' if form.channel_sms.data else None,
+                    form.start_date.data,
-                    'letters' if form.channel_letter.data else None,
+                    form.start_volume.data,
-                )), before_each='', after_each=''),
+                    form.peak_volume.data,
-                form.start_date.data,
+                    formatted_list(filter(None, (
-                form.start_volume.data,
+                        'one off' if form.method_one_off.data else None,
-                form.peak_volume.data,
+                        'file upload' if form.method_upload.data else None,
-                formatted_list(filter(None, (
+                        'API' if form.method_api.data else None,
-                    'one off' if form.method_one_off.data else None,
+                    )), before_each='', after_each='')
-                    'file upload' if form.method_upload.data else None,
+                ),
-                    'API' if form.method_api.data else None,
+                user_email=current_user.email_address,
-                )), before_each='', after_each='')
+                user_name=current_user.name
-
-            )
-        }
-        headers = {
-            "X-DeskPRO-API-Key": current_app.config.get('DESKPRO_API_KEY'),
-            'Content-Type': "application/x-www-form-urlencoded"
-        }
-        resp = requests.post(
-            current_app.config.get('DESKPRO_API_HOST') + '/api/tickets',
-            data=data,
-            headers=headers
-        )
-        if resp.status_code != 201:
-            current_app.logger.error(
-                "Deskpro create ticket request failed with {} '{}'".format(
-                    resp.status_code,
-                    resp.json())
             )
+        except DeskproError:
             abort(500, "Request to go live submission failed")
 
         flash('We’ve received your request to go live', 'default')

app/templates/views/api/index.html

@@ -71,6 +71,9 @@
               <dt>{{ key }}:</dt>
               <dd class="api-notifications-item-data-item">{{ notification[key] }}</dd>
             {% endfor %}
+            {% if notification['notification_type'] == 'letter' %}
+              <a href="{{ url_for('.view_notification', service_id=current_service.id, notification_id=notification.id) }}">View letter</a>
+            {% endif %}
           </dl>
         </div>
       </details>

app/templates/views/notifications.html

@@ -47,11 +47,6 @@
       <input type="hidden" name="csrf_token" value="{{ csrf_token() }}"/>
     </form>
   {% endif %}
-    <p class="bottom-gutter">
-       <a href="{{ download_link }}" download="download" class="heading-small">Download this report</a>
-       &emsp;
-       Data available for 7 days
-    </p>
   {{ ajax_block(
     partials,
     url_for('.get_notifications_as_json', service_id=current_service.id, message_type=message_type, status=status, page=page),

app/templates/views/two-factor-email.html

@@ -2,7 +2,7 @@
 {% from "components/page-footer.html" import page_footer %}
 
 {% block per_page_title %}
-  Email verification
+  {{ title }}
 {% endblock %}
 
 {% block maincolumn_content %}
@@ -10,7 +10,8 @@
 <div class="grid-row">
   <div class="column-two-thirds">
     <h1 class="heading-large">{{ title }}</h1>
-    <p> We’ve sent you an email with your login link</p>
+    <p>We’ve emailed you a link to sign in to Notify.</p>
+    <p>Clicking the link will open Notify in a new browser window, so you can close this one.</p>
     {{ page_footer(
       secondary_link=url_for('main.email_not_received'),
       secondary_link_text='Not received an email?'
@@ -18,4 +19,4 @@
   </div>
 </div>
 
-{% endblock %}
+{% endblock %}

requirements.txt

@@ -20,4 +20,4 @@ notifications-python-client==4.7.1
 awscli==1.14.22
 awscli-cwlogs>=1.4,<1.5
 
-git+https://github.com/alphagov/notifications-utils.git@23.4.0#egg=notifications-utils==23.4.0
+git+https://github.com/alphagov/notifications-utils.git@23.5.1#egg=notifications-utils==23.5.1

tests/app/main/views/test_api_integration.py

@@ -11,6 +11,7 @@ from tests.conftest import (
     mock_get_service,
     mock_get_live_service,
     mock_get_service_with_letters,
+    mock_get_notifications,
     normalize_spaces,
     SERVICE_ONE_ID,
     mock_get_valid_service_callback_api,
@@ -66,6 +67,29 @@ def test_should_show_api_page_with_no_notifications(
     assert 'When you send messages via the API they’ll appear here.' in rows[len(rows) - 1].text.strip()
 
 
+@pytest.mark.parametrize('template_type, has_links', [
+    ('sms', False),
+    ('letter', True),
+])
+def test_letter_notifications_should_have_link_to_view_letter(
+    client_request,
+    api_user_active,
+    fake_uuid,
+    mock_has_permissions,
+    mocker,
+    template_type,
+    has_links
+):
+    mock_get_notifications(mocker, api_user_active, diff_template_type=template_type)
+
+    page = client_request.get(
+        'main.api_integration',
+        service_id=fake_uuid,
+    )
+
+    assert (page.select_one('details a') is not None) == has_links
+
+
 def test_should_show_api_page_for_live_service(
     logged_in_client,
     mock_login,

tests/app/main/views/test_feedback.py

@@ -3,8 +3,9 @@ from functools import partial
 import pytest
 from flask import url_for
 from werkzeug.exceptions import InternalServerError
-from unittest.mock import Mock, ANY
+from unittest.mock import ANY
 from freezegun import freeze_time
+from notifications_utils.clients import DeskproError
 from tests.conftest import (
     mock_get_services,
     mock_get_services_with_no_services,
@@ -80,10 +81,7 @@ def test_get_feedback_page(client, ticket_type, expected_status_code):
 @freeze_time('2016-12-12 12:00:00.000000')
 @pytest.mark.parametrize('ticket_type', [PROBLEM_TICKET_TYPE, QUESTION_TICKET_TYPE])
 def test_passed_non_logged_in_user_details_through_flow(client, mocker, ticket_type):
-    mock_post = mocker.patch(
+    mock_post = mocker.patch('app.main.views.feedback.deskpro_client.create_ticket')
-        'app.main.views.feedback.requests.post',
-        return_value=Mock(status_code=201)
-    )
 
     data = {'feedback': 'blah', 'name': 'Steve Irwin', 'email_address': 'rip@gmail.com'}
 
@@ -95,18 +93,12 @@ def test_passed_non_logged_in_user_details_through_flow(client, mocker, ticket_t
     assert resp.status_code == 302
     assert resp.location == url_for('main.thanks', urgent=True, anonymous=False, _external=True)
     mock_post.assert_called_with(
-        ANY,
+        subject='Notify feedback {}'.format(data['name']),
-        data={
+        message='Environment: http://localhost/\n\nblah',
-            'department_id': ANY,
+        user_email='rip@gmail.com',
-            'agent_team_id': ANY,
+        user_name='Steve Irwin',
-            'subject': 'Notify feedback {}'.format(data['name']),
+        ticket_type=ticket_type,
-            'message': 'Environment: http://localhost/\n\nblah',
+        urgency=ANY,
-            'person_email': 'rip@gmail.com',
-            'person_name': 'Steve Irwin',
-            'label': ticket_type,
-            'urgency': ANY,
-        },
-        headers=ANY
     )
 
 
@@ -122,10 +114,7 @@ def test_passes_user_details_through_flow(
     ticket_type,
     data
 ):
-    mock_post = mocker.patch(
+    mock_post = mocker.patch('app.main.views.feedback.deskpro_client.create_ticket')
-        'app.main.views.feedback.requests.post',
-        return_value=Mock(status_code=201)
-    )
 
     resp = logged_in_client.post(
         url_for('main.feedback', ticket_type=ticket_type),
@@ -135,20 +124,14 @@ def test_passes_user_details_through_flow(
     assert resp.status_code == 302
     assert resp.location == url_for('main.thanks', urgent=True, anonymous=False, _external=True)
     mock_post.assert_called_with(
-        ANY,
+        subject='Notify feedback Test User',
-        data={
+        message=ANY,
-            'department_id': ANY,
+        user_email='test@user.gov.uk',
-            'agent_team_id': ANY,
+        user_name='Test User',
-            'subject': 'Notify feedback Test User',
+        ticket_type=ticket_type,
-            'message': ANY,
+        urgency=ANY,
-            'person_email': 'test@user.gov.uk',
-            'person_name': 'Test User',
-            'label': ticket_type,
-            'urgency': ANY,
-        },
-        headers=ANY
     )
-    assert mock_post.call_args[1]['data']['message'] == '\n'.join([
+    assert mock_post.call_args[1]['message'] == '\n'.join([
         'Environment: http://localhost/',
         'Service "service one": {}'.format(url_for(
             'main.service_dashboard',
@@ -178,10 +161,7 @@ def test_email_address_required_for_problems(
     things_expected_in_url,
     expected_error
 ):
-    mocker.patch(
+    mocker.patch('app.main.views.feedback.deskpro_client')
-        'app.main.views.feedback.requests.post',
-        return_value=Mock(status_code=201)
-    )
     response = client.post(
         url_for('main.feedback', ticket_type=ticket_type),
         data=data,
@@ -221,14 +201,14 @@ def test_urgency(
     is_urgent,
 ):
     mocker.patch('app.main.views.feedback.in_business_hours', return_value=is_in_business_hours)
-    mock_post = mocker.patch('app.main.views.feedback.requests.post', return_value=Mock(status_code=201))
+    mock_post = mocker.patch('app.main.views.feedback.deskpro_client.create_ticket')
     response = logged_in_client.post(
         url_for('main.feedback', ticket_type=ticket_type, severe=severe),
         data={'feedback': 'blah', 'email_address': 'test@example.com'},
     )
     assert response.status_code == 302
     assert response.location == url_for('main.thanks', urgent=is_urgent, anonymous=False, _external=True)
-    assert mock_post.call_args[1]['data']['urgency'] == numeric_urgency
+    assert mock_post.call_args[1]['urgency'] == numeric_urgency
 
 
 ids, params = zip(*[
@@ -466,22 +446,16 @@ def test_bat_email_page(
 @pytest.mark.parametrize('ticket_type', [PROBLEM_TICKET_TYPE, QUESTION_TICKET_TYPE])
 def test_log_error_on_post(app_, mocker, ticket_type):
     mock_post = mocker.patch(
-        'app.main.views.feedback.requests.post',
+        'app.main.views.feedback.deskpro_client.create_ticket',
-        return_value=Mock(
+        side_effect=DeskproError
-            status_code=401,
+    )
-            json=lambda: {
-                'error_code': 'invalid_auth',
-                'error_message': 'Please provide a valid API key or token'}))
     with app_.test_request_context():
-        mock_logger = mocker.patch.object(app_.logger, 'error')
         with app_.test_client() as client:
             with pytest.raises(InternalServerError):
                 client.post(
                     url_for('main.feedback', ticket_type=ticket_type),
                     data={'feedback': "blah", 'name': "Steve Irwin", 'email_address': 'rip@gmail.com'})
             assert mock_post.called
-            mock_logger.assert_called_with(
-                "Deskpro create ticket request failed with {} '{}'".format(mock_post().status_code, mock_post().json()))
 
 
 @pytest.mark.parametrize('logged_in', [True, False])

tests/app/main/views/test_service_settings.py

@@ -1,10 +1,9 @@
 import uuid
-from unittest.mock import call, ANY, Mock
+from unittest.mock import call, ANY
 
 import pytest
 from bs4 import BeautifulSoup
 from flask import url_for
-from werkzeug.exceptions import InternalServerError
 
 import app
 from app.utils import email_safe
@@ -453,10 +452,7 @@ def test_should_redirect_after_request_to_go_live(
     single_sms_sender,
     mock_get_service_settings_page_common
 ):
-    mock_post = mocker.patch(
+    mock_post = mocker.patch('app.main.views.service_settings.deskpro_client.create_ticket')
-        'app.main.views.feedback.requests.post',
-        return_value=Mock(status_code=201),
-    )
     page = client_request.post(
         'main.service_request_to_go_live',
         service_id=SERVICE_ONE_ID,
@@ -474,19 +470,13 @@ def test_should_redirect_after_request_to_go_live(
         _follow_redirects=True
     )
     mock_post.assert_called_with(
-        ANY,
+        subject='Request to go live - service one',
-        data={
+        message=ANY,
-            'subject': 'Request to go live - service one',
+        user_name=active_user_with_permissions.name,
-            'department_id': ANY,
+        user_email=active_user_with_permissions.email_address
-            'agent_team_id': ANY,
-            'message': ANY,
-            'person_name': active_user_with_permissions.name,
-            'person_email': active_user_with_permissions.email_address
-        },
-        headers=ANY
     )
 
-    returned_message = mock_post.call_args[1]['data']['message']
+    returned_message = mock_post.call_args[1]['message']
     assert 'On behalf of service one' in returned_message
     assert 'Organisation type: central' in returned_message
     assert 'Channel: email and text messages' in returned_message
@@ -503,40 +493,6 @@ def test_should_redirect_after_request_to_go_live(
     )
 
 
-def test_log_error_on_request_to_go_live(
-        app_,
-        logged_in_client,
-        service_one,
-        mocker,
-):
-    mock_post = mocker.patch(
-        'app.main.views.service_settings.requests.post',
-        return_value=Mock(
-            status_code=401,
-            json=lambda: {
-                'error_code': 'invalid_auth',
-                'error_message': 'Please provide a valid API key or token'
-            }
-        )
-    )
-    mock_logger = mocker.patch.object(app_.logger, 'error')
-    with pytest.raises(InternalServerError):
-        logged_in_client.post(
-            url_for('main.service_request_to_go_live', service_id=service_one['id']),
-            data={
-                'mou': 'yes',
-                'channel': 'emails',
-                'start_date': 'start_date',
-                'start_volume': 'start_volume',
-                'peak_volume': 'peak_volume',
-                'upload_or_api': 'API'
-            }
-        )
-    mock_logger.assert_called_with(
-        "Deskpro create ticket request failed with {} '{}'".format(mock_post().status_code, mock_post().json())
-    )
-
-
 @pytest.mark.parametrize('route', [
     'main.service_settings',
     'main.service_name_change',