Midcourse reverification: increased quality

Also, removed an unused template

common/djangoapps/student/views.py

@@ -10,6 +10,7 @@ import string       # pylint: disable=W0402
 import urllib
 import uuid
 import time
+from collections import defaultdict
 from pytz import UTC
 
 from django.conf import settings
@@ -182,29 +183,61 @@ def cert_info(user, course):
     return _cert_info(user, course, certificate_status_for_student(user, course.id))
 
 
-def reverification_info(user, course, enrollment):
+def reverification_info(course_enrollment_pairs, user, statuses):
     """
-    If
-    - a course has an open re-verification window, and
-    - that user has a verified enrollment in the course
-    then we return a tuple with relevant information.
+    Returns reverification-related information for *all* of user's enrollments whose
+    reverification status is in status_list
+
+    Args:
+        course_enrollment_pairs (list): list of (course, enrollment) tuples
+        user (User): the user whose information we want
+        statuses (list): a list of reverification statuses we want information for
+            example: ["must_reverify", "denied"]
+
+    Returns:
+        dictionary of lists: dictionary with one key per status, e.g.
+            dict["must_reverify"] = []
+            dict["must_reverify"] = [some information]
+    """
+    reverifications = defaultdict(list)
+    for (course, enrollment) in course_enrollment_pairs:
+        info = single_course_reverification_info(user, course, enrollment)
+        for status in statuses:
+            if info and (status in info):
+                reverifications[status].append(info)
+
+    # Sort the data by the reverification_end_date
+    for status in statuses:
+        if reverifications[status]:
+            reverifications[status] = sorted(reverifications[status], key=lambda x: x[3])
+    return reverifications
+
+
+def single_course_reverification_info(user, course, enrollment):
+    """Returns midcourse reverification-related information for user with enrollment in course.
+
+    If a course has an open re-verification window, and that user has a verified enrollment in
+    the course, we return a tuple with relevant information.  Returns None if there is no info..
 
-    Else, return None.
+    Args:
+        user (User): the user we want to get information for
+        course (Course): the course in which the student is enrolled
+        enrollment (CourseEnrollment): the object representing the type of enrollment user has in course
 
-    Five-tuple data: (course_id, course_display_name, course_number, reverification_end_date, reverification_status)
+    Returns:
+        5-tuple: (course_id, course_display_name, course_number, reverification_end_date, reverification_status)
+        OR, None: None if there is no re-verification info for this enrollment
     """
     window = MidcourseReverificationWindow.get_window(course.id, datetime.datetime.now(UTC))
 
-    # If there's an open window AND the user is verified
-    if (window and (enrollment.mode == "verified")):
-            return (
-                course.id,
-                course.display_name,
-                course.number,
-                window.end_date.strftime('%B %d, %Y %X %p'),
-                SoftwareSecurePhotoVerification.user_status(user, window)[0],
-            )
-    return None
+    # If there's no window OR the user is not verified, we don't get reverification info
+    if (not window) or (enrollment.mode != "verified"):
+        return None
+    return (
+        course.id, course.display_name, course.number,
+        window.end_date.strftime('%B %d, %Y %X %p'),
+        SoftwareSecurePhotoVerification.user_status(user, window)[0],
+    )
 
 
 def get_course_enrollment_pairs(user, course_org_filter, org_filter_out_set):
