From 692e5d777ac20403b2f0c9fa9ef964b8b530de8f Mon Sep 17 00:00:00 2001 From: Bill DeRusha Date: Mon, 7 Dec 2015 10:24:29 -0500 Subject: [PATCH] Update copy for HCFA Update Zendesk ticket to correctly add group. Add logic to hide new audit certs --- common/djangoapps/student/views.py | 7 +- common/djangoapps/util/views.py | 64 +++++++++++++-- common/templates/course_modes/choose.html | 11 +-- lms/djangoapps/certificates/models.py | 11 +++ lms/djangoapps/courseware/tests/test_views.py | 30 +++---- lms/djangoapps/courseware/views.py | 79 +++++++++---------- lms/djangoapps/mobile_api/users/tests.py | 5 ++ .../_dashboard_certificate_information.html | 3 +- .../financial-assistance.html | 4 +- 9 files changed, 140 insertions(+), 74 deletions(-) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 506dc069a6..c3896022f2 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -295,6 +295,7 @@ def _cert_info(user, course_overview, cert_status, course_mode): # pylint: disa CertificateStatuses.downloadable: 'ready', CertificateStatuses.notpassing: 'notpassing', CertificateStatuses.restricted: 'restricted', + CertificateStatuses.auditing: 'auditing', } default_status = 'processing' @@ -309,7 +310,7 @@ def _cert_info(user, course_overview, cert_status, course_mode): # pylint: disa if cert_status is None: return default_info - is_hidden_status = cert_status['status'] in ('unavailable', 'processing', 'generating', 'notpassing') + is_hidden_status = cert_status['status'] in ('unavailable', 'processing', 'generating', 'notpassing', 'auditing') if course_overview.certificates_display_behavior == 'early_no_info' and is_hidden_status: return {} @@ -325,7 +326,7 @@ def _cert_info(user, course_overview, cert_status, course_mode): # pylint: disa 'can_unenroll': status not in DISABLE_UNENROLL_CERT_STATES, } - if (status in ('generating', 'ready', 'notpassing', 'restricted') and + if (status in ('generating', 'ready', 'notpassing', 'restricted', 'auditing') and course_overview.end_of_course_survey_url is not None): status_dict.update({ 'show_survey_button': True, @@ -373,7 +374,7 @@ def _cert_info(user, course_overview, cert_status, course_mode): # pylint: disa cert_status['download_url'] ) - if status in ('generating', 'ready', 'notpassing', 'restricted'): + if status in ('generating', 'ready', 'notpassing', 'restricted', 'auditing'): if 'grade' not in cert_status: # Note: as of 11/20/2012, we know there are students in this state-- cs169.1x, # who need to be regraded (we weren't tracking 'notpassing' at first). diff --git a/common/djangoapps/util/views.py b/common/djangoapps/util/views.py index 99083db7fc..aac618862d 100644 --- a/common/djangoapps/util/views.py +++ b/common/djangoapps/util/views.py @@ -4,6 +4,7 @@ import sys from functools import wraps from django.conf import settings +from django.core.cache import caches from django.core.validators import ValidationError, validate_email from django.views.decorators.csrf import requires_csrf_token from django.views.defaults import server_error @@ -115,6 +116,10 @@ def calculate(request): class _ZendeskApi(object): + + CACHE_PREFIX = 'ZENDESK_API_CACHE' + CACHE_TIMEOUT = 60 * 60 + def __init__(self): """ Instantiate the Zendesk API. @@ -150,8 +155,39 @@ class _ZendeskApi(object): """ 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. -def _record_feedback_in_zendesk(realname, email, subject, details, tags, additional_info): + 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 _record_feedback_in_zendesk( + realname, + email, + subject, + details, + tags, + additional_info, + group_name=None, + require_update=False +): """ Create a new user-requested Zendesk ticket. @@ -159,6 +195,12 @@ def _record_feedback_in_zendesk(realname, email, subject, details, tags, additio 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. """ zendesk_api = _ZendeskApi() @@ -184,8 +226,18 @@ def _record_feedback_in_zendesk(realname, email, subject, details, tags, additio "tags": zendesk_tags } } + 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'] try: ticket_id = zendesk_api.create_ticket(new_ticket) + if 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 @@ -196,10 +248,12 @@ def _record_feedback_in_zendesk(realname, email, subject, details, tags, additio try: zendesk_api.update_ticket(ticket_id, ticket_update) except zendesk.ZendeskError: - log.exception("Error updating Zendesk ticket") - # The update is not strictly necessary, so do not indicate failure to the user - pass - + 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 diff --git a/common/templates/course_modes/choose.html b/common/templates/course_modes/choose.html index d6564a9a6d..ddddcd0390 100644 --- a/common/templates/course_modes/choose.html +++ b/common/templates/course_modes/choose.html @@ -143,15 +143,15 @@ from django.core.urlresolvers import reverse
-

