From 40282316d0f9382802dc0916de7cb60a1fd4315d Mon Sep 17 00:00:00 2001 From: Clinton Blackburn Date: Thu, 2 Jul 2015 19:14:00 -0400 Subject: [PATCH] Creating Zendesk refund notifications via API Refund notifications are now created using the Zendesk API. This ensures the correct requester information is set for the ticket, and allows for tagging of tickets. XCOM-451 --- lms/djangoapps/commerce/signals.py | 100 ++++++++++---- lms/djangoapps/commerce/tests/__init__.py | 7 +- lms/djangoapps/commerce/tests/test_signals.py | 123 ++++++++++++------ 3 files changed, 160 insertions(+), 70 deletions(-) diff --git a/lms/djangoapps/commerce/signals.py b/lms/djangoapps/commerce/signals.py index b135325dca..4ab72b620b 100644 --- a/lms/djangoapps/commerce/signals.py +++ b/lms/djangoapps/commerce/signals.py @@ -1,26 +1,28 @@ """ Signal handling functions for use with external commerce service. """ +import json import logging from urlparse import urljoin from django.conf import settings from django.contrib.auth.models import AnonymousUser -from django.core.mail import EmailMultiAlternatives from django.dispatch import receiver from django.utils.translation import ugettext as _ from ecommerce_api_client.exceptions import HttpClientError +import requests + from microsite_configuration import microsite from request_cache.middleware import RequestCache from student.models import UNENROLL_DONE - from commerce import ecommerce_api_client, is_commerce_service_configured log = logging.getLogger(__name__) @receiver(UNENROLL_DONE) -def handle_unenroll_done(sender, course_enrollment=None, skip_refund=False, **kwargs): # pylint: disable=unused-argument +def handle_unenroll_done(sender, course_enrollment=None, skip_refund=False, + **kwargs): # pylint: disable=unused-argument """ Signal receiver for unenrollments, used to automatically initiate refunds when applicable. @@ -140,33 +142,77 @@ def refund_seat(course_enrollment, request_user): return refund_ids -def send_refund_notification(course_enrollment, refund_ids): - """ - Issue an email notification to the configured email recipient about a - newly-initiated refund request. +def create_zendesk_ticket(requester_name, requester_email, subject, body, tags=None): + """ Create a Zendesk ticket via API. """ + if not (settings.ZENDESK_URL and settings.ZENDESK_USER and settings.ZENDESK_API_KEY): + log.debug('Zendesk is not configured. Cannot create a ticket.') + return + + # Copy the tags to avoid modifying the original list. + tags = list(tags or []) + tags.append('LMS') + + # Remove duplicates + tags = list(set(tags)) + + data = { + 'ticket': { + 'requester': { + 'name': requester_name, + 'email': requester_email + }, + 'subject': subject, + 'comment': {'body': body}, + 'tags': tags + } + } + + # Encode the data to create a JSON payload + payload = json.dumps(data) + + # Set the request parameters + url = urljoin(settings.ZENDESK_URL, '/api/v2/tickets.json') + user = '{}/token'.format(settings.ZENDESK_USER) + pwd = settings.ZENDESK_API_KEY + headers = {'content-type': 'application/json'} + + try: + response = requests.post(url, data=payload, auth=(user, pwd), headers=headers) + + # Check for HTTP codes other than 201 (Created) + if response.status_code != 201: + log.error(u'Failed to create ticket. Status: [%d], Body: [%s]', response.status_code, response.content) + else: + log.debug('Successfully created ticket.') + except Exception: # pylint: disable=broad-except + log.exception('Failed to create ticket.') + return + + +def generate_refund_notification_body(student, refund_ids): # pylint: disable=invalid-name + """ Returns a refund notification message body. """ + msg = _( + "A refund request has been initiated for {username} ({email}). " + "To process this request, please visit the link(s) below." + ).format(username=student.username, email=student.email) + + refund_urls = [urljoin(settings.ECOMMERCE_PUBLIC_URL_ROOT, '/dashboard/refunds/{}/'.format(refund_id)) + for refund_id in refund_ids] + + return '{msg}\n\n{urls}'.format(msg=msg, urls='\n'.join(refund_urls)) + + +def send_refund_notification(course_enrollment, refund_ids): + """ Notify the support team of the refund request. """ + + tags = ['auto_refund'] - This function does not do any exception handling; callers are responsible - for capturing and recovering from any errors. - """ if microsite.is_request_in_microsite(): # this is not presently supported with the external service. raise NotImplementedError("Unable to send refund processing emails to microsite teams.") - for_user = course_enrollment.user + student = course_enrollment.user subject = _("[Refund] User-Requested Refund") - message = _( - "A refund request has been initiated for {username} ({email}). " - "To process this request, please visit the link(s) below." - ).format(username=for_user.username, email=for_user.email) - - refund_urls = [ - urljoin(settings.ECOMMERCE_PUBLIC_URL_ROOT, '/dashboard/refunds/{}/'.format(refund_id)) - for refund_id in refund_ids - ] - text_body = '\r\n'.join([message] + refund_urls + ['']) - refund_links = ['{0}'.format(url) for url in refund_urls] - html_body = '

