Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add datadog reporting to the feedback submission mechanism

Browse Source
This commit is contained in:
Greg Price
2013-06-07 11:25:36 -04:00
parent 976280e693
commit 17647abd5c
4 changed files with 136 additions and 95 deletions
Download Patch File Download Diff File Expand all files Collapse all files

117
common/djangoapps/util/tests/test_zendesk.py → common/djangoapps/util/tests/test_submit_feedback.py
View File

@@ -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")

106
common/djangoapps/util/views.py
View File

@@ -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):

6
lms/templates/help_modal.html
View File

@@ -64,7 +64,7 @@ discussion_link = get_discussion_link(course) if course else None
<label data-field="details">Tell us the details*
<span class="tip">Include error messages, steps which lead to the issue, etc</span></label>
<textarea name="details"></textarea>
<input name="tag" type="hidden">
<input name="issue_type" type="hidden">
<div class="submit">
<input name="submit" type="submit" value="Submit">
</div>
@@ -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("<h2>" + title + "</h2><hr>");
e.preventDefault();

2
lms/urls.py
View File

@@ -111,7 +111,7 @@ if not settings.MITX_FEATURES["USE_CUSTOM_THEME"]:
# Favicon
(r'^favicon\.ico$', 'django.views.generic.simple.redirect_to', {'url': '/static/images/favicon.ico'}),
url(r'^submit_feedback$', 'util.views.submit_feedback_via_zendesk'),
url(r'^submit_feedback$', 'util.views.submit_feedback'),
# TODO: These urls no longer work. They need to be updated before they are re-enabled
# url(r'^reactivate/(?P<key>[^/]*)$', 'student.views.reactivation_email'),