diff --git a/common/djangoapps/util/tests/test_zendesk.py b/common/djangoapps/util/tests/test_submit_feedback.py
similarity index 75%
rename from common/djangoapps/util/tests/test_zendesk.py
rename to common/djangoapps/util/tests/test_submit_feedback.py
index 51d06a92ed..b50c97f0ac 100644
--- a/common/djangoapps/util/tests/test_zendesk.py
+++ b/common/djangoapps/util/tests/test_submit_feedback.py
@@ -15,8 +15,9 @@ import mock
@mock.patch.dict("django.conf.settings.MITX_FEATURES", {"ENABLE_FEEDBACK_SUBMISSION": True})
@override_settings(ZENDESK_URL="dummy", ZENDESK_USER="dummy", ZENDESK_API_KEY="dummy")
+@mock.patch("util.views.dog_stats_api")
@mock.patch("util.views._ZendeskApi", autospec=True)
-class SubmitFeedbackViaZendeskTest(TestCase):
+class SubmitFeedbackTest(TestCase):
def setUp(self):
"""Set up data for the test case"""
self._request_factory = RequestFactory()
@@ -26,18 +27,18 @@ class SubmitFeedbackViaZendeskTest(TestCase):
username="test",
profile__name="Test User"
)
- # This contains a tag to ensure that tags are submitted correctly
+ # This contains an issue_type to ensure that tags are submitted correctly
self._anon_fields = {
"email": "test@edx.org",
"name": "Test User",
"subject": "a subject",
"details": "some details",
- "tag": "a tag"
+ "issue_type": "test_issue"
}
- # This does not contain a tag to ensure that tag is optional
+ # This does not contain an issue_type to ensure that it is optional
self._auth_fields = {"subject": "a subject", "details": "some details"}
- def _test_request(self, user, fields):
+ def _build_and_run_request(self, user, fields):
"""
Generate a request and invoke the view, returning the response.
@@ -51,9 +52,9 @@ class SubmitFeedbackViaZendeskTest(TestCase):
HTTP_USER_AGENT="test_user_agent"
)
req.user = user
- return views.submit_feedback_via_zendesk(req)
+ return views.submit_feedback(req)
- def _assert_bad_request(self, response, field, zendesk_mock_class):
+ def _assert_bad_request(self, response, field, zendesk_mock_class, datadog_mock):
"""
Assert that the given `response` contains correct failure data.
@@ -67,8 +68,9 @@ class SubmitFeedbackViaZendeskTest(TestCase):
self.assertTrue("error" in resp_json)
# There should be absolutely no interaction with Zendesk
self.assertFalse(zendesk_mock_class.return_value.mock_calls)
+ self.assertFalse(datadog_mock.mock_calls)
- def _test_bad_request_omit_field(self, user, fields, omit_field, zendesk_mock_class):
+ def _test_bad_request_omit_field(self, user, fields, omit_field, zendesk_mock_class, datadog_mock):
"""
Invoke the view with a request missing a field and assert correctness.
@@ -79,10 +81,10 @@ class SubmitFeedbackViaZendeskTest(TestCase):
have been invoked.
"""
filtered_fields = {k: v for (k, v) in fields.items() if k != omit_field}
- resp = self._test_request(user, filtered_fields)
- self._assert_bad_request(resp, omit_field, zendesk_mock_class)
+ resp = self._build_and_run_request(user, filtered_fields)
+ self._assert_bad_request(resp, omit_field, zendesk_mock_class, datadog_mock)
- def _test_bad_request_empty_field(self, user, fields, empty_field, zendesk_mock_class):
+ def _test_bad_request_empty_field(self, user, fields, empty_field, zendesk_mock_class, datadog_mock):
"""
Invoke the view with an empty field and assert correctness.
@@ -94,8 +96,8 @@ class SubmitFeedbackViaZendeskTest(TestCase):
"""
altered_fields = fields.copy()
altered_fields[empty_field] = ""
- resp = self._test_request(user, altered_fields)
- self._assert_bad_request(resp, empty_field, zendesk_mock_class)
+ resp = self._build_and_run_request(user, altered_fields)
+ self._assert_bad_request(resp, empty_field, zendesk_mock_class, datadog_mock)
def _test_success(self, user, fields):
"""
@@ -105,30 +107,30 @@ class SubmitFeedbackViaZendeskTest(TestCase):
`fields` in the POST body. The response should have a 200 (success)
status code.
"""
- resp = self._test_request(user, fields)
+ resp = self._build_and_run_request(user, fields)
self.assertEqual(resp.status_code, 200)
- def test_bad_request_anon_user_no_name(self, zendesk_mock_class):
+ def test_bad_request_anon_user_no_name(self, zendesk_mock_class, datadog_mock):
"""Test a request from an anonymous user not specifying `name`."""
- self._test_bad_request_omit_field(self._anon_user, self._anon_fields, "name", zendesk_mock_class)
- self._test_bad_request_empty_field(self._anon_user, self._anon_fields, "name", zendesk_mock_class)
+ self._test_bad_request_omit_field(self._anon_user, self._anon_fields, "name", zendesk_mock_class, datadog_mock)
+ self._test_bad_request_empty_field(self._anon_user, self._anon_fields, "name", zendesk_mock_class, datadog_mock)
- def test_bad_request_anon_user_no_email(self, zendesk_mock_class):
+ def test_bad_request_anon_user_no_email(self, zendesk_mock_class, datadog_mock):
"""Test a request from an anonymous user not specifying `email`."""
- self._test_bad_request_omit_field(self._anon_user, self._anon_fields, "email", zendesk_mock_class)
- self._test_bad_request_empty_field(self._anon_user, self._anon_fields, "email", zendesk_mock_class)
+ self._test_bad_request_omit_field(self._anon_user, self._anon_fields, "email", zendesk_mock_class, datadog_mock)
+ self._test_bad_request_empty_field(self._anon_user, self._anon_fields, "email", zendesk_mock_class, datadog_mock)
- def test_bad_request_anon_user_no_subject(self, zendesk_mock_class):
+ def test_bad_request_anon_user_no_subject(self, zendesk_mock_class, datadog_mock):
"""Test a request from an anonymous user not specifying `subject`."""
- self._test_bad_request_omit_field(self._anon_user, self._anon_fields, "subject", zendesk_mock_class)
- self._test_bad_request_empty_field(self._anon_user, self._anon_fields, "subject", zendesk_mock_class)
+ self._test_bad_request_omit_field(self._anon_user, self._anon_fields, "subject", zendesk_mock_class, datadog_mock)
+ self._test_bad_request_empty_field(self._anon_user, self._anon_fields, "subject", zendesk_mock_class, datadog_mock)
- def test_bad_request_anon_user_no_details(self, zendesk_mock_class):
+ def test_bad_request_anon_user_no_details(self, zendesk_mock_class, datadog_mock):
"""Test a request from an anonymous user not specifying `details`."""
- self._test_bad_request_omit_field(self._anon_user, self._anon_fields, "details", zendesk_mock_class)
- self._test_bad_request_empty_field(self._anon_user, self._anon_fields, "details", zendesk_mock_class)
+ self._test_bad_request_omit_field(self._anon_user, self._anon_fields, "details", zendesk_mock_class, datadog_mock)
+ self._test_bad_request_empty_field(self._anon_user, self._anon_fields, "details", zendesk_mock_class, datadog_mock)
- def test_valid_request_anon_user(self, zendesk_mock_class):
+ def test_valid_request_anon_user(self, zendesk_mock_class, datadog_mock):
"""
Test a valid request from an anonymous user.
@@ -138,14 +140,14 @@ class SubmitFeedbackViaZendeskTest(TestCase):
zendesk_mock_instance = zendesk_mock_class.return_value
zendesk_mock_instance.create_ticket.return_value = 42
self._test_success(self._anon_user, self._anon_fields)
- expected_calls = [
+ expected_zendesk_calls = [
mock.call.create_ticket(
{
"ticket": {
"requester": {"name": "Test User", "email": "test@edx.org"},
"subject": "a subject",
"comment": {"body": "some details"},
- "tags": ["a tag"]
+ "tags": ["test_issue"]
}
}
),
@@ -164,19 +166,23 @@ class SubmitFeedbackViaZendeskTest(TestCase):
}
)
]
- self.assertEqual(zendesk_mock_instance.mock_calls, expected_calls)
+ expected_datadog_calls = [
+ mock.call.increment(views.DATADOG_FEEDBACK_METRIC, tags=["issue_type:test_issue"])
+ ]
+ self.assertEqual(zendesk_mock_instance.mock_calls, expected_zendesk_calls)
+ self.assertEqual(datadog_mock.mock_calls, expected_datadog_calls)
- def test_bad_request_auth_user_no_subject(self, zendesk_mock_class):
+ def test_bad_request_auth_user_no_subject(self, zendesk_mock_class, datadog_mock):
"""Test a request from an authenticated user not specifying `subject`."""
- self._test_bad_request_omit_field(self._auth_user, self._auth_fields, "subject", zendesk_mock_class)
- self._test_bad_request_empty_field(self._auth_user, self._auth_fields, "subject", zendesk_mock_class)
+ self._test_bad_request_omit_field(self._auth_user, self._auth_fields, "subject", zendesk_mock_class, datadog_mock)
+ self._test_bad_request_empty_field(self._auth_user, self._auth_fields, "subject", zendesk_mock_class, datadog_mock)
- def test_bad_request_auth_user_no_details(self, zendesk_mock_class):
+ def test_bad_request_auth_user_no_details(self, zendesk_mock_class, datadog_mock):
"""Test a request from an authenticated user not specifying `details`."""
- self._test_bad_request_omit_field(self._auth_user, self._auth_fields, "details", zendesk_mock_class)
- self._test_bad_request_empty_field(self._auth_user, self._auth_fields, "details", zendesk_mock_class)
+ self._test_bad_request_omit_field(self._auth_user, self._auth_fields, "details", zendesk_mock_class, datadog_mock)
+ self._test_bad_request_empty_field(self._auth_user, self._auth_fields, "details", zendesk_mock_class, datadog_mock)
- def test_valid_request_auth_user(self, zendesk_mock_class):
+ def test_valid_request_auth_user(self, zendesk_mock_class, datadog_mock):
"""
Test a valid request from an authenticated user.
@@ -186,7 +192,7 @@ class SubmitFeedbackViaZendeskTest(TestCase):
zendesk_mock_instance = zendesk_mock_class.return_value
zendesk_mock_instance.create_ticket.return_value = 42
self._test_success(self._auth_user, self._auth_fields)
- expected_calls = [
+ expected_zendesk_calls = [
mock.call.create_ticket(
{
"ticket": {
@@ -213,20 +219,25 @@ class SubmitFeedbackViaZendeskTest(TestCase):
}
)
]
- self.assertEqual(zendesk_mock_instance.mock_calls, expected_calls)
+ expected_datadog_calls = [
+ mock.call.increment(views.DATADOG_FEEDBACK_METRIC, tags=[])
+ ]
+ self.assertEqual(zendesk_mock_instance.mock_calls, expected_zendesk_calls)
+ self.assertEqual(datadog_mock.mock_calls, expected_datadog_calls)
- def test_get_request(self, zendesk_mock_class):
+ def test_get_request(self, zendesk_mock_class, datadog_mock):
"""Test that a GET results in a 405 even with all required fields"""
req = self._request_factory.get("/submit_feedback", data=self._anon_fields)
req.user = self._anon_user
- resp = views.submit_feedback_via_zendesk(req)
+ resp = views.submit_feedback(req)
self.assertEqual(resp.status_code, 405)
self.assertIn("Allow", resp)
self.assertEqual(resp["Allow"], "POST")
# There should be absolutely no interaction with Zendesk
self.assertFalse(zendesk_mock_class.mock_calls)
+ self.assertFalse(datadog_mock.mock_calls)
- def test_zendesk_error_on_create(self, zendesk_mock_class):
+ def test_zendesk_error_on_create(self, zendesk_mock_class, datadog_mock):
"""
Test Zendesk returning an error on ticket creation.
@@ -235,11 +246,15 @@ class SubmitFeedbackViaZendeskTest(TestCase):
err = ZendeskError(msg="", error_code=404)
zendesk_mock_instance = zendesk_mock_class.return_value
zendesk_mock_instance.create_ticket.side_effect = err
- resp = self._test_request(self._anon_user, self._anon_fields)
+ resp = self._build_and_run_request(self._anon_user, self._anon_fields)
self.assertEqual(resp.status_code, 500)
self.assertFalse(resp.content)
+ expected_datadog_calls = [
+ mock.call.increment(views.DATADOG_FEEDBACK_METRIC, tags=["issue_type:test_issue"])
+ ]
+ self.assertEqual(datadog_mock.mock_calls, expected_datadog_calls)
- def test_zendesk_error_on_update(self, zendesk_mock_class):
+ def test_zendesk_error_on_update(self, zendesk_mock_class, datadog_mock):
"""
Test for Zendesk returning an error on ticket update.
@@ -250,20 +265,24 @@ class SubmitFeedbackViaZendeskTest(TestCase):
err = ZendeskError(msg="", error_code=500)
zendesk_mock_instance = zendesk_mock_class.return_value
zendesk_mock_instance.update_ticket.side_effect = err
- resp = self._test_request(self._anon_user, self._anon_fields)
+ resp = self._build_and_run_request(self._anon_user, self._anon_fields)
self.assertEqual(resp.status_code, 200)
+ expected_datadog_calls = [
+ mock.call.increment(views.DATADOG_FEEDBACK_METRIC, tags=["issue_type:test_issue"])
+ ]
+ self.assertEqual(datadog_mock.mock_calls, expected_datadog_calls)
@mock.patch.dict("django.conf.settings.MITX_FEATURES", {"ENABLE_FEEDBACK_SUBMISSION": False})
- def test_not_enabled(self, zendesk_mock_class):
+ def test_not_enabled(self, zendesk_mock_class, datadog_mock):
"""
Test for Zendesk submission not enabled in `settings`.
We should raise Http404.
"""
with self.assertRaises(Http404):
- self._test_request(self._anon_user, self._anon_fields)
+ self._build_and_run_request(self._anon_user, self._anon_fields)
- def test_zendesk_not_configured(self, zendesk_mock_class):
+ def test_zendesk_not_configured(self, zendesk_mock_class, datadog_mock):
"""
Test for Zendesk not fully configured in `settings`.
@@ -273,7 +292,7 @@ class SubmitFeedbackViaZendeskTest(TestCase):
def test_case(missing_config):
with mock.patch(missing_config, None):
with self.assertRaises(Exception):
- self._test_request(self._anon_user, self._anon_fields)
+ self._build_and_run_request(self._anon_user, self._anon_fields)
test_case("django.conf.settings.ZENDESK_URL")
test_case("django.conf.settings.ZENDESK_USER")
diff --git a/common/djangoapps/util/views.py b/common/djangoapps/util/views.py
index d0aa0dc680..3bf760c66f 100644
--- a/common/djangoapps/util/views.py
+++ b/common/djangoapps/util/views.py
@@ -12,6 +12,7 @@ from django.core.validators import ValidationError, validate_email
from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseNotAllowed, HttpResponseServerError
from django.shortcuts import redirect
from django_future.csrf import ensure_csrf_cookie
+from dogapi import dog_stats_api
from mitxmako.shortcuts import render_to_response, render_to_string
from urllib import urlencode
import zendesk
@@ -73,11 +74,63 @@ class _ZendeskApi(object):
self._zendesk_instance.update_ticket(ticket_id=ticket_id, data=update)
-def submit_feedback_via_zendesk(request):
+def _record_feedback_in_zendesk(realname, email, subject, details, tags, additional_info):
"""
Create a new user-requested Zendesk ticket.
- If Zendesk submission is not enabled, any request will raise `Http404`.
+ 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.
+ """
+ zendesk_api = _ZendeskApi()
+
+ additional_info_string = (
+ "Additional information:\n\n" +
+ "\n".join("%s: %s" % (key, value) for (key, value) in additional_info.items() if value is not None)
+ )
+
+ zendesk_tags = list(tags.values())
+ new_ticket = {
+ "ticket": {
+ "requester": {"name": realname, "email": email},
+ "subject": subject,
+ "comment": {"body": details},
+ "tags": zendesk_tags
+ }
+ }
+ try:
+ ticket_id = zendesk_api.create_ticket(new_ticket)
+ except zendesk.ZendeskError as err:
+ log.error("Error creating Zendesk ticket: %s", str(err))
+ 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 as err:
+ log.error("Error updating Zendesk ticket: %s", str(err))
+ # The update is not strictly necessary, so do not indicate failure to the user
+ pass
+
+ return True
+
+
+DATADOG_FEEDBACK_METRIC = "lms_feedback_submissions"
+
+
+def _record_feedback_in_datadog(tags):
+ datadog_tags = ["{k}:{v}".format(k=k, v=v) for k, v in tags.items()]
+ dog_stats_api.increment(DATADOG_FEEDBACK_METRIC, tags=datadog_tags)
+
+
+def submit_feedback(request):
+ """
+ Create a new user-requested ticket, currently implemented with Zendesk.
+
+ If feedback submission is not enabled, any request will raise `Http404`.
If any configuration parameter (`ZENDESK_URL`, `ZENDESK_USER`, or
`ZENDESK_API_KEY`) is missing, any request will raise an `Exception`.
The request must be a POST request specifying `subject` and `details`.
@@ -85,12 +138,9 @@ def submit_feedback_via_zendesk(request):
`email`. If the user is authenticated, the `name` and `email` will be
populated from the user's information. If any required parameter is
missing, a 400 error will be returned indicating which field is missing and
- providing an error message. If Zendesk returns any error on ticket
- creation, a 500 error will be returned with no body. 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. Whether or not the update succeeds, if the user's ticket is
- successfully created, an empty successful response (200) will be returned.
+ providing an error message. If Zendesk ticket creation fails, 500 error
+ will be returned with no body; if ticket creation succeeds, an empty
+ successful response (200) will be returned.
"""
if not settings.MITX_FEATURES.get('ENABLE_FEEDBACK_SUBMISSION', False):
raise Http404()
@@ -124,9 +174,9 @@ def submit_feedback_via_zendesk(request):
subject = request.POST["subject"]
details = request.POST["details"]
- tags = []
- if "tag" in request.POST:
- tags = [request.POST["tag"]]
+ tags = dict(
+ [(tag, request.POST[tag]) for tag in ["issue_type"] if tag in request.POST]
+ )
if request.user.is_authenticated():
realname = request.user.profile.name
@@ -143,38 +193,10 @@ def submit_feedback_via_zendesk(request):
for header in ["HTTP_REFERER", "HTTP_USER_AGENT"]:
additional_info[header] = request.META.get(header)
- zendesk_api = _ZendeskApi()
+ success = _record_feedback_in_zendesk(realname, email, subject, details, tags, additional_info)
+ _record_feedback_in_datadog(tags)
- additional_info_string = (
- "Additional information:\n\n" +
- "\n".join("%s: %s" % (key, value) for (key, value) in additional_info.items() if value is not None)
- )
-
- new_ticket = {
- "ticket": {
- "requester": {"name": realname, "email": email},
- "subject": subject,
- "comment": {"body": details},
- "tags": tags
- }
- }
- try:
- ticket_id = zendesk_api.create_ticket(new_ticket)
- except zendesk.ZendeskError as err:
- log.error("Error creating Zendesk ticket: %s", str(err))
- return HttpResponse(status=500)
-
- # 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 as err:
- log.error("Error updating Zendesk ticket: %s", str(err))
- # The update is not strictly necessary, so do not indicate failure to the user
- pass
-
- return HttpResponse()
+ return HttpResponse(status=(200 if success else 500))
def info(request):
diff --git a/lms/templates/help_modal.html b/lms/templates/help_modal.html
index facd74e9ae..9d849beb1a 100644
--- a/lms/templates/help_modal.html
+++ b/lms/templates/help_modal.html
@@ -64,7 +64,7 @@ discussion_link = get_discussion_link(course) if course else None
Tell us the details*
Include error messages, steps which lead to the issue, etc
-
@@ -114,9 +114,9 @@ discussion_link = get_discussion_link(course) if course else None
$("#feedback_success_wrapper").css("display", "none");
$("#help_wrapper").css("display", "block");
});
- showFeedback = function(e, tag, title) {
+ showFeedback = function(e, issue_type, title) {
$("#help_wrapper").css("display", "none");
- $("#feedback_form input[name='tag']").val(tag);
+ $("#feedback_form input[name='issue_type']").val(issue_type);
$("#feedback_form_wrapper").css("display", "block");
$("#feedback_form_wrapper header").html("" + title + " [^/]*)$', 'student.views.reactivation_email'),