@@ -212,7 +245,6 @@ def get_course_enrollment_pairs(user, course_org_filter, org_filter_out_set):
     Get the relevant set of (Course, CourseEnrollment) pairs to be displayed on
     a student's dashboard.
     """
-    course_enrollment_pairs = []
     for enrollment in CourseEnrollment.enrollments_for_user(user):
         try:
             course = course_from_id(enrollment.course_id)
@@ -424,27 +456,8 @@ def dashboard(request):
     verification_status, verification_msg = SoftwareSecurePhotoVerification.user_status(user)
 
     # Gets data for midcourse reverifications, if any are necessary or have failed
-    reverifications_must_reverify = []
-    reverifications_denied = []
-    reverifications_pending = []
-    reverifications_approved = []
-    for (course, enrollment) in course_enrollment_pairs:
-        info = reverification_info(user, course, enrollment)
-        if info:
-            if "approved" in info:
-                reverifications_approved.append(info)
-            elif "pending" in info:
-                reverifications_pending.append(info)
-            elif "must_reverify" in info:
-                reverifications_must_reverify.append(info)
-            elif "denied" in info:
-                reverifications_denied.append(info)
-
-    # Sort the data by the reverification_end_date
-    reverifications_must_reverify = sorted(reverifications_must_reverify, key=lambda x: x[3])
-    reverifications_denied = sorted(reverifications_denied, key=lambda x: x[3])
-    reverifications_pending = sorted(reverifications_pending, key=lambda x: x[3])
-    reverifications_approved = sorted(reverifications_approved, key=lambda x: x[3])
+    statuses = ["approved", "denied", "pending", "must_reverify"]
+    reverifications = reverification_info(course_enrollment_pairs, user, statuses)
 
     show_refund_option_for = frozenset(course.id for course, _enrollment in course_enrollment_pairs
                                        if _enrollment.refundable())
@@ -466,10 +479,7 @@ def dashboard(request):
                'all_course_modes': course_modes,
                'cert_statuses': cert_statuses,
                'show_email_settings_for': show_email_settings_for,
-               'reverifications_must_reverify': reverifications_must_reverify,
-               'reverifications_denied': reverifications_denied,
-               'reverifications_pending': reverifications_pending,
-               'reverifications_approved': reverifications_approved,
+               'reverifications': reverifications,
                'verification_status': verification_status,
                'verification_msg': verification_msg,
                'show_refund_option_for': show_refund_option_for,

lms/djangoapps/courseware/views.py

@@ -2,6 +2,7 @@ import logging
 import urllib
 
 from functools import partial
+from collections import defaultdict
 
 from django.conf import settings
 from django.core.context_processors import csrf
@@ -29,7 +30,7 @@ from courseware.models import StudentModule, StudentModuleHistory
 from course_modes.models import CourseMode
 
 from student.models import UserTestGroup, CourseEnrollment
-from student.views import course_from_id, reverification_info
+from student.views import course_from_id, single_course_reverification_info
 from util.cache import cache, cache_if_anonymous
 from xblock.fragment import Fragment
 from xmodule.modulestore import Location
@@ -266,10 +267,9 @@ def index(request, course_id, chapter=None, section=None,
             'fragment': Fragment(),
             'staff_access': staff_access,
             'masquerade': masq,
-            'xqa_server': settings.FEATURES.get('USE_XQA_SERVER', 'http://xqa:server@content-qa.mitx.mit.edu/xqa')
+            'xqa_server': settings.FEATURES.get('USE_XQA_SERVER', 'http://xqa:server@content-qa.mitx.mit.edu/xqa'),
+            'reverifications': fetch_reverify_banner_info(request, course_id),
             }
-        reverify_context = fetch_reverify_banner_keypairs(request, course_id)
-        context = dict(context.items() + reverify_context.items())
 
         # Only show the chat if it's enabled by the course and in the
         # settings.
@@ -454,11 +454,11 @@ def course_info(request, course_id):
     course = get_course_with_access(request.user, course_id, 'load')
     staff_access = has_access(request.user, course, 'staff')
     masq = setup_masquerade(request, staff_access)    # allow staff to toggle masquerade on info page
+    reverifications = fetch_reverify_banner_info(request, course_id)
 
     context = {'request': request, 'course_id': course_id, 'cache': None,
-               'course': course, 'staff_access': staff_access, 'masquerade': masq}
-    reverify_context = fetch_reverify_banner_keypairs(request, course_id)
-    context = dict(context.items() + reverify_context.items())
+               'course': course, 'staff_access': staff_access, 'masquerade': masq,
+               'reverifications': reverifications, }
 
     return render_to_response('courseware/info.html', context)
 
@@ -661,10 +661,8 @@ def _progress(request, course_id, student_id):
         'grade_summary': grade_summary,
         'staff_access': staff_access,
         'student': student,
+        'reverifications': fetch_reverify_banner_info(request, course_id)
     }
-    reverify_context = fetch_reverify_banner_keypairs(request, course_id)
-    context = dict(context.items() + reverify_context.items())
-
 
     with grades.manual_transaction():
         response = render_to_response('courseware/progress.html', context)
@@ -672,28 +670,23 @@ def _progress(request, course_id, student_id):
     return response
 
 
-def fetch_reverify_banner_keypairs(request, course_id):
+def fetch_reverify_banner_info(request, course_id):
     """