${_("Earn an Honor Certificate")}

+

${_("Audit This Course")}

-

${_("Take this course for free and have complete access to all the course material, activities, tests, and forums. Please note that learners who earn a passing grade will earn a certificate in this course.")}

+

${_("Audit this course for free and have complete access to all the course material, activities, tests, and forums.")}

@@ -163,9 +163,10 @@ from django.core.urlresolvers import reverse
-

${_("Audit This Course")}

+

${_("Audit This Course (No Certificate)")}

-

${_("Audit this course for free and have complete access to all the course material, activities, tests, and forums. Please note that this track does not offer a certificate for learners who earn a passing grade.")}

+ ## Translators: b_start notes the beginning of a section of text bolded for emphasis, and b_end marks the end of the bolded text. +

${_("Audit this course for free and have complete access to all the course material, activities, tests, and forums. {b_start}Please note that this track does not offer a certificate for learners who earn a passing grade.{b_end}".format(**b_tag_kwargs))}

diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index 20d70c42db..839d1912a7 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -86,6 +86,7 @@ class CertificateStatuses(object): regenerating = 'regenerating' restricted = 'restricted' unavailable = 'unavailable' + auditing = 'auditing' class CertificateSocialNetworks(object): @@ -305,10 +306,20 @@ def certificate_status_for_student(student, course_id): } if generated_certificate.grade: cert_status['grade'] = generated_certificate.grade + + if generated_certificate.mode == 'audit': + course_mode_slugs = [mode.slug for mode in CourseMode.modes_for_course(course_id)] + # Short term fix to make sure old audit users with certs still see their certs + # only do this if there if no honor mode + if 'honor' not in course_mode_slugs: + cert_status['status'] = CertificateStatuses.auditing + return cert_status + if generated_certificate.status == CertificateStatuses.downloadable: cert_status['download_url'] = generated_certificate.download_url return cert_status + except GeneratedCertificate.DoesNotExist: pass return {'status': CertificateStatuses.unavailable, 'mode': GeneratedCertificate.MODES.honor} diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index f2cfe41c25..ef87ea0844 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -536,31 +536,32 @@ class ViewsTestCase(ModuleStoreTestCase): self.assertEqual(response.status_code, 204) __, ___, ticket_subject, ticket_body, tags, additional_info = mock_record_feedback.call_args[0] - for info in (country, income, reason_for_applying, goals, effort): - self.assertIn(info, ticket_body) - self.assertIn('This user HAS NOT allowed this content to be used for edX marketing purposes.', ticket_body) + mocked_kwargs = mock_record_feedback.call_args[1] + group_name = mocked_kwargs['group_name'] + require_update = mocked_kwargs['require_update'] + private_comment = '\n'.join(additional_info.values()) + for info in (country, income, reason_for_applying, goals, effort, username, legal_name, course): + self.assertIn(info, private_comment) + + self.assertEqual(additional_info['Allowed for marketing purposes'], 'No') self.assertEqual( ticket_subject, - 'Financial assistance request for user {username} in course {course}'.format( + 'Financial assistance request for learner {username} in course {course}'.format( username=username, - course=course + course=self.course.display_name ) ) - self.assertDictContainsSubset( - { - 'issue_type': 'Financial Assistance', - 'course_id': course - }, - tags - ) + 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): response = self._submit_financial_assistance_form({ 'username': self.user.username, - 'course': '', + 'course': unicode(self.course.id), 'name': '', 'email': '', 'country': '', @@ -574,7 +575,8 @@ class ViewsTestCase(ModuleStoreTestCase): @ddt.data( ({}, 400), - ({'username': 'wwhite'}, 403) + ({'username': 'wwhite'}, 403), + ({'username': 'dummy', 'course': 'bad course ID'}, 400) ) @ddt.unpack def test_submit_financial_assistance_errors(self, data, status): diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 72f15070c5..8742e3f4e8 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -7,6 +7,7 @@ import json import textwrap import urllib +from collections import OrderedDict from datetime import datetime from django.utils.translation import ugettext as _ @@ -1414,20 +1415,22 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True): # Translators: "percent_sign" is the symbol "%". "platform_name" is a # string identifying the name of this installation, such as "edX". FINANCIAL_ASSISTANCE_HEADER = _( - '{platform_name} now offers financial assistance for learners who want to earn verified certificates but' + '{platform_name} now offers financial assistance for learners who want to earn Verified Certificates but' ' who may not be able to pay the Verified Certificate fee. Eligible learners receive 90{percent_sign} off' ' the Verified Certificate fee for a course.\nTo apply for financial assistance, enroll in the' ' audit track for a course that offers Verified Certificates, and then complete this application.' - ' Note that you must complete a separate application for each course you take.' + ' Note that you must complete a separate application for each course you take.\n We will use this' + 'information to evaluate your application for financial assistance and to further develop our' + 'financial assistance program.' ).format( percent_sign="%", platform_name=settings.PLATFORM_NAME ).split('\n') -FA_INCOME_LABEL = _('Annual Income') +FA_INCOME_LABEL = _('Annual Household Income') FA_REASON_FOR_APPLYING_LABEL = _( - 'Tell us about your current financial situation, including any unusual circumstances.' + 'Tell us about your current financial situation.' ) FA_GOALS_LABEL = _( 'Tell us about your learning or professional goals. How will a Verified Certificate in' @@ -1435,7 +1438,7 @@ FA_GOALS_LABEL = _( ) FA_EFFORT_LABEL = _( 'Tell us about your plans for this course. What steps will you take to help you complete' - ' the course work a receive a certificate?' + ' the course work and receive a certificate?' ) FA_SHORT_ANSWER_INSTRUCTIONS = _('Use between 250 and 500 words or so in your response.') @@ -1461,6 +1464,7 @@ def financial_assistance_request(request): return HttpResponseForbidden() course_id = data['course'] + course = modulestore().get_course(CourseKey.from_string(course_id)) legal_name = data['name'] email = data['email'] country = data['country'] @@ -1473,52 +1477,40 @@ def financial_assistance_request(request): except ValueError: # Thrown if JSON parsing fails return HttpResponseBadRequest('Could not parse request JSON.') + except InvalidKeyError: + # Thrown if course key parsing fails + return HttpResponseBadRequest('Could not parse request course key.') except KeyError as err: # Thrown if fields are missing return HttpResponseBadRequest('The field {} is required.'.format(err.message)) - ticket_body = textwrap.dedent( - ''' - Annual Income: {income} - Country: {country} - - {reason_label} - {separator} - {reason_for_applying} - - {goals_label} - {separator} - {goals} - - {effort_label} - {separator} - {effort} - - This user {allowed_for_marketing} allowed this content to be used for edX marketing purposes. - '''.format( - income=income, - country=country, - reason_label=FA_REASON_FOR_APPLYING_LABEL, - reason_for_applying=reason_for_applying, - goals_label=FA_GOALS_LABEL, - goals=goals, - effort_label=FA_EFFORT_LABEL, - effort=effort, - allowed_for_marketing='HAS' if marketing_permission else 'HAS NOT', - separator='=' * 16 - ) - ) - zendesk_submitted = _record_feedback_in_zendesk( legal_name, email, - 'Financial assistance request for user {username} in course {course_id}'.format( + 'Financial assistance request for learner {username} in course {course_name}'.format( username=username, - course_id=course_id + course_name=course.display_name ), - ticket_body, - {'issue_type': 'Financial Assistance', 'course_id': course_id}, - {'Client IP': ip_address} + 'Financial Assistance Request', + {'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(( + ('Username', username), + ('Full Name', legal_name), + ('Course ID', course_id), + ('Annual Household Income', income), + ('Country', country), + ('Allowed for marketing purposes', 'Yes' if marketing_permission else 'No'), + (FA_REASON_FOR_APPLYING_LABEL, '\n' + reason_for_applying + '\n\n'), + (FA_GOALS_LABEL, '\n' + goals + '\n\n'), + (FA_EFFORT_LABEL, '\n' + effort + '\n\n'), + ('Client IP', ip_address), + )), + group_name='Financial Assistance', + require_update=True ) if not zendesk_submitted: @@ -1629,7 +1621,8 @@ def financial_assistance_form(request): 'type': 'checkbox', 'required': False, 'instructions': _( - 'Annual income and personal information such as email address will not be shared.' + 'Annual income and personal information such as email address will not be shared. ' + 'Financial information will not be used for marketing purposes.' ), 'restrictions': {} } diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 37e8ec6ace..404645e639 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -205,6 +205,11 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest @patch.dict(settings.FEATURES, {'CERTIFICATES_HTML_VIEW': True}) def test_web_certificate(self): + CourseMode.objects.create( + course_id=self.course.id, + mode_display_name="Honor", + mode_slug=CourseMode.HONOR, + ) self.login_and_enroll() self.course.cert_html_view_enabled = True diff --git a/lms/templates/dashboard/_dashboard_certificate_information.html b/lms/templates/dashboard/_dashboard_certificate_information.html index bf6f716a5b..13109ea82e 100644 --- a/lms/templates/dashboard/_dashboard_certificate_information.html +++ b/lms/templates/dashboard/_dashboard_certificate_information.html @@ -27,10 +27,9 @@ else: status_css_class = 'course-status-processing' %>
- % if cert_status['status'] == 'processing':

