diff --git a/app/__init__.py b/app/__init__.py index 48a664431..1f6288974 100644 --- a/app/__init__.py +++ b/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) diff --git a/app/main/views/feedback.py b/app/main/views/feedback.py index 92c2ce479..c3f796baf 100644 --- a/app/main/views/feedback.py +++ b/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'), - 'person_name': user_name, - 'department_id': current_app.config.get('DESKPRO_DEPT_ID'), - 'agent_team_id': current_app.config.get('DESKPRO_ASSIGNED_AGENT_TEAM_ID'), - 'subject': 'Notify feedback {}'.format(user_name), - 'message': feedback_msg, - 'label': ticket_type, - 'urgency': 10 if urgent else 1, - } - 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() - ) + + try: + deskpro_client.create_ticket( + subject='Notify feedback {}'.format(user_name), + message=feedback_msg, + ticket_type=ticket_type, + urgency=10 if urgent else 1, + user_email=user_email, + user_name=user_name ) + except DeskproError: abort(500, "Feedback submission failed") return redirect(url_for('.thanks', urgent=urgent, anonymous=anonymous)) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index c274c01bb..e1f44d919 100644 --- a/app/main/views/service_settings.py +++ b/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 = { - 'person_email': current_user.email_address, - 'person_name': current_user.name, - 'department_id': current_app.config.get('DESKPRO_DEPT_ID'), - 'agent_team_id': current_app.config.get('DESKPRO_ASSIGNED_AGENT_TEAM_ID'), - 'subject': 'Request to go live - {}'.format(current_service['name']), - 'message': ( - 'On behalf of {} ({})\n' - '\n---' - '\nOrganisation type: {}' - '\nMOU in place: {}' - '\nChannel: {}\nStart date: {}\nStart volume: {}' - '\nPeak volume: {}' - '\nFeatures: {}' - ).format( - current_service['name'], - url_for('main.service_dashboard', service_id=current_service['id'], _external=True), - current_service['organisation_type'], - form.mou.data, - formatted_list(filter(None, ( - 'email' if form.channel_email.data else None, - 'text messages' if form.channel_sms.data else None, - 'letters' if form.channel_letter.data else None, - )), before_each='', after_each=''), - form.start_date.data, - form.start_volume.data, - form.peak_volume.data, - formatted_list(filter(None, ( - 'one off' if form.method_one_off.data else None, - 'file upload' if form.method_upload.data else None, - 'API' if form.method_api.data else None, - )), before_each='', after_each='') - - ) - } - 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()) + try: + deskpro_client.create_ticket( + subject='Request to go live - {}'.format(current_service['name']), + message=( + 'On behalf of {} ({})\n' + '\n---' + '\nOrganisation type: {}' + '\nMOU in place: {}' + '\nChannel: {}\nStart date: {}\nStart volume: {}' + '\nPeak volume: {}' + '\nFeatures: {}' + ).format( + current_service['name'], + url_for('main.service_dashboard', service_id=current_service['id'], _external=True), + current_service['organisation_type'], + form.mou.data, + formatted_list(filter(None, ( + 'email' if form.channel_email.data else None, + 'text messages' if form.channel_sms.data else None, + 'letters' if form.channel_letter.data else None, + )), before_each='', after_each=''), + form.start_date.data, + form.start_volume.data, + form.peak_volume.data, + formatted_list(filter(None, ( + 'one off' if form.method_one_off.data else None, + 'file upload' if form.method_upload.data else None, + 'API' if form.method_api.data else None, + )), before_each='', after_each='') + ), + user_email=current_user.email_address, + user_name=current_user.name ) + except DeskproError: abort(500, "Request to go live submission failed") flash('We’ve received your request to go live', 'default') diff --git a/requirements.txt b/requirements.txt index e7af49127..726af8056 100644 --- a/requirements.txt +++ b/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.0#egg=notifications-utils==23.5.0 diff --git a/tests/app/main/views/test_feedback.py b/tests/app/main/views/test_feedback.py index af819470c..4a1ac5120 100644 --- a/tests/app/main/views/test_feedback.py +++ b/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( - 'app.main.views.feedback.requests.post', - return_value=Mock(status_code=201) - ) + mock_post = mocker.patch('app.main.views.feedback.deskpro_client.create_ticket') 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, - data={ - 'department_id': ANY, - 'agent_team_id': ANY, - 'subject': 'Notify feedback {}'.format(data['name']), - 'message': 'Environment: http://localhost/\n\nblah', - 'person_email': 'rip@gmail.com', - 'person_name': 'Steve Irwin', - 'label': ticket_type, - 'urgency': ANY, - }, - headers=ANY + subject='Notify feedback {}'.format(data['name']), + message='Environment: http://localhost/\n\nblah', + user_email='rip@gmail.com', + user_name='Steve Irwin', + ticket_type=ticket_type, + urgency=ANY, ) @@ -122,10 +114,7 @@ def test_passes_user_details_through_flow( ticket_type, data ): - 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') 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, - data={ - 'department_id': ANY, - 'agent_team_id': ANY, - 'subject': 'Notify feedback Test User', - 'message': ANY, - 'person_email': 'test@user.gov.uk', - 'person_name': 'Test User', - 'label': ticket_type, - 'urgency': ANY, - }, - headers=ANY + subject='Notify feedback Test User', + message=ANY, + user_email='test@user.gov.uk', + user_name='Test User', + ticket_type=ticket_type, + urgency=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( - 'app.main.views.feedback.requests.post', - return_value=Mock(status_code=201) - ) + mocker.patch('app.main.views.feedback.deskpro_client') 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', - return_value=Mock( - status_code=401, - json=lambda: { - 'error_code': 'invalid_auth', - 'error_message': 'Please provide a valid API key or token'})) + 'app.main.views.feedback.deskpro_client.create_ticket', + side_effect=DeskproError + ) 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]) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 9bae58b3e..be48fda16 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/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( - 'app.main.views.feedback.requests.post', - return_value=Mock(status_code=201), - ) + mock_post = mocker.patch('app.main.views.service_settings.deskpro_client.create_ticket') 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, - data={ - 'subject': 'Request to go live - service one', - 'department_id': ANY, - 'agent_team_id': ANY, - 'message': ANY, - 'person_name': active_user_with_permissions.name, - 'person_email': active_user_with_permissions.email_address - }, - headers=ANY + subject='Request to go live - service one', + message=ANY, + user_name=active_user_with_permissions.name, + user_email=active_user_with_permissions.email_address ) - 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',