-    Fetches needed context variables to display reverification banner in courseware
+    Fetches needed context variable to display reverification banner in courseware
     """
-    reverifications_must_reverify = []
-    reverifications_denied = []
+    reverifications = defaultdict(list)
     user = request.user
     if not user.id:
-        return {'reverifications_must_reverify': None, 'reverifications_denied': None, }
+        return {'reverifications': None, }
     enrollment = CourseEnrollment.get_or_create_enrollment(request.user, course_id)
     course = course_from_id(course_id)
-    info = reverification_info(user, course, enrollment)
+    info = single_course_reverification_info(user, course, enrollment)
     if info:
         if "must_reverify" in info:
-            reverifications_must_reverify.append(info)
+            reverifications["must_reverify"].append(info)
         elif "denied" in info:
-            reverifications_denied.append(info)
-    return {
-        'reverifications_must_reverify': reverifications_must_reverify,
-        'reverifications_denied': reverifications_denied,
-    }
-
+            reverifications["denied"].append(info)
+    return reverifications
 
 @login_required
 def submission_history(request, course_id, student_username, location):

lms/djangoapps/verify_student/views.py

@@ -409,34 +409,13 @@ def midcourse_reverify_dash(_request):
             log.error("User {0} enrolled in non-existent course {1}"
                       .format(user.username, enrollment.course_id))
 
-    reverifications_must_reverify = []
-    reverifications_denied = []
-    reverifications_pending = []
-    reverifications_approved = []
-    for (course, enrollment) in course_enrollment_pairs:
-        info = reverification_info(user, course, enrollment)
-        if info:
-            if "approved" in info:
-                reverifications_approved.append(info)
-            elif "pending" in info:
-                reverifications_pending.append(info)
-            elif "must_reverify" in info:
-                reverifications_must_reverify.append(info)
-            elif "denied" in info:
-                reverifications_denied.append(info)
-
-    # Sort the data by the reverification_end_date
-    reverifications_must_reverify = sorted(reverifications_must_reverify, key=lambda x: x[3])
-    reverifications_denied = sorted(reverifications_denied, key=lambda x: x[3])
-    reverifications_pending = sorted(reverifications_pending, key=lambda x: x[3])
-    reverifications_approved = sorted(reverifications_approved, key=lambda x: x[3])
+    statuses = ["approved", "pending", "must_reverify", "denied"]
+
+    reverifications = reverification_info(course_enrollment_pairs, user, statuses)
 
     context = {
         "user_full_name": user.profile.name,
-        'reverifications_must_reverify': reverifications_must_reverify,
-        'reverifications_denied': reverifications_denied,
-        'reverifications_pending': reverifications_pending,
-        'reverifications_approved': reverifications_approved,
+        'reverifications': reverifications,
     }
     return render_to_response("verify_student/midcourse_reverify_dash.html", context)
 

lms/templates/courseware/reverify_banner.html

-<%!
-    from django.core.urlresolvers import reverse
-    from xmodule.util.date_utils import get_time_display
-    from django.utils.translation import ugettext as _
-%>
-YOLO
-%if status == "must_reverify":
-You need to verify your ID again before DATE.
-
-To continue in the verified track in this course, you need to re-verify by DATE.  Why you need to verify yourself again.
-
-%elif status == "denied":
-
-You re-verification for this course failed and you are no longer eligible for a Verified Certificate.  If you think this is in error, please contact us at support@edx.org

lms/templates/dashboard.html

@@ -153,7 +153,7 @@
 </%block>
 
 <!-- TODO later will need to make this ping for all courses on the dash -->
-% if reverifications_must_reverify or reverifications_denied:
+% if reverifications["must_reverify"] or reverifications["denied"]:
   <section class="dashboard-banner">
     <%include file='dashboard/_dashboard_prompt_midcourse_reverify.html' />
   </section>

lms/templates/dashboard/_dashboard_prompt_midcourse_reverify.html

@@ -2,12 +2,12 @@
 <%! from django.core.urlresolvers import reverse %>
 <!--TODO replace this with something a clever deisgn person approves of-->
 <!--TODO replace this with a shiny loopy thing to actually print out all courses-->
-% if reverifications_must_reverify:
+% if reverifications["must_reverify"]:
 <div class="wrapper-msg">
   <div class="msg msg-reverify has-actions">
       <div class="msg-content">
         <h2 class="title">${_("You need to re-verify to continue")}</h2>
-        % for course_id, course_name, course_number, date, status in reverifications_must_reverify:
+        % for course_id, course_name, course_number, date, status in reverifications["must_reverify"]:
         <div class="copy">
           <p class='activation-message'>
           	${_('To continue in the verified track in <strong>{course_name}</strong>, you need to re-verify your identity by {date}.').format(course_name=course_name, date=date)}
@@ -26,12 +26,12 @@
 % endfor
 %endif
 
-%if reverifications_denied:
+%if reverifications["denied"]:
 <div class="wrapper-msg">
   <div class="msg msg-reverify has-actions">
       <div class="msg-content">
         <h2 class="title">${_("Your re-verification failed")}</h2>
-        % for course_id, course_name, course_number, date, status in reverifications_denied:
+        % for course_id, course_name, course_number, date, status in reverifications["denied"]:
         <div class="copy">
           <p class='activation-message'>
             ${_('Your re-verification for <strong>{course_name}</strong> failed and you are no longer eligible for a Verified Certificate.  If you think this is in error, please contact us at support@edx.org.')}

lms/templates/dashboard/_dashboard_reverification_sidebar.html

@@ -2,29 +2,29 @@
 <%! from django.core.urlresolvers import reverse %>
 <!--TODO replace this with something a clever deisgn person approves of-->
 <!--TODO replace this with a shiny loopy thing to actually print out all courses-->
-% if reverifications_must_reverify or reverifications_denied or reverifications_pending or reverifications_approved:
+% if reverifications["must_reverify"] or reverifications["pending"] or reverifications["denied"] or reverifications["approved"]:
 
     <h2 class="title">${_("Re-verification now open for")}</h2>
-    % if reverifications_must_reverify:
-        % for course_id, course_name, course_number, date, status in reverifications_must_reverify:
+    % if reverifications["must_reverify"]:
+        % for course_id, course_name, course_number, date, status in reverifications["must_reverify"]:
             ${_('Must Reverify: <strong>{course_name}</strong>').format(course_name=course_name)}
         % endfor
     %endif
 
-    % if reverifications_pending:
-        % for course_id, course_name, course_number, date, status in reverifications_pending:
+    % if reverifications["pending"]:
+        % for course_id, course_name, course_number, date, status in reverifications["pending"]:
             ${_('Denied: <strong>{course_name}</strong>').format(course_name=course_name)}
         % endfor
     %endif
 
-    % if reverifications_denied:
-        % for course_id, course_name, course_number, date, status in reverifications_denied:
+    % if reverifications["denied"]:
+        % for course_id, course_name, course_number, date, status in reverifications["denied"]:
             ${_('Denied: <strong>{course_name}</strong>').format(course_name=course_name)}
         % endfor
     %endif
 
-    % if reverifications_approved:
-        % for course_id, course_name, course_number, date, status in reverifications_approved:
+    % if reverifications["approved"]:
+        % for course_id, course_name, course_number, date, status in reverifications["approved"]:
             ${_('Denied: <strong>{course_name}</strong>').format(course_name=course_name)}
         % endfor
     %endif

lms/templates/verify_student/midcourse_reverify_dash.html

@@ -18,12 +18,12 @@
 
         <div class="copy">
 
-          % if reverifications_must_reverify:
-            % if len(reverifications_must_reverify) > 1:
+          % if reverifications["must_reverify"]:
+            % if len(reverifications["must_reverify"]) > 1:
               <div class="wrapper-reverify-open">
                 <h3 class="title">You currently need to re-verify for the following courses:</h3>
                 <ul class="reverification-list">
-                % for course_id, course_name, course_number, date, status in reverifications_must_reverify:
+                % for course_id, course_name, course_number, date, status in reverifications["must_reverify"]:
                   <li class="item">
                     <div class="course-info">
                       <h3 class="course-name">${course_name} (${course_number})</h3>
@@ -35,12 +35,12 @@
                 </ul>
               </div>
 
-            % elif reverifications_must_reverify:
+            % elif reverifications["must_reverify"]:
               <div class="wrapper-reverify-open">
                 <h3 class="title">You currently need to re-verify for the following course:</h3>
 
                 <ul class="reverification-list">
-                % for course_id, course_name, course_number, date, status in reverifications_must_reverify:
+                % for course_id, course_name, course_number, date, status in reverifications["must_reverify"]:
                   <li class="item">
                     <div class="course-info">
                       <h3 class="course-name">${course_name} (${course_number})</h3>
@@ -53,12 +53,12 @@
               </div>
           %endif
 
-          % if reverifications_pending or reverifications_approved or reverifications_denied:
+          % if reverifications["pending"] or reverifications["approved"] or reverifications["denied"]:
             <div class="wrapper-reverify-status">
               <h3 class="title">The status of your submitted re-verifications:</h3>
               <ul class="reverification-list reverification-status">
 
-              % for course_id, course_name, course_number, date, status in reverifications_pending:
+              % for course_id, course_name, course_number, date, status in reverifications["pending"]:
                 <li class="item pending">
                   <div class="course-info">
                     <h3 class="course-name">${course_name} (${course_number})</h3>
@@ -68,7 +68,7 @@
                 </li>
               % endfor
 
-              % for course_id, course_name, course_number, date, status in reverifications_approved:
+              % for course_id, course_name, course_number, date, status in reverifications["approved"]:
                 <li class="item complete">
                   <div class="course-info">
                     <h3 class="course-name">${course_name} (${course_number})</h3>
@@ -78,7 +78,7 @@
                 </li>
               % endfor
 
-              % for course_id, course_name, course_number, date, status in reverifications_denied:
+              % for course_id, course_name, course_number, date, status in reverifications["denied"]:
                 <li class="item failed">
                   <div class="course-info">
                     <h3 class="course-name">${course_name} (${course_number})</h3>
@@ -96,7 +96,7 @@
           <p class="title">You have no re-verifications at present.</p>
           % endif
 
-          % if reverifications_must_reverify:
+          % if reverifications["must_reverify"]:
             <p class="support">Don't want to re-verify right now? <a href="">Return to where you left off <i class="icon-angle-right"></i></a></p>
           % endif