From 3e1519c62ac00508eff0f3dd386a1887915a6a5e Mon Sep 17 00:00:00 2001 From: Julia Hansbrough <julia@edx.org> Date: Tue, 28 Jan 2014 22:04:44 +0000 Subject: [PATCH] Midcourse reverification: increased quality Also, removed an unused template --- common/djangoapps/student/views.py | 96 ++++++++++--------- lms/djangoapps/courseware/views.py | 39 ++++---- lms/djangoapps/verify_student/views.py | 29 +----- lms/templates/courseware/reverify_banner.html | 14 --- lms/templates/dashboard.html | 2 +- .../_dashboard_prompt_midcourse_reverify.html | 8 +- .../_dashboard_reverification_sidebar.html | 18 ++-- .../midcourse_reverify_dash.html | 20 ++-- 8 files changed, 97 insertions(+), 129 deletions(-) delete mode 100644 lms/templates/courseware/reverify_banner.html diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 3d0de014ec6..b8f14b13c89 100644 --- a/common/djangoapps/student/views.py +++ b/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, diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index f050507e227..2981bc06a41 100644 --- a/lms/djangoapps/courseware/views.py +++ b/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): diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index 54e7c46965e..f1b5b6bf53b 100644 --- a/lms/djangoapps/verify_student/views.py +++ b/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) diff --git a/lms/templates/courseware/reverify_banner.html b/lms/templates/courseware/reverify_banner.html deleted file mode 100644 index 24e3c9221e1..00000000000 --- a/lms/templates/courseware/reverify_banner.html +++ /dev/null @@ -1,14 +0,0 @@ -<%! - 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 diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html index 16d63ea71e7..1c24be8174f 100644 --- a/lms/templates/dashboard.html +++ b/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> diff --git a/lms/templates/dashboard/_dashboard_prompt_midcourse_reverify.html b/lms/templates/dashboard/_dashboard_prompt_midcourse_reverify.html index 905eab0720a..baa37d71489 100644 --- a/lms/templates/dashboard/_dashboard_prompt_midcourse_reverify.html +++ b/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.')} diff --git a/lms/templates/dashboard/_dashboard_reverification_sidebar.html b/lms/templates/dashboard/_dashboard_reverification_sidebar.html index 500020fcf8d..561a235ba20 100644 --- a/lms/templates/dashboard/_dashboard_reverification_sidebar.html +++ b/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 diff --git a/lms/templates/verify_student/midcourse_reverify_dash.html b/lms/templates/verify_student/midcourse_reverify_dash.html index 30b2054225a..e7dacc4a107 100644 --- a/lms/templates/verify_student/midcourse_reverify_dash.html +++ b/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 -- GitLab