From dd1f8346970d23ffa5aaa60fa4025c0347a36404 Mon Sep 17 00:00:00 2001 From: Clinton Blackburn Date: Thu, 15 Jun 2017 20:27:36 -0400 Subject: [PATCH] Displaying verification denial reasons on dashboard Learners now see (on the dashboard) a list of reasons why their verifications were denied. LEARNER-1486 --- common/djangoapps/student/views.py | 26 ++++++++- lms/djangoapps/verify_student/models.py | 55 +++++++++++-------- .../verify_student/tests/test_models.py | 42 ++++++-------- lms/djangoapps/verify_student/views.py | 2 +- .../_dashboard_status_verification.html | 14 ++++- 5 files changed, 87 insertions(+), 52 deletions(-) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 966324d8e6..c9d1870931 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -788,7 +788,8 @@ def dashboard(request): # Verification Attempts # Used to generate the "you must reverify for course x" banner - verification_status, verification_msg = SoftwareSecurePhotoVerification.user_status(user) + verification_status, verification_error_codes = SoftwareSecurePhotoVerification.user_status(user) + verification_errors = get_verification_error_reasons_for_display(verification_error_codes) # Gets data for midcourse reverifications, if any are necessary or have failed statuses = ["approved", "denied", "pending", "must_reverify"] @@ -866,7 +867,7 @@ def dashboard(request): 'reverifications': reverifications, 'verification_status': verification_status, 'verification_status_by_course': verify_status_by_course, - 'verification_msg': verification_msg, + 'verification_errors': verification_errors, 'show_refund_option_for': show_refund_option_for, 'block_courses': block_courses, 'denied_banner': denied_banner, @@ -898,6 +899,27 @@ def dashboard(request): return response +def get_verification_error_reasons_for_display(verification_error_codes): + verification_errors = [] + verification_error_map = { + 'photos_mismatched': _('Photos are mismatched'), + 'id_image_missing_name': _('Name missing from ID photo'), + 'id_image_missing': _('ID photo not provided'), + 'id_invalid': _('ID is invalid'), + 'user_image_not_clear': _('Learner photo is blurry'), + 'name_mismatch': _('Name on ID does not match name on account'), + 'user_image_missing': _('Learner photo not provided'), + 'id_image_not_clear': _('ID photo is blurry'), + } + + for error in verification_error_codes: + error_text = verification_error_map.get(error) + if error_text: + verification_errors.append(error_text) + + return verification_errors + + def _create_recent_enrollment_message(course_enrollments, course_modes): # pylint: disable=invalid-name """ Builds a recent course enrollment message. diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index beb8b6848e..4c36e6d6da 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -18,6 +18,7 @@ from email.utils import formatdate import pytz import requests +import six from config_models.models import ConfigurationModel from django.conf import settings from django.contrib.auth.models import User @@ -742,33 +743,43 @@ class SoftwareSecurePhotoVerification(PhotoVerification): `[{"photoIdReasons": ["Not provided"]}]` - Returns a list of error messages + Returns: + str[]: List of error messages. """ - # Translates the category names and messages into something more human readable - message_dict = { - ("photoIdReasons", "Not provided"): _("No photo ID was provided."), - ("photoIdReasons", "Text not clear"): _("We couldn't read your name from your photo ID image."), - ("generalReasons", "Name mismatch"): _("The name associated with your account and the name on your ID do not match."), - ("userPhotoReasons", "Image not clear"): _("The image of your face was not clear."), - ("userPhotoReasons", "Face out of view"): _("Your face was not visible in your self-photo."), + parsed_errors = [] + error_map = { + 'EdX name not provided': 'name_mismatch', + 'Name mismatch': 'name_mismatch', + 'Photo/ID Photo mismatch': 'photos_mismatched', + 'ID name not provided': 'id_image_missing_name', + 'Invalid Id': 'id_invalid', + 'No text': 'id_invalid', + 'Not provided': 'id_image_missing', + 'Photo hidden/No photo': 'id_image_not_clear', + 'Text not clear': 'id_image_not_clear', + 'Face out of view': 'user_image_not_clear', + 'Image not clear': 'user_image_not_clear', + 'Photo not provided': 'user_image_missing', } try: - msg_json = json.loads(self.error_msg) - msg_dict = msg_json[0] + messages = set() + message_groups = json.loads(self.error_msg) - msg = [] - for category in msg_dict: - # find the messages associated with this category - category_msgs = msg_dict[category] - for category_msg in category_msgs: - msg.append(message_dict[(category, category_msg)]) - return u", ".join(msg) - except (ValueError, KeyError): - # if we can't parse the message as JSON or the category doesn't - # match one of our known categories, show a generic error - log.error('PhotoVerification: Error parsing this error message: %s', self.error_msg) - return _("There was an error verifying your ID photos.") + for message_group in message_groups: + messages = messages.union(set(*six.itervalues(message_group))) + + for message in messages: + parsed_error = error_map.get(message) + + if parsed_error: + parsed_errors.append(parsed_error) + else: + log.debug('Ignoring photo verification error message: %s', message) + except Exception: # pylint: disable=broad-except + log.exception('Failed to parse error message for SoftwareSecurePhotoVerification %d', self.pk) + + return parsed_errors def image_url(self, name, override_receipt_id=None): """ diff --git a/lms/djangoapps/verify_student/tests/test_models.py b/lms/djangoapps/verify_student/tests/test_models.py index 35efe8b57c..8b92027792 100644 --- a/lms/djangoapps/verify_student/tests/test_models.py +++ b/lms/djangoapps/verify_student/tests/test_models.py @@ -325,20 +325,15 @@ class TestPhotoVerification(MockS3Mixin, ModuleStoreTestCase): self.assertEquals(status, ('none', '')) # test for when one has been created - attempt = SoftwareSecurePhotoVerification(user=user) - attempt.status = 'approved' - attempt.save() - + attempt = SoftwareSecurePhotoVerification.objects.create(user=user, status='approved') status = SoftwareSecurePhotoVerification.user_status(user) self.assertEquals(status, ('approved', '')) # create another one for the same user, make sure the right one is # returned - attempt2 = SoftwareSecurePhotoVerification(user=user) - attempt2.status = 'denied' - attempt2.error_msg = '[{"photoIdReasons": ["Not provided"]}]' - attempt2.save() - + SoftwareSecurePhotoVerification.objects.create( + user=user, status='denied', error_msg='[{"photoIdReasons": ["Not provided"]}]' + ) status = SoftwareSecurePhotoVerification.user_status(user) self.assertEquals(status, ('approved', '')) @@ -346,31 +341,26 @@ class TestPhotoVerification(MockS3Mixin, ModuleStoreTestCase): # properly attempt.delete() status = SoftwareSecurePhotoVerification.user_status(user) - self.assertEquals(status, ('must_reverify', "No photo ID was provided.")) + self.assertEquals(status, ('must_reverify', ['id_image_missing'])) + # pylint: disable=line-too-long def test_parse_error_msg_success(self): user = UserFactory.create() attempt = SoftwareSecurePhotoVerification(user=user) attempt.status = 'denied' - attempt.error_msg = '[{"photoIdReasons": ["Not provided"]}]' + attempt.error_msg = '[{"userPhotoReasons": ["Face out of view"]}, {"photoIdReasons": ["Photo hidden/No photo", "ID name not provided"]}]' parsed_error_msg = attempt.parsed_error_msg() - self.assertEquals("No photo ID was provided.", parsed_error_msg) + self.assertEquals(parsed_error_msg, ['id_image_missing_name', 'user_image_not_clear', 'id_image_not_clear']) - def test_parse_error_msg_failure(self): + @ddt.data( + 'Not Provided', + '{"IdReasons": ["Not provided"]}', + u'[{"ïḋṚëäṡöṅṡ": ["Ⓝⓞⓣ ⓟⓡⓞⓥⓘⓓⓔⓓ "]}]', + ) + def test_parse_error_msg_failure(self, msg): user = UserFactory.create() - attempt = SoftwareSecurePhotoVerification(user=user) - attempt.status = 'denied' - # when we can't parse into json - bad_messages = { - 'Not Provided', - '[{"IdReasons": ["Not provided"]}]', - '{"IdReasons": ["Not provided"]}', - u'[{"ïḋṚëäṡöṅṡ": ["Ⓝⓞⓣ ⓟⓡⓞⓥⓘⓓⓔⓓ "]}]', - } - for msg in bad_messages: - attempt.error_msg = msg - parsed_error_msg = attempt.parsed_error_msg() - self.assertEquals(parsed_error_msg, "There was an error verifying your ID photos.") + attempt = SoftwareSecurePhotoVerification.objects.create(user=user, status='denied', error_msg=msg) + self.assertEqual(attempt.parsed_error_msg(), []) def test_active_at_datetime(self): user = UserFactory.create() diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index 2c900bd25e..578669b3c2 100644 --- a/lms/djangoapps/verify_student/views.py +++ b/lms/djangoapps/verify_student/views.py @@ -1179,7 +1179,7 @@ class ReverifyView(View): Most of the work is done client-side by composing the same Backbone views used in the initial verification flow. """ - status, _ = SoftwareSecurePhotoVerification.user_status(request.user) + status, __ = SoftwareSecurePhotoVerification.user_status(request.user) expiration_datetime = SoftwareSecurePhotoVerification.get_expiration_datetime(request.user) can_reverify = False diff --git a/lms/templates/dashboard/_dashboard_status_verification.html b/lms/templates/dashboard/_dashboard_status_verification.html index 28fcd7def8..2dce1a12af 100644 --- a/lms/templates/dashboard/_dashboard_status_verification.html +++ b/lms/templates/dashboard/_dashboard_status_verification.html @@ -18,7 +18,19 @@ from django.utils.translation import ugettext as _ %elif verification_status in ['denied','must_reverify', 'must_retry']:
  • ${_("Current Verification Status: Denied")} -

    ${_("Your verification submission was not accepted. To receive a verified certificate, you must submit a new photo of yourself and your government-issued photo ID before the verification deadline for your course.")}

    +

    + ${_("Your verification submission was not accepted. To receive a verified certificate, you must submit a new photo of yourself and your government-issued photo ID before the verification deadline for your course.")} + + %if verification_errors: +

    + ${_("Your verification was denied for the following reasons:")}
    +

    + %endif +

    ${_("Resubmit Verification")}