From 658cd5c62ecb093ff5aeef9383eb4146dee5ab93 Mon Sep 17 00:00:00 2001 From: Ayub <muhammadayubkhan6@gmail.com> Date: Fri, 16 Aug 2019 22:05:35 +0500 Subject: [PATCH] BOM-70 (#21327) * Update Financial Assistance logic Use the zendesk proxy app instead of the unsupported zendesk library. * Move to pre-fetching the group IDs. Rather than making extra requests to zendesk to list all groups and find a specific group ID. Just make a pre-filled list of group IDs for the groups we care about. When a group name is passed in, it is checked against this list and the ticket is created in the correct group so the right people can respond to it. --- cms/envs/common.py | 4 + common/djangoapps/util/views.py | 232 +----------------- lms/djangoapps/courseware/tests/test_views.py | 28 +-- lms/djangoapps/courseware/views/views.py | 42 +--- lms/envs/common.py | 4 + .../zendesk_proxy/tests/test_utils.py | 40 ++- .../core/djangoapps/zendesk_proxy/utils.py | 104 ++++++-- requirements/edx/base.in | 1 - requirements/edx/base.txt | 5 +- requirements/edx/development.txt | 1 - requirements/edx/testing.txt | 1 - 11 files changed, 154 insertions(+), 308 deletions(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index 8257d1f71cb..9c094e3b029 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1844,6 +1844,10 @@ ZENDESK_USER = None ZENDESK_API_KEY = None ZENDESK_CUSTOM_FIELDS = {} ZENDESK_OAUTH_ACCESS_TOKEN = '' +# A mapping of string names to Zendesk Group IDs +# To get the IDs of your groups you can go to +# {zendesk_url}/api/v2/groups.json +ZENDESK_GROUP_ID_MAPPING = {} ############## Settings for Completion API ######################### diff --git a/common/djangoapps/util/views.py b/common/djangoapps/util/views.py index 16592505363..68822649e2d 100644 --- a/common/djangoapps/util/views.py +++ b/common/djangoapps/util/views.py @@ -7,11 +7,9 @@ from functools import wraps import calc import crum -import zendesk from django.conf import settings from django.contrib.auth.decorators import login_required -from django.core.cache import caches -from django.http import Http404, HttpResponse, HttpResponseForbidden +from django.http import Http404, HttpResponse, HttpResponseForbidden, HttpResponseServerError from django.views.decorators.csrf import requires_csrf_token from django.views.defaults import server_error from opaque_keys import InvalidKeyError @@ -20,8 +18,6 @@ from six.moves import map import track.views from edxmako.shortcuts import render_to_response -from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers -from student.models import CourseEnrollment from student.roles import GlobalStaff log = logging.getLogger(__name__) @@ -168,232 +164,6 @@ def calculate(request): return HttpResponse(json.dumps({'result': str(result)})) -class _ZendeskApi(object): - - CACHE_PREFIX = 'ZENDESK_API_CACHE' - CACHE_TIMEOUT = 60 * 60 - - def __init__(self): - """ - Instantiate the Zendesk API. - - All of `ZENDESK_URL`, `ZENDESK_USER`, and `ZENDESK_API_KEY` must be set - in `django.conf.settings`. - """ - self._zendesk_instance = zendesk.Zendesk( - settings.ZENDESK_URL, - settings.ZENDESK_USER, - settings.ZENDESK_API_KEY, - use_api_token=True, - api_version=2, - # As of 2012-05-08, Zendesk is using a CA that is not - # installed on our servers - client_args={"disable_ssl_certificate_validation": True} - ) - - def create_ticket(self, ticket): - """ - Create the given `ticket` in Zendesk. - - The ticket should have the format specified by the zendesk package. - """ - ticket_url = self._zendesk_instance.create_ticket(data=ticket) - return zendesk.get_id_from_url(ticket_url) - - def update_ticket(self, ticket_id, update): - """ - Update the Zendesk ticket with id `ticket_id` using the given `update`. - - The update should have the format specified by the zendesk package. - """ - self._zendesk_instance.update_ticket(ticket_id=ticket_id, data=update) - - def get_group(self, name): - """ - Find the Zendesk group named `name`. Groups are cached for - CACHE_TIMEOUT seconds. - - If a matching group exists, it is returned as a dictionary - with the format specifed by the zendesk package. - - Otherwise, returns None. - """ - cache = caches['default'] - cache_key = '{prefix}_group_{name}'.format(prefix=self.CACHE_PREFIX, name=name) - cached = cache.get(cache_key) - if cached: - return cached - groups = self._zendesk_instance.list_groups()['groups'] - for group in groups: - if group['name'] == name: - cache.set(cache_key, group, self.CACHE_TIMEOUT) - return group - return None - - -def _get_zendesk_custom_field_context(request, **kwargs): - """ - Construct a dictionary of data that can be stored in Zendesk custom fields. - """ - context = {} - - course_id = request.POST.get("course_id") - if not course_id: - return context - - context["course_id"] = course_id - if not request.user.is_authenticated: - return context - - enrollment = CourseEnrollment.get_enrollment(request.user, CourseKey.from_string(course_id)) - if enrollment and enrollment.is_active: - context["enrollment_mode"] = enrollment.mode - - enterprise_learner_data = kwargs.get('learner_data', None) - if enterprise_learner_data: - enterprise_customer_name = enterprise_learner_data[0]['enterprise_customer']['name'] - context["enterprise_customer_name"] = enterprise_customer_name - - return context - - -def _format_zendesk_custom_fields(context): - """ - Format the data in `context` for compatibility with the Zendesk API. - Ignore any keys that have not been configured in `ZENDESK_CUSTOM_FIELDS`. - """ - custom_fields = [] - for key, val, in settings.ZENDESK_CUSTOM_FIELDS.items(): - if key in context: - custom_fields.append({"id": val, "value": context[key]}) - - return custom_fields - - -def _record_feedback_in_zendesk( - realname, - email, - subject, - details, - tags, - additional_info, - group_name=None, - require_update=False, - support_email=None, - custom_fields=None -): - """ - Create a new user-requested Zendesk ticket. - - Once created, the ticket will be updated with a private comment containing - additional information from the browser and server, such as HTTP headers - and user state. Returns a boolean value indicating whether ticket creation - was successful, regardless of whether the private comment update succeeded. - - If `group_name` is provided, attaches the ticket to the matching Zendesk group. - - If `require_update` is provided, returns False when the update does not - succeed. This allows using the private comment to add necessary information - which the user will not see in followup emails from support. - - If `custom_fields` is provided, submits data to those fields in Zendesk. - """ - zendesk_api = _ZendeskApi() - - additional_info_string = ( - u"Additional information:\n\n" + - u"\n".join(u"%s: %s" % (key, value) for (key, value) in additional_info.items() if value is not None) - ) - - # Tag all issues with LMS to distinguish channel in Zendesk; requested by student support team - zendesk_tags = list(tags.values()) + ["LMS"] - - # Per edX support, we would like to be able to route feedback items by site via tagging - current_site_name = configuration_helpers.get_value("SITE_NAME") - if current_site_name: - current_site_name = current_site_name.replace(".", "_") - zendesk_tags.append("site_name_{site}".format(site=current_site_name)) - - new_ticket = { - "ticket": { - "requester": {"name": realname, "email": email}, - "subject": subject, - "comment": {"body": details}, - "tags": zendesk_tags - } - } - - if custom_fields: - new_ticket["ticket"]["custom_fields"] = custom_fields - - group = None - if group_name is not None: - group = zendesk_api.get_group(group_name) - if group is not None: - new_ticket['ticket']['group_id'] = group['id'] - if support_email is not None: - # If we do not include the `recipient` key here, Zendesk will default to using its default reply - # email address when support agents respond to tickets. By setting the `recipient` key here, - # we can ensure that WL site users are responded to via the correct Zendesk support email address. - new_ticket['ticket']['recipient'] = support_email - try: - ticket_id = zendesk_api.create_ticket(new_ticket) - if group_name is not None and group is None: - # Support uses Zendesk groups to track tickets. In case we - # haven't been able to correctly group this ticket, log its ID - # so it can be found later. - log.warning('Unable to find group named %s for Zendesk ticket with ID %s.', group_name, ticket_id) - except zendesk.ZendeskError: - log.exception("Error creating Zendesk ticket") - return False - - # Additional information is provided as a private update so the information - # is not visible to the user. - ticket_update = {"ticket": {"comment": {"public": False, "body": additional_info_string}}} - try: - zendesk_api.update_ticket(ticket_id, ticket_update) - except zendesk.ZendeskError: - log.exception("Error updating Zendesk ticket with ID %s.", ticket_id) - # The update is not strictly necessary, so do not indicate - # failure to the user unless it has been requested with - # `require_update`. - if require_update: - return False - return True - - -def get_feedback_form_context(request): - """ - Extract the submitted form fields to be used as a context for - feedback submission. - """ - context = {} - - context["subject"] = request.POST["subject"] - context["details"] = request.POST["details"] - context["tags"] = dict( - [(tag, request.POST[tag]) for tag in ["issue_type", "course_id"] if request.POST.get(tag)] - ) - - context["additional_info"] = {} - - if request.user.is_authenticated: - context["realname"] = request.user.profile.name - context["email"] = request.user.email - context["additional_info"]["username"] = request.user.username - else: - context["realname"] = request.POST["name"] - context["email"] = request.POST["email"] - - for header, pretty in [("HTTP_REFERER", "Page"), ("HTTP_USER_AGENT", "Browser"), ("REMOTE_ADDR", "Client IP"), - ("SERVER_NAME", "Host")]: - context["additional_info"][pretty] = request.META.get(header) - - context["support_email"] = configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) - - return context - - def info(request): ''' Info page (link from main header) ''' # pylint: disable=unused-argument diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 4741995b2d9..1f549823ea2 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -38,6 +38,7 @@ from xblock.fields import Scope, String import courseware.views.views as views import shoppingcart + from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory from course_modes.models import CourseMode from course_modes.tests.factories import CourseModeFactory @@ -484,11 +485,9 @@ class ViewsTestCase(ModuleStoreTestCase): def assert_enrollment_link_present(self, is_anonymous): """ Prepare ecommerce checkout data and assert if the ecommerce link is contained in the response. - Arguments: is_anonymous(bool): Tell the method to use an anonymous user or the logged in one. _id(bool): Tell the method to either expect an id in the href or not. - """ sku = 'TEST123' configuration = CommerceConfiguration.objects.create(checkout_on_ecommerce_service=True) @@ -606,7 +605,6 @@ class ViewsTestCase(ModuleStoreTestCase): """ Visits the about page for `course_id` and tests that both the text "Classes End", as well as the specified `expected_end_text`, is present on the page. - If `expected_end_text` is None, verifies that the about page *does not* contain the text "Classes End". """ @@ -846,8 +844,8 @@ class ViewsTestCase(ModuleStoreTestCase): url = reverse('submit_financial_assistance_request') return self.client.post(url, json.dumps(data), content_type='application/json') - @patch.object(views, '_record_feedback_in_zendesk') - def test_submit_financial_assistance_request(self, mock_record_feedback): + @patch.object(views, 'create_zendesk_ticket', return_value=200) + def test_submit_financial_assistance_request(self, mock_create_zendesk_ticket): username = self.user.username course = six.text_type(self.course_key) legal_name = 'Jesse Pinkman' @@ -871,10 +869,12 @@ class ViewsTestCase(ModuleStoreTestCase): response = self._submit_financial_assistance_form(data) self.assertEqual(response.status_code, 204) - __, __, ticket_subject, __, tags, additional_info = mock_record_feedback.call_args[0] - mocked_kwargs = mock_record_feedback.call_args[1] - group_name = mocked_kwargs['group_name'] - require_update = mocked_kwargs['require_update'] + __, __, ticket_subject, __ = mock_create_zendesk_ticket.call_args[0] + mocked_kwargs = mock_create_zendesk_ticket.call_args[1] + group_name = mocked_kwargs['group'] + tags = mocked_kwargs['tags'] + additional_info = mocked_kwargs['additional_info'] + private_comment = '\n'.join(list(additional_info.values())) for info in (country, income, reason_for_applying, goals, effort, username, legal_name, course): self.assertIn(info, private_comment) @@ -891,10 +891,9 @@ class ViewsTestCase(ModuleStoreTestCase): self.assertDictContainsSubset({'course_id': course}, tags) self.assertIn('Client IP', additional_info) self.assertEqual(group_name, 'Financial Assistance') - self.assertTrue(require_update) - @patch.object(views, '_record_feedback_in_zendesk', return_value=False) - def test_zendesk_submission_failed(self, _mock_record_feedback): + @patch.object(views, 'create_zendesk_ticket', return_value=500) + def test_zendesk_submission_failed(self, _mock_create_zendesk_ticket): response = self._submit_financial_assistance_form({ 'username': self.user.username, 'course': six.text_type(self.course.id), @@ -1029,7 +1028,6 @@ class BaseDueDateTests(ModuleStoreTestCase): def set_up_course(self, **course_kwargs): """ Create a stock course with a specific due date. - :param course_kwargs: All kwargs are passed to through to the :class:`CourseFactory` """ course = CourseFactory.create(**course_kwargs) @@ -1158,7 +1156,6 @@ class StartDateTests(ModuleStoreTestCase): def set_up_course(self): """ Create a stock course with a specific due date. - :param course_kwargs: All kwargs are passed to through to the :class:`CourseFactory` """ course = CourseFactory.create(start=datetime(2013, 9, 16, 7, 17, 28)) @@ -1291,7 +1288,6 @@ class ProgressPageTests(ProgressPageBaseTests): def test_unenrolled_student_progress_for_credit_course(self, default_store): """ Test that student progress page does not break while checking for an unenrolled student. - Scenario: When instructor checks the progress of a student who is not enrolled in credit course. It should return 200 response. """ @@ -2746,7 +2742,6 @@ class TestIndexViewWithVerticalPositions(ModuleStoreTestCase): def test_vertical_positions(self, input_position, expected_position): """ Tests the following cases: - * Load first position when negative position inputted. * Load first position when 0/-0 position inputted. * Load given position when 0 < input_position <= num_positions_available. @@ -2971,7 +2966,6 @@ class TestRenderXBlockSelfPaced(TestRenderXBlock): class TestIndexViewCrawlerStudentStateWrites(SharedModuleStoreTestCase): """ Ensure that courseware index requests do not trigger student state writes. - This is to prevent locking issues that have caused latency spikes in the courseware_studentmodule table when concurrent requests each try to update the same rows for sequence, section, and course positions. diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index f9be6c3c15f..479cb972864 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -94,6 +94,7 @@ from openedx.core.djangoapps.programs.utils import ProgramMarketingDataExtender from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.util.user_messages import PageLevelMessages +from openedx.core.djangoapps.zendesk_proxy.utils import create_zendesk_ticket from openedx.core.djangolib.markup import HTML, Text from openedx.features.course_duration_limits.access import generate_course_expired_fragment from openedx.features.course_experience import ( @@ -112,7 +113,7 @@ from track import segment from util.cache import cache, cache_if_anonymous from util.db import outer_atomic from util.milestones_helpers import get_prerequisite_courses_display -from util.views import _record_feedback_in_zendesk, ensure_valid_course_key, ensure_valid_usage_key +from util.views import ensure_valid_course_key, ensure_valid_usage_key from xmodule.course_module import COURSE_VISIBILITY_PUBLIC, COURSE_VISIBILITY_PUBLIC_OUTLINE from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem @@ -287,9 +288,7 @@ def jump_to_id(request, course_id, module_id): def jump_to(_request, course_id, location): """ Show the page that contains a specific location. - If the location is invalid or not in any class, return a 404. - Otherwise, delegates to the index view to figure out whether this user has access, and what they should see. """ @@ -314,7 +313,6 @@ def jump_to(_request, course_id, location): def course_info(request, course_id): """ Display the course's info.html, or 404 if there is no such course. - Assumes the course_id is in a valid format. """ # TODO: LEARNER-611: This can be deleted with Course Info removal. The new @@ -709,7 +707,6 @@ class CourseTabView(EdxFragmentView): def syllabus(request, course_id): """ Display the course's syllabus.html, or 404 if there is no such course. - Assumes the course_id is in a valid format. """ @@ -739,15 +736,12 @@ def registered_for_course(course, user): class EnrollStaffView(View): """ Displays view for registering in the course to a global staff user. - User can either choose to 'Enroll' or 'Don't Enroll' in the course. Enroll: Enrolls user in course and redirects to the courseware. Don't Enroll: Redirects user to course about page. - Arguments: - request : HTTP request - course_id : course id - Returns: - RedirectResponse """ @@ -978,9 +972,7 @@ def progress(request, course_id, student_id=None): def _progress(request, course_key, student_id): """ Unwrapped version of "progress". - User progress. We show the grade bar and every problem score. - Course staff are allowed to see the progress of students in their class. """ @@ -1109,13 +1101,11 @@ def _certificate_message(student, course, enrollment_mode): def _get_cert_data(student, course, enrollment_mode, course_grade=None): """Returns students course certificate related data. - Arguments: student (User): Student for whom certificate to retrieve. course (Course): Course object for which certificate data to retrieve. enrollment_mode (String): Course mode in which student is enrolled. course_grade (CourseGrade): Student's course grade record. - Returns: returns dict if course certificate is available else None. """ @@ -1134,14 +1124,11 @@ def _get_cert_data(student, course, enrollment_mode, course_grade=None): def _credit_course_requirements(course_key, student): """Return information about which credit requirements a user has satisfied. - Arguments: course_key (CourseKey): Identifier for the course. student (User): Currently logged in user. - Returns: dict if the credit eligibility enabled and it is a credit course and the user is enrolled in either verified or credit mode, and None otherwise. - """ # If credit eligibility is not enabled or this is not a credit course, # short-circuit and return `None`. This indicates that credit requirements @@ -1198,7 +1185,6 @@ def _course_home_redirect_enabled(): """ Return True value if user needs to be redirected to course home based on value of `ENABLE_MKTG_SITE` and `ENABLE_COURSE_HOME_REDIRECT feature` flags - Returns: boolean True or False """ if configuration_helpers.get_value( @@ -1318,16 +1304,13 @@ def get_static_tab_fragment(request, course, tab): def get_course_lti_endpoints(request, course_id): """ View that, given a course_id, returns the a JSON object that enumerates all of the LTI endpoints for that course. - The LTI 2.0 result service spec at http://www.imsglobal.org/lti/ltiv2p0/uml/purl.imsglobal.org/vocab/lis/v2/outcomes/Result/service.html says "This specification document does not prescribe a method for discovering the endpoint URLs." This view function implements one way of discovering these endpoints, returning a JSON array when accessed. - Arguments: request (django request object): the HTTP request object that triggered this view function course_id (unicode): id associated with the course - Returns: (django response object): HTTP response. 404 if course is not found, otherwise 200 with JSON body. """ @@ -1404,12 +1387,10 @@ def course_survey(request, course_id): def is_course_passed(student, course, course_grade=None): """ check user's course passing status. return True if passed - Arguments: student : user object course : course object course_grade (CourseGrade) : contains student grade details. - Returns: returns bool value """ @@ -1423,24 +1404,19 @@ def is_course_passed(student, course, course_grade=None): @require_POST def generate_user_cert(request, course_id): """Start generating a new certificate for the user. - Certificate generation is allowed if: * The user has passed the course, and * The user does not already have a pending/completed certificate. - Note that if an error occurs during certificate generation (for example, if the queue is down), then we simply mark the certificate generation task status as "error" and re-run the task with a management command. To students, the certificate will appear to be "generating" until it is re-run. - Args: request (HttpRequest): The POST request to this view. course_id (unicode): The identifier for the course. - Returns: HttpResponse: 200 on success, 400 if a new certificate cannot be generated. - """ if not request.user.is_authenticated: @@ -1491,13 +1467,11 @@ def generate_user_cert(request, course_id): def _track_successful_certificate_generation(user_id, course_id): """ Track a successful certificate generation event. - Arguments: user_id (str): The ID of the user generating the certificate. course_id (CourseKey): Identifier for the course. Returns: None - """ event_name = 'edx.bi.user.certificate.generate' segment.track(user_id, event_name, { @@ -1632,7 +1606,7 @@ def financial_assistance_request(request): # Thrown if fields are missing return HttpResponseBadRequest(u'The field {} is required.'.format(text_type(err))) - zendesk_submitted = _record_feedback_in_zendesk( + zendesk_submitted = create_zendesk_ticket( legal_name, email, u'Financial assistance request for learner {username} in course {course_name}'.format( @@ -1640,12 +1614,12 @@ def financial_assistance_request(request): course_name=course.display_name ), u'Financial Assistance Request', - {'course_id': course_id}, + tags={'course_id': course_id}, # Send the application as additional info on the ticket so # that it is not shown when support replies. This uses # OrderedDict so that information is presented in the right # order. - OrderedDict(( + additional_info=OrderedDict(( ('Username', username), ('Full Name', legal_name), ('Course ID', course_id), @@ -1657,11 +1631,9 @@ def financial_assistance_request(request): (FA_EFFORT_LABEL, '\n' + effort + '\n\n'), ('Client IP', ip_address), )), - group_name='Financial Assistance', - require_update=True + group='Financial Assistance', ) - - if not zendesk_submitted: + if not (zendesk_submitted == 200 or zendesk_submitted == 201): # The call to Zendesk failed. The frontend will display a # message to the user. return HttpResponse(status=status.HTTP_500_INTERNAL_SERVER_ERROR) diff --git a/lms/envs/common.py b/lms/envs/common.py index 2f56f92d103..c5bbf56a40e 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1329,6 +1329,10 @@ ZENDESK_USER = '' ZENDESK_API_KEY = '' ZENDESK_CUSTOM_FIELDS = {} ZENDESK_OAUTH_ACCESS_TOKEN = '' +# A mapping of string names to Zendesk Group IDs +# To get the IDs of your groups you can go to +# {zendesk_url}/api/v2/groups.json +ZENDESK_GROUP_ID_MAPPING = {} ##### EMBARGO ##### EMBARGO_SITE_REDIRECT_URL = None diff --git a/openedx/core/djangoapps/zendesk_proxy/tests/test_utils.py b/openedx/core/djangoapps/zendesk_proxy/tests/test_utils.py index d0c7223aba7..16bc039e364 100644 --- a/openedx/core/djangoapps/zendesk_proxy/tests/test_utils.py +++ b/openedx/core/djangoapps/zendesk_proxy/tests/test_utils.py @@ -3,10 +3,14 @@ Tests of Zendesk interaction utility functions """ from __future__ import absolute_import -import ddt + +import json +from collections import OrderedDict + from django.test.utils import override_settings -from mock import MagicMock, patch +import ddt +from mock import MagicMock, patch from openedx.core.djangoapps.zendesk_proxy.utils import create_zendesk_ticket from openedx.core.lib.api.test_utils import ApiTestCase @@ -14,7 +18,8 @@ from openedx.core.lib.api.test_utils import ApiTestCase @ddt.ddt @override_settings( ZENDESK_URL="https://www.superrealurlsthataredefinitelynotfake.com", - ZENDESK_OAUTH_ACCESS_TOKEN="abcdefghijklmnopqrstuvwxyz1234567890" + ZENDESK_OAUTH_ACCESS_TOKEN="abcdefghijklmnopqrstuvwxyz1234567890", + ZENDESK_GROUP_ID_MAPPING={"Financial Assistance": 123}, ) class TestUtils(ApiTestCase): def setUp(self): @@ -61,3 +66,32 @@ class TestUtils(ApiTestCase): body=self.request_data['body'], ) self.assertEqual(status_code, 500) + + def test_financial_assistant_ticket(self): + """ Test Financial Assistent request ticket. """ + ticket_creation_response_data = { + "ticket": { + "id": 35436, + "subject": "My printer is on fire!", + } + } + response_text = json.dumps(ticket_creation_response_data) + with patch('requests.post', return_value=MagicMock(status_code=200, text=response_text)): + with patch('requests.put', return_value=MagicMock(status_code=200)): + status_code = create_zendesk_ticket( + requester_name=self.request_data['name'], + requester_email=self.request_data['email'], + subject=self.request_data['subject'], + body=self.request_data['body'], + group='Financial Assistance', + additional_info=OrderedDict( + ( + ('Username', 'test'), + ('Full Name', 'Legal Name'), + ('Course ID', 'course_key'), + ('Annual Household Income', 'Income'), + ('Country', 'Country'), + ) + ), + ) + self.assertEqual(status_code, 200) diff --git a/openedx/core/djangoapps/zendesk_proxy/utils.py b/openedx/core/djangoapps/zendesk_proxy/utils.py index d905b58b82b..5f0c50ef895 100644 --- a/openedx/core/djangoapps/zendesk_proxy/utils.py +++ b/openedx/core/djangoapps/zendesk_proxy/utils.py @@ -3,29 +3,44 @@ Utility functions for zendesk interaction. """ from __future__ import absolute_import + import json import logging -from six.moves.urllib.parse import urljoin # pylint: disable=import-error -from django.conf import settings import requests +from django.conf import settings from rest_framework import status +from six.moves.urllib.parse import urljoin # pylint: disable=import-error log = logging.getLogger(__name__) -def create_zendesk_ticket(requester_name, requester_email, subject, body, custom_fields=None, uploads=None, tags=None): +def _std_error_message(details, payload): + """Internal helper to standardize error message. This allows for simpler splunk alerts.""" + return u'zendesk_proxy action required\n{}\nNo ticket created for payload {}'.format(details, payload) + + +def _get_request_headers(): + return { + 'content-type': 'application/json', + 'Authorization': u"Bearer {}".format(settings.ZENDESK_OAUTH_ACCESS_TOKEN), + } + + +def create_zendesk_ticket( + requester_name, + requester_email, + subject, + body, + group=None, + custom_fields=None, + uploads=None, + tags=None, + additional_info=None +): """ Create a Zendesk ticket via API. - - Note that we do this differently in other locations (lms/djangoapps/commerce/signals.py and - common/djangoapps/util/views.py). Both of those callers use basic auth, and should be switched over to this oauth - implementation once the immediate pressures of zendesk_proxy are resolved. """ - def _std_error_message(details, payload): - """Internal helper to standardize error message. This allows for simpler splunk alerts.""" - return u'zendesk_proxy action required\n{}\nNo ticket created for payload {}'.format(details, payload) - if tags: # Remove duplicates from tags list tags = list(set(tags)) @@ -53,15 +68,20 @@ def create_zendesk_ticket(requester_name, requester_email, subject, body, custom log.error(_std_error_message("zendesk not configured", payload)) return status.HTTP_503_SERVICE_UNAVAILABLE + if group: + if group in settings.ZENDESK_GROUP_ID_MAPPING: + group_id = settings.ZENDESK_GROUP_ID_MAPPING[group] + data['ticket']['group_id'] = group_id + else: + msg = u"Group ID not found for group {}. Please update ZENDESK_GROUP_ID_MAPPING".format(group) + log.error(_std_error_message(msg, payload)) + return status.HTTP_400_BAD_REQUEST + # Set the request parameters url = urljoin(settings.ZENDESK_URL, '/api/v2/tickets.json') - headers = { - 'content-type': 'application/json', - 'Authorization': u"Bearer {}".format(settings.ZENDESK_OAUTH_ACCESS_TOKEN), - } try: - response = requests.post(url, data=payload, headers=headers) + response = requests.post(url, data=payload, headers=_get_request_headers()) # Check for HTTP codes other than 201 (Created) if response.status_code == status.HTTP_201_CREATED: @@ -73,7 +93,59 @@ def create_zendesk_ticket(requester_name, requester_email, subject, body, custom payload ) ) + if additional_info: + try: + ticket = response.json()['ticket'] + except (ValueError, KeyError): + log.error( + _std_error_message( + u"Got an unexpected response from zendesk api. Can't" + u" get the ticket number to add extra info. {}".format(additional_info), + response.content + ) + ) + return status.HTTP_400_BAD_REQUEST + return post_additional_info_as_comment(ticket['id'], additional_info) + return response.status_code except Exception: # pylint: disable=broad-except log.exception(_std_error_message('Internal server error', payload)) return status.HTTP_500_INTERNAL_SERVER_ERROR + + +def post_additional_info_as_comment(ticket_id, additional_info): + """ + Post the Additional Provided as a comment, So that it is only visible + to management and not students. + """ + additional_info_string = ( + u"Additional information:\n\n" + + u"\n".join(u"%s: %s" % (key, value) for (key, value) in additional_info.items() if value is not None) + ) + + data = { + 'ticket': { + 'comment': { + 'body': additional_info_string, + 'public': False + } + } + } + + url = urljoin(settings.ZENDESK_URL, 'api/v2/tickets/{}.json'.format(ticket_id)) + + try: + response = requests.put(url, data=json.dumps(data), headers=_get_request_headers()) + if response.status_code == 200: + log.debug(u'Successfully created comment for ticket {}'.format(ticket_id)) + else: + log.error( + _std_error_message( + u'Unexpected response: {} - {}'.format(response.status_code, response.content), + data + ) + ) + return response.status_code + except Exception: # pylint: disable=broad-except + log.exception(_std_error_message('Internal server error', data)) + return status.HTTP_500_INTERNAL_SERVER_ERROR diff --git a/requirements/edx/base.in b/requirements/edx/base.in index f21e44b7fbd..53ec488bf8c 100644 --- a/requirements/edx/base.in +++ b/requirements/edx/base.in @@ -154,6 +154,5 @@ web-fragments # Provides the ability to render fragments o XBlock # Courseware component architecture xblock-utils # Provides utilities used by the Discussion XBlock xss-utils # https://github.com/edx/edx-platform/pull/20633 Fix XSS via Translations -zendesk # Python API for the Zendesk customer support system geoip2==2.9.0 # Python API for the GeoIP web services and databases edx-bulk-grades # LMS REST API for managing bulk grading operations diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 4fea86d6bc5..1343f41eaba 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -135,7 +135,7 @@ glob2==0.7 gunicorn==19.9.0 help-tokens==1.0.4 html5lib==1.0.1 -httplib2==0.13.1 # via zendesk +httplib2==0.13.1 idna==2.8 inflection==0.3.1 # via drf-yasg ipaddress==1.0.22 @@ -223,7 +223,7 @@ scipy==1.2.1 semantic-version==2.6.0 # via edx-drf-extensions shapely==1.6.4.post2 shortuuid==0.5.0 # via edx-django-oauth2-provider -simplejson==3.16.0 # via mailsnake, sailthru-client, zendesk +simplejson==3.16.0 # via mailsnake, sailthru-client singledispatch==3.4.0.3 six==1.11.0 slumber==0.7.1 # via edx-bulk-grades, edx-enterprise, edx-rest-api-client @@ -255,7 +255,6 @@ xblock-utils==1.2.2 xblock==1.2.3 xmlsec==1.3.3 # via python3-saml xss-utils==0.1.1 -zendesk==1.1.1 # The following packages are considered to be unsafe in a requirements file: # setuptools==41.1.0 # via fs, lazy, python-levenshtein diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 8ccd31688e1..0a4e0fedd28 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -349,7 +349,6 @@ xblock==1.2.3 xmlsec==1.3.3 xmltodict==0.12.0 xss-utils==0.1.1 -zendesk==1.1.1 zipp==0.5.2 # The following packages are considered to be unsafe in a requirements file: diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 84028c8bf15..602127fd022 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -335,7 +335,6 @@ xblock==1.2.3 xmlsec==1.3.3 xmltodict==0.12.0 # via moto xss-utils==0.1.1 -zendesk==1.1.1 zipp==0.5.2 # via importlib-metadata # The following packages are considered to be unsafe in a requirements file: -- GitLab