${_("Final course details are being wrapped up at this time. Your final standing will be available shortly.")}

-% elif cert_status['status'] in ('generating', 'ready', 'notpassing', 'restricted'): +% elif cert_status['status'] in ('generating', 'ready', 'notpassing', 'restricted', 'auditing'):

${_("Your final grade:")} ${"{0:.0f}%".format(float(cert_status['grade'])*100)}. % if cert_status['status'] == 'notpassing': diff --git a/lms/templates/financial-assistance/financial-assistance.html b/lms/templates/financial-assistance/financial-assistance.html index e611f8453b..04ef3efabb 100644 --- a/lms/templates/financial-assistance/financial-assistance.html +++ b/lms/templates/financial-assistance/financial-assistance.html @@ -19,9 +19,9 @@ from edxmako.shortcuts import marketing_link

${_("A Note to Learners")}

${_("Dear edX Learner,")}

${_("EdX Financial Assistance is a program we created to give learners in all financial circumstances a chance to earn a Verified Certificate upon successful completion of an edX course.")}

-

${_("If you are interested in working toward a Verified Certificate, but cannot afford to pay the fee, please apply now. Please note space is limited.")}

+

${_("If you are interested in working toward a Verified Certificate, but cannot afford to pay the fee, please apply now. Please note financial assistance is limited.")}

${_("In order to be eligible for edX Financial Assistance, you must demonstrate that paying the Verified Certificate fee would cause you economic hardship. To apply, you will be asked to answer a few questions about why you are applying and how the Verified Certificate will benefit you.")}

-

${_("Once your application is approved, we'll email to let you know and give you instructions for how to verify your identity on edX.org; then you can start working toward completing your edX course.")}

+

${_("If your application is approved, we'll give you instructions for verifying your identity on edx.org so you can start working toward completing your edX course.")}

${_("EdX is committed to making it possible for you to take high quality courses from leading institutions regardless of your financial situation, earn a Verified Certificate, and share your success with others.")}

${_("Sincerely, Anant")}