From 131ca13a50f7eeb58bf86ede5356a8ed9f3fa3bc Mon Sep 17 00:00:00 2001 From: Afzal Wali Date: Fri, 11 Nov 2016 19:54:07 +0500 Subject: [PATCH] Added the templates for email subject, email body and the changes in submit_feedback function that now conditionally sends emails. Suggestions. --- .../util/tests/test_submit_feedback.py | 31 +++++ common/djangoapps/util/views.py | 129 +++++++++++------- .../emails/contact_us_feedback_email_body.txt | 12 ++ .../contact_us_feedback_email_subject.txt | 1 + 4 files changed, 125 insertions(+), 48 deletions(-) create mode 100644 common/templates/emails/contact_us_feedback_email_body.txt create mode 100644 common/templates/emails/contact_us_feedback_email_subject.txt diff --git a/common/djangoapps/util/tests/test_submit_feedback.py b/common/djangoapps/util/tests/test_submit_feedback.py index ae09f618ff01..077765e1d78d 100644 --- a/common/djangoapps/util/tests/test_submit_feedback.py +++ b/common/djangoapps/util/tests/test_submit_feedback.py @@ -5,6 +5,7 @@ from django.test import TestCase from django.test.client import RequestFactory from django.test.utils import override_settings +from smtplib import SMTPException from student.tests.factories import UserFactory from util import views from zendesk import ZendeskError @@ -14,11 +15,25 @@ from student.tests.test_configuration_overrides import fake_get_value +def fake_support_backend_values(name, default=None): # pylint: disable=unused-argument + """ + Method for getting configuration override values for support email. + """ + config_dict = { + "CONTACT_FORM_SUBMISSION_BACKEND": "email", + "email_from_address": "support_from@example.com", + } + return config_dict[name] + + @mock.patch.dict("django.conf.settings.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 SubmitFeedbackTest(TestCase): + """ + Class to test the submit_feedback function in views. + """ def setUp(self): """Set up data for the test case""" super(SubmitFeedbackTest, self).setUp() @@ -356,3 +371,19 @@ def test_case(missing_config): test_case("django.conf.settings.ZENDESK_URL") test_case("django.conf.settings.ZENDESK_USER") test_case("django.conf.settings.ZENDESK_API_KEY") + + @mock.patch("openedx.core.djangoapps.site_configuration.helpers.get_value", fake_support_backend_values) + def test_valid_request_over_email(self, zendesk_mock_class, datadog_mock): # pylint: disable=unused-argument + with mock.patch("util.views.send_mail") as patched_send_email: + resp = self._build_and_run_request(self._anon_user, self._anon_fields) + self.assertEqual(patched_send_email.call_count, 1) + self.assertIn(self._anon_fields["email"], str(patched_send_email.call_args)) + self.assertEqual(resp.status_code, 200) + + @mock.patch("openedx.core.djangoapps.site_configuration.helpers.get_value", fake_support_backend_values) + def test_exception_request_over_email(self, zendesk_mock_class, datadog_mock): # pylint: disable=unused-argument + with mock.patch("util.views.send_mail", side_effect=SMTPException) as patched_send_email: + resp = self._build_and_run_request(self._anon_user, self._anon_fields) + self.assertEqual(patched_send_email.call_count, 1) + self.assertIn(self._anon_fields["email"], str(patched_send_email.call_args)) + self.assertEqual(resp.status_code, 500) diff --git a/common/djangoapps/util/views.py b/common/djangoapps/util/views.py index 6493186fffde..9a2b57af3405 100644 --- a/common/djangoapps/util/views.py +++ b/common/djangoapps/util/views.py @@ -1,31 +1,37 @@ import json import logging +from smtplib import SMTPException import sys from functools import wraps from django.conf import settings from django.contrib.auth.decorators import login_required from django.core.cache import caches +from django.core.mail import send_mail from django.core.validators import ValidationError, validate_email from django.views.decorators.csrf import requires_csrf_token from django.views.defaults import server_error from django.http import (Http404, HttpResponse, HttpResponseNotAllowed, HttpResponseServerError, HttpResponseForbidden) import dogstats_wrapper as dog_stats_api -from edxmako.shortcuts import render_to_response import zendesk -from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers - import calc -import track.views from opaque_keys import InvalidKeyError + from opaque_keys.edx.keys import CourseKey +from edxmako.shortcuts import render_to_response, render_to_string +from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers +import track.views from student.roles import GlobalStaff log = logging.getLogger(__name__) +DATADOG_FEEDBACK_METRIC = "lms_feedback_submissions" +SUPPORT_BACKEND_ZENDESK = "support_ticket" +SUPPORT_BACKEND_EMAIL = "email" + def ensure_valid_course_key(view_func): """ @@ -281,17 +287,47 @@ def _record_feedback_in_zendesk( return True -DATADOG_FEEDBACK_METRIC = "lms_feedback_submissions" - - def _record_feedback_in_datadog(tags): datadog_tags = [u"{k}:{v}".format(k=k, v=v) for k, v in tags.items()] dog_stats_api.increment(DATADOG_FEEDBACK_METRIC, tags=datadog_tags) +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 tag in request.POST] + ) + + 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 submit_feedback(request): """ - Create a new user-requested ticket, currently implemented with Zendesk. + Create a Zendesk ticket or if not available, send an email with the + feedback form fields. If feedback submission is not enabled, any request will raise `Http404`. If any configuration parameter (`ZENDESK_URL`, `ZENDESK_USER`, or @@ -309,74 +345,71 @@ def submit_feedback(request): raise Http404() if request.method != "POST": return HttpResponseNotAllowed(["POST"]) - if ( - not settings.ZENDESK_URL or - not settings.ZENDESK_USER or - not settings.ZENDESK_API_KEY - ): - raise Exception("Zendesk enabled but not configured") def build_error_response(status_code, field, err_msg): return HttpResponse(json.dumps({"field": field, "error": err_msg}), status=status_code) - additional_info = {} - required_fields = ["subject", "details"] + if not request.user.is_authenticated(): required_fields += ["name", "email"] + required_field_errs = { "subject": "Please provide a subject.", "details": "Please provide details.", "name": "Please provide your name.", "email": "Please provide a valid e-mail.", } - for field in required_fields: if field not in request.POST or not request.POST[field]: return build_error_response(400, field, required_field_errs[field]) - subject = request.POST["subject"] - details = request.POST["details"] - tags = dict( - [(tag, request.POST[tag]) for tag in ["issue_type", "course_id"] if tag in request.POST] - ) - - if request.user.is_authenticated(): - realname = request.user.profile.name - email = request.user.email - additional_info["username"] = request.user.username - else: - realname = request.POST["name"] - email = request.POST["email"] + if not request.user.is_authenticated(): try: - validate_email(email) + validate_email(request.POST["email"]) except ValidationError: return build_error_response(400, "email", required_field_errs["email"]) - for header, pretty in [ - ("HTTP_REFERER", "Page"), - ("HTTP_USER_AGENT", "Browser"), - ("REMOTE_ADDR", "Client IP"), - ("SERVER_NAME", "Host") - ]: - additional_info[pretty] = request.META.get(header) + success = False + context = get_feedback_form_context(request) + support_backend = configuration_helpers.get_value('CONTACT_FORM_SUBMISSION_BACKEND', SUPPORT_BACKEND_ZENDESK) - success = _record_feedback_in_zendesk( - realname, - email, - subject, - details, - tags, - additional_info, - support_email=configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) - ) - _record_feedback_in_datadog(tags) + if support_backend == SUPPORT_BACKEND_EMAIL: + try: + send_mail( + subject=render_to_string('emails/contact_us_feedback_email_subject.txt', context), + message=render_to_string('emails/contact_us_feedback_email_body.txt', context), + from_email=context["support_email"], + recipient_list=[context["support_email"]], + fail_silently=False + ) + success = True + except SMTPException: + log.exception('Error sending feedback to contact_us email address.') + success = False + + else: + if not settings.ZENDESK_URL or not settings.ZENDESK_USER or not settings.ZENDESK_API_KEY: + raise Exception("Zendesk enabled but not configured") + + success = _record_feedback_in_zendesk( + context["realname"], + context["email"], + context["subject"], + context["details"], + context["tags"], + context["additional_info"], + support_email=context["support_email"] + ) + + _record_feedback_in_datadog(context["tags"]) return HttpResponse(status=(200 if success else 500)) def info(request): ''' Info page (link from main header) ''' + # pylint: disable=unused-argument return render_to_response("info.html", {}) diff --git a/common/templates/emails/contact_us_feedback_email_body.txt b/common/templates/emails/contact_us_feedback_email_body.txt new file mode 100644 index 000000000000..167ebd855041 --- /dev/null +++ b/common/templates/emails/contact_us_feedback_email_body.txt @@ -0,0 +1,12 @@ +<%! from django.utils.translation import ugettext as _ %> +${_("Feedback Form")} + +${_("Email: {email}").format(email=email)} +${_("Full Name: {realname}").format(realname=realname)} +${_("Inquiry Type: {inquiry_type}").format(inquiry_type=subject)} +${_("Message: {message}").format(message=details)} +${_("Tags: {tags}").format(tags=tags)} +${_("Additional Info:")} +% for additional_info_key in additional_info.keys(): + ${additional_info_key} : ${additional_info[additional_info_key]} +% endfor diff --git a/common/templates/emails/contact_us_feedback_email_subject.txt b/common/templates/emails/contact_us_feedback_email_subject.txt new file mode 100644 index 000000000000..dca9f1b0c50f --- /dev/null +++ b/common/templates/emails/contact_us_feedback_email_subject.txt @@ -0,0 +1 @@ +<%! from django.utils.translation import ugettext as _ %>${_("Feedback from user")} \ No newline at end of file