{}

'.format('
'.join([message] + refund_links)) - - email_message = EmailMultiAlternatives(subject, text_body, for_user.email, [settings.PAYMENT_SUPPORT_EMAIL]) - email_message.attach_alternative(html_body, "text/html") - email_message.send() + body = generate_refund_notification_body(student, refund_ids) + requester_name = student.profile.name or student.username + create_zendesk_ticket(requester_name, student.email, subject, body, tags) diff --git a/lms/djangoapps/commerce/tests/__init__.py b/lms/djangoapps/commerce/tests/__init__.py index 4af326a662..394969aeb2 100644 --- a/lms/djangoapps/commerce/tests/__init__.py +++ b/lms/djangoapps/commerce/tests/__init__.py @@ -1,6 +1,5 @@ # -*- coding: utf-8 -*- """ Commerce app tests package. """ -import json from django.test import TestCase from django.test.utils import override_settings @@ -11,7 +10,7 @@ import mock from commerce import ecommerce_api_client from student.tests.factories import UserFactory - +JSON = 'application/json' TEST_PUBLIC_URL_ROOT = 'http://www.example.com' TEST_API_URL = 'http://www-internal.example.com/api' TEST_API_SIGNING_KEY = 'edx' @@ -48,7 +47,7 @@ class EcommerceApiClientTest(TestCase): httpretty.POST, '{}/baskets/1/'.format(TEST_API_URL), status=200, body='{}', - adding_headers={'Content-Type': 'application/json'} + adding_headers={'Content-Type': JSON} ) mock_tracker = mock.Mock() mock_tracker.resolve_context = mock.Mock(return_value={'client_id': self.TEST_CLIENT_ID}) @@ -82,7 +81,7 @@ class EcommerceApiClientTest(TestCase): httpretty.GET, '{}/baskets/1/order/'.format(TEST_API_URL), status=200, body=expected_content, - adding_headers={'Content-Type': 'application/json'}, + adding_headers={'Content-Type': JSON}, ) actual_object = ecommerce_api_client(self.user).baskets(1).order.get() self.assertEqual(actual_object, {u"result": u"Préparatoire"}) diff --git a/lms/djangoapps/commerce/tests/test_signals.py b/lms/djangoapps/commerce/tests/test_signals.py index 014d572a57..9e37edbb66 100644 --- a/lms/djangoapps/commerce/tests/test_signals.py +++ b/lms/djangoapps/commerce/tests/test_signals.py @@ -1,27 +1,37 @@ """ Tests for signal handling in commerce djangoapp. """ +import base64 +import json +from urlparse import urljoin + +import ddt from django.contrib.auth.models import AnonymousUser from django.test import TestCase from django.test.utils import override_settings - -from course_modes.models import CourseMode -import ddt +import httpretty import mock from opaque_keys.edx.keys import CourseKey +from requests import Timeout + from student.models import UNENROLL_DONE from student.tests.factories import UserFactory, CourseEnrollmentFactory - -from commerce.signals import refund_seat, send_refund_notification -from commerce.tests import TEST_PUBLIC_URL_ROOT, TEST_API_URL, TEST_API_SIGNING_KEY +from commerce.signals import (refund_seat, send_refund_notification, generate_refund_notification_body, + create_zendesk_ticket) +from commerce.tests import TEST_PUBLIC_URL_ROOT, TEST_API_URL, TEST_API_SIGNING_KEY, JSON from commerce.tests.mocks import mock_create_refund +from course_modes.models import CourseMode + +ZENDESK_URL = 'http://zendesk.example.com/' +ZENDESK_USER = 'test@example.com' +ZENDESK_API_KEY = 'abc123' @ddt.ddt @override_settings( ECOMMERCE_PUBLIC_URL_ROOT=TEST_PUBLIC_URL_ROOT, - ECOMMERCE_API_URL=TEST_API_URL, - ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY, + ECOMMERCE_API_URL=TEST_API_URL, ECOMMERCE_API_SIGNING_KEY=TEST_API_SIGNING_KEY, + ZENDESK_URL=ZENDESK_URL, ZENDESK_USER=ZENDESK_USER, ZENDESK_API_KEY=ZENDESK_API_KEY ) class TestRefundSignal(TestCase): """ @@ -197,40 +207,75 @@ class TestRefundSignal(TestCase): with self.assertRaises(NotImplementedError): send_refund_notification(self.course_enrollment, [1, 2, 3]) - @override_settings(PAYMENT_SUPPORT_EMAIL='payment@example.com') - @mock.patch('commerce.signals.EmailMultiAlternatives') - def test_notification_content(self, mock_email_class): + def test_send_refund_notification(self): + """ Verify the support team is notified of the refund request. """ + + with mock.patch('commerce.signals.create_zendesk_ticket') as mock_zendesk: + refund_ids = [1, 2, 3] + send_refund_notification(self.course_enrollment, refund_ids) + body = generate_refund_notification_body(self.student, refund_ids) + mock_zendesk.assert_called_with(self.student.profile.name, self.student.email, + "[Refund] User-Requested Refund", body, ['auto_refund']) + + def _mock_zendesk_api(self, status=201): + """ Mock Zendesk's ticket creation API. """ + httpretty.register_uri(httpretty.POST, urljoin(ZENDESK_URL, '/api/v2/tickets.json'), status=status, + body='{}', content_type=JSON) + + def call_create_zendesk_ticket(self, name=u'Test user', email=u'user@example.com', subject=u'Test Ticket', + body=u'I want a refund!', tags=None): + """ Call the create_zendesk_ticket function. """ + tags = tags or [u'auto_refund'] + create_zendesk_ticket(name, email, subject, body, tags) + + @override_settings(ZENDESK_URL=ZENDESK_URL, ZENDESK_USER=None, ZENDESK_API_KEY=None) + def test_create_zendesk_ticket_no_settings(self): + """ Verify the Zendesk API is not called if the settings are not all set. """ + with mock.patch('requests.post') as mock_post: + self.call_create_zendesk_ticket() + self.assertFalse(mock_post.called) + + def test_create_zendesk_ticket_request_error(self): """ - Ensure the email sender, recipient, subject, content type, and content - are all correct. + Verify exceptions are handled appropriately if the request to the Zendesk API fails. + + We simply need to ensure the exception is not raised beyond the function. """ - # mock_email_class is the email message class/constructor. - # mock_message is the instance returned by the constructor. - # we need to make assertions regarding both. - mock_message = mock.MagicMock() - mock_email_class.return_value = mock_message + with mock.patch('requests.post', side_effect=Timeout) as mock_post: + self.call_create_zendesk_ticket() + self.assertTrue(mock_post.called) - refund_ids = [1, 2, 3] - send_refund_notification(self.course_enrollment, refund_ids) + @httpretty.activate + def test_create_zendesk_ticket(self): + """ Verify the Zendesk API is called. """ + self._mock_zendesk_api() - # check headers and text content - self.assertEqual( - mock_email_class.call_args[0], - ("[Refund] User-Requested Refund", mock.ANY, self.student.email, ['payment@example.com']), - ) - text_body = mock_email_class.call_args[0][1] - # check for a URL for each refund - for exp in [r'{0}/dashboard/refunds/{1}/'.format(TEST_PUBLIC_URL_ROOT, refund_id) - for refund_id in refund_ids]: - self.assertRegexpMatches(text_body, exp) + name = u'Test user' + email = u'user@example.com' + subject = u'Test Ticket' + body = u'I want a refund!' + tags = [u'auto_refund'] + self.call_create_zendesk_ticket(name, email, subject, body, tags) + last_request = httpretty.last_request() - # check HTML content - self.assertEqual(mock_message.attach_alternative.call_args[0], (mock.ANY, "text/html")) - html_body = mock_message.attach_alternative.call_args[0][0] - # check for a link to each refund - for exp in [r'a href="{0}/dashboard/refunds/{1}/"'.format(TEST_PUBLIC_URL_ROOT, refund_id) - for refund_id in refund_ids]: - self.assertRegexpMatches(html_body, exp) + # Verify the headers + expected = { + 'content-type': JSON, + 'Authorization': 'Basic ' + base64.b64encode( + '{user}/token:{pwd}'.format(user=ZENDESK_USER, pwd=ZENDESK_API_KEY)) + } + self.assertDictContainsSubset(expected, last_request.headers) - # make sure we actually SEND the message too. - self.assertTrue(mock_message.send.called) + # Verify the content + expected = { + u'ticket': { + u'requester': { + u'name': name, + u'email': email + }, + u'subject': subject, + u'comment': {u'body': body}, + u'tags': [u'LMS'] + tags + } + } + self.assertDictEqual(json.loads(last_request.body), expected)