From a54b3c9f77d5f0970d6ce93d6c8216c85421634f Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Fri, 24 Sep 2021 11:37:28 +0100 Subject: [PATCH] Pass valid ticket type to Zendesk The feedback endpoints use `ticket_type` to decide what to display and whether or not a ticket should be escalated. We were using the ticket_type as the value for the Zendesk ticket_type. However, the Zendesk API accepts 4 values for its ticket_type and these are different from the ticket_type values we use in our code. This change converts the Notify ticket_type value to a valid Zendesk ticket_type value when creating a Notify feedback ticket. --- app/main/views/feedback.py | 13 +++++++- tests/app/main/views/test_feedback.py | 44 ++++++++++++++++----------- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/app/main/views/feedback.py b/app/main/views/feedback.py index 498433afd..4ae1985df 100644 --- a/app/main/views/feedback.py +++ b/app/main/views/feedback.py @@ -130,7 +130,7 @@ def feedback(ticket_type): ticket = NotifySupportTicket( subject='Notify feedback', message=feedback_msg, - ticket_type=ticket_type, + ticket_type=get_zendesk_ticket_type(ticket_type), p1=out_of_hours_emergency, user_name=user_name, user_email=user_email, @@ -231,3 +231,14 @@ def needs_escalation(ticket_type, severe): not current_user.is_authenticated, not in_business_hours(), )) + + +def get_zendesk_ticket_type(ticket_type): + # Zendesk has 4 ticket types - "problem", "incident", "task" and "question". + # We don't want to use a Zendesk "problem" ticket type when someone reports a + # Notify problem because they are designed to group multiple incident tickets together, + # allowing them to be solved as a group. + if ticket_type == PROBLEM_TICKET_TYPE: + return NotifySupportTicket.TYPE_INCIDENT + + return NotifySupportTicket.TYPE_QUESTION diff --git a/tests/app/main/views/test_feedback.py b/tests/app/main/views/test_feedback.py index a2fdbc4f7..edbf42ac0 100644 --- a/tests/app/main/views/test_feedback.py +++ b/tests/app/main/views/test_feedback.py @@ -5,6 +5,9 @@ import pytest from bs4 import BeautifulSoup, element from flask import url_for from freezegun import freeze_time +from notifications_utils.clients.zendesk.zendesk_client import ( + NotifySupportTicket, +) from app.main.views.feedback import in_business_hours from app.models.feedback import ( @@ -136,12 +139,13 @@ 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_ticket = mocker.patch( - 'app.main.views.feedback.NotifySupportTicket', - return_value='feedback_ticket', - ) +@pytest.mark.parametrize('ticket_type, zendesk_ticket_type', [ + (PROBLEM_TICKET_TYPE, 'incident'), + (QUESTION_TICKET_TYPE, 'question'), + (GENERAL_TICKET_TYPE, 'question'), +]) +def test_passed_non_logged_in_user_details_through_flow(client, mocker, ticket_type, zendesk_ticket_type): + mock_create_ticket = mocker.spy(NotifySupportTicket, '__init__') mock_send_ticket_to_zendesk = mocker.patch( 'app.main.views.feedback.zendesk_client.send_ticket_to_zendesk', autospec=True, @@ -161,10 +165,11 @@ def test_passed_non_logged_in_user_details_through_flow(client, mocker, ticket_t email_address_provided=True, _external=True, ) - mock_ticket.assert_called_once_with( + mock_create_ticket.assert_called_once_with( + ANY, subject='Notify feedback', message='blah\n', - ticket_type=ticket_type, + ticket_type=zendesk_ticket_type, p1=False, user_name='Anne Example', user_email='anne@example.com', @@ -172,7 +177,7 @@ def test_passed_non_logged_in_user_details_through_flow(client, mocker, ticket_t org_type=None, service_id=None ) - mock_send_ticket_to_zendesk.assert_called_once_with('feedback_ticket') + mock_send_ticket_to_zendesk.assert_called_once() @freeze_time("2016-12-12 12:00:00.000000") @@ -180,18 +185,20 @@ def test_passed_non_logged_in_user_details_through_flow(client, mocker, ticket_t {'feedback': 'blah'}, {'feedback': 'blah', 'name': 'Ignored', 'email_address': 'ignored@email.com'} ]) -@pytest.mark.parametrize('ticket_type', [PROBLEM_TICKET_TYPE, QUESTION_TICKET_TYPE]) +@pytest.mark.parametrize('ticket_type, zendesk_ticket_type', [ + (PROBLEM_TICKET_TYPE, 'incident'), + (QUESTION_TICKET_TYPE, 'question'), + (GENERAL_TICKET_TYPE, 'question'), +]) def test_passes_user_details_through_flow( client_request, mock_get_non_empty_organisations_and_services_for_user, mocker, ticket_type, + zendesk_ticket_type, data ): - mock_ticket = mocker.patch( - 'app.main.views.feedback.NotifySupportTicket', - return_value='feedback_ticket', - ) + mock_create_ticket = mocker.spy(NotifySupportTicket, '__init__') mock_send_ticket_to_zendesk = mocker.patch( 'app.main.views.feedback.zendesk_client.send_ticket_to_zendesk', autospec=True, @@ -209,10 +216,11 @@ def test_passes_user_details_through_flow( _external=True, ), ) - mock_ticket.assert_called_once_with( + mock_create_ticket.assert_called_once_with( + ANY, subject='Notify feedback', message=ANY, - ticket_type=ticket_type, + ticket_type=zendesk_ticket_type, p1=False, user_name='Test User', user_email='test@user.gov.uk', @@ -221,7 +229,7 @@ def test_passes_user_details_through_flow( service_id=SERVICE_ONE_ID ) - assert mock_ticket.call_args[1]['message'] == '\n'.join([ + assert mock_create_ticket.call_args[1]['message'] == '\n'.join([ 'blah', 'Service: "service one"', url_for( @@ -231,7 +239,7 @@ def test_passes_user_details_through_flow( ), '' ]) - mock_send_ticket_to_zendesk.assert_called_once_with('feedback_ticket') + mock_send_ticket_to_zendesk.assert_called_once() @freeze_time('2016-12-12 12:00:00.000000')