From 3134a76118a92a892f76d5947fe9f8f574ce132e Mon Sep 17 00:00:00 2001 From: Saleem Latif Date: Wed, 9 Dec 2015 11:38:37 +0500 Subject: [PATCH] Revised Generate Certificates Section and added Certificate Generation history UI. --- .../lms/test_lms_instructor_dashboard.py | 5 -- lms/djangoapps/certificates/models.py | 54 ++++++++++++- .../instructor/views/instructor_dashboard.py | 16 +++- .../models/certificate_exception.js | 1 + .../views/certificate_bulk_whitelist.js | 2 +- .../js/instructor_dashboard/certificates.js | 11 +-- .../certificates_exception_spec.js | 5 +- .../instructor_dashboard/certificates_spec.js | 47 ++++++------ .../sass/course/instructor/_instructor_2.scss | 71 ++++++++++++------ .../certificate-white-list-editor.underscore | 2 +- .../certificate-white-list.underscore | 8 +- .../instructor_dashboard_2/certificates.html | 75 ++++++++++++++----- 12 files changed, 209 insertions(+), 88 deletions(-) diff --git a/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py b/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py index 4b75fc6506..ac50323b5a 100644 --- a/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py +++ b/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py @@ -673,7 +673,6 @@ class CertificatesTest(BaseInstructorDashboardTest): self.certificates_section.add_certificate_exception(self.user_name, notes) self.assertIn(self.user_name, self.certificates_section.last_certificate_exception.text) self.assertIn(notes, self.certificates_section.last_certificate_exception.text) - self.assertIn(str(self.user_id), self.certificates_section.last_certificate_exception.text) # Verify that added exceptions are also synced with backend # Revisit Page @@ -685,7 +684,6 @@ class CertificatesTest(BaseInstructorDashboardTest): # validate certificate exception synced with server is visible in certificate exceptions list self.assertIn(self.user_name, self.certificates_section.last_certificate_exception.text) self.assertIn(notes, self.certificates_section.last_certificate_exception.text) - self.assertIn(str(self.user_id), self.certificates_section.last_certificate_exception.text) def test_instructor_can_remove_certificate_exception(self): """ @@ -701,13 +699,11 @@ class CertificatesTest(BaseInstructorDashboardTest): self.certificates_section.add_certificate_exception(self.user_name, notes) self.assertIn(self.user_name, self.certificates_section.last_certificate_exception.text) self.assertIn(notes, self.certificates_section.last_certificate_exception.text) - self.assertIn(str(self.user_id), self.certificates_section.last_certificate_exception.text) # Remove Certificate Exception self.certificates_section.remove_first_certificate_exception() self.assertNotIn(self.user_name, self.certificates_section.last_certificate_exception.text) self.assertNotIn(notes, self.certificates_section.last_certificate_exception.text) - self.assertNotIn(str(self.user_id), self.certificates_section.last_certificate_exception.text) # Verify that added exceptions are also synced with backend # Revisit Page @@ -719,7 +715,6 @@ class CertificatesTest(BaseInstructorDashboardTest): # validate certificate exception synced with server is visible in certificate exceptions list self.assertNotIn(self.user_name, self.certificates_section.last_certificate_exception.text) self.assertNotIn(notes, self.certificates_section.last_certificate_exception.text) - self.assertNotIn(str(self.user_id), self.certificates_section.last_certificate_exception.text) def test_error_on_duplicate_certificate_exception(self): """ diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index 10299922d3..e85b72d942 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -45,7 +45,6 @@ Eligibility: then the student will be issued a certificate regardless of his grade, unless he has allow_certificate set to False. """ -from datetime import datetime import json import logging import uuid @@ -88,6 +87,12 @@ class CertificateStatuses(object): unavailable = 'unavailable' auditing = 'auditing' + readable_statuses = { + downloadable: "already received", + notpassing: "didn't receive", + error: "error states" + } + class CertificateSocialNetworks(object): """ @@ -138,15 +143,26 @@ class CertificateWhitelist(models.Model): if student: white_list = white_list.filter(user=student) result = [] + generated_certificates = GeneratedCertificate.objects.filter( + course_id=course_id, + user__in=[exception.user for exception in white_list], + status=CertificateStatuses.downloadable + ) + generated_certificates = { + certificate['user']: certificate['created_date'] + for certificate in generated_certificates.values('user', 'created_date') + } for item in white_list: + certificate_generated = generated_certificates.get(item.user.id, '') result.append({ 'id': item.id, 'user_id': item.user.id, 'user_name': unicode(item.user.username), 'user_email': unicode(item.user.email), 'course_id': unicode(item.course_id), - 'created': item.created.strftime("%A, %B %d, %Y"), + 'created': item.created.strftime("%B %d, %Y"), + 'certificate_generated': certificate_generated and certificate_generated.strftime("%B %d, %Y"), 'notes': unicode(item.notes or ''), }) return result @@ -248,6 +264,40 @@ class CertificateGenerationHistory(TimeStampedModel): instructor_task = models.ForeignKey(InstructorTask) is_regeneration = models.BooleanField(default=False) + def get_task_name(self): + """ + Return "regenerated" if record corresponds to Certificate Regeneration task, otherwise returns 'generated' + """ + return "regenerated" if self.is_regeneration else "generated" + + def get_certificate_generation_candidates(self): + """ + Return the candidates for certificate generation task. It could either be students or certificate statuses + depending upon the nature of certificate generation task. Returned value could be one of the following, + + 1. "All learners" Certificate Generation task was initiated for all learners of the given course. + 2. Comma separated list of certificate statuses, This usually happens when instructor regenerates certificates. + 3. "for exceptions", This is the case when instructor generates certificates for white-listed + students. + """ + task_input = self.instructor_task.task_input + try: + task_input_json = json.loads(task_input) + except ValueError: + # if task input is empty, it means certificates were generated for all learners + return "All learners" + + # get statuses_to_regenerate from task_input convert statuses to human readable strings and return + statuses = task_input_json.get('statuses_to_regenerate', None) + if statuses: + return ", ".join( + [CertificateStatuses.readable_statuses.get(status, "") for status in statuses] + ) + + # If statuses_to_regenerate is not present in task_input then, certificate generation task was run to + # generate certificates for white listed students + return "for exceptions" + class Meta(object): app_label = "certificates" diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 9aa1c9a684..ac901b116b 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -37,7 +37,13 @@ from student.models import CourseEnrollment from shoppingcart.models import Coupon, PaidCourseRegistration, CourseRegCodeItem from course_modes.models import CourseMode, CourseModesArchive from student.roles import CourseFinanceAdminRole, CourseSalesAdminRole -from certificates.models import CertificateGenerationConfiguration, CertificateWhitelist, GeneratedCertificate +from certificates.models import ( + CertificateGenerationConfiguration, + CertificateWhitelist, + GeneratedCertificate, + CertificateStatuses, + CertificateGenerationHistory, +) from certificates import api as certs_api from util.date_utils import get_default_time_display @@ -300,6 +306,10 @@ def _section_certificates(course): ) ) instructor_generation_enabled = settings.FEATURES.get('CERTIFICATES_INSTRUCTOR_GENERATION', False) + certificate_statuses_with_count = { + certificate['status']: certificate['count'] + for certificate in GeneratedCertificate.get_unique_statuses(course_key=course.id) + } return { 'section_key': 'certificates', @@ -310,7 +320,9 @@ def _section_certificates(course): 'instructor_generation_enabled': instructor_generation_enabled, 'html_cert_enabled': html_cert_enabled, 'active_certificate': certs_api.get_active_web_certificate(course), - 'certificate_statuses': GeneratedCertificate.get_unique_statuses(course_key=course.id), + 'certificate_statuses_with_count': certificate_statuses_with_count, + 'status': CertificateStatuses, + 'certificate_generation_history': CertificateGenerationHistory.objects.filter(course_id=course.id), 'urls': { 'generate_example_certificates': reverse( 'generate_example_certificates', diff --git a/lms/static/js/certificates/models/certificate_exception.js b/lms/static/js/certificates/models/certificate_exception.js index ad54f9776b..cee08e0d74 100644 --- a/lms/static/js/certificates/models/certificate_exception.js +++ b/lms/static/js/certificates/models/certificate_exception.js @@ -21,6 +21,7 @@ user_name: '', user_email: '', created: '', + certificate_generated: '', notes: '' }, diff --git a/lms/static/js/certificates/views/certificate_bulk_whitelist.js b/lms/static/js/certificates/views/certificate_bulk_whitelist.js index 73e6bc6585..419994e717 100644 --- a/lms/static/js/certificates/views/certificate_bulk_whitelist.js +++ b/lms/static/js/certificates/views/certificate_bulk_whitelist.js @@ -200,7 +200,7 @@ if (event && event.preventDefault) { event.preventDefault(); } if (event.currentTarget.files.length === 1) { this.$el.find(DOM_SELECTORS.upload_csv_button).removeClass('is-disabled') - .addClass('button-blue'); + .addClass('btn-blue'); this.$el.find(DOM_SELECTORS.browse_file).val( event.currentTarget.value.substring(event.currentTarget.value.lastIndexOf("\\") + 1)); } diff --git a/lms/static/js/instructor_dashboard/certificates.js b/lms/static/js/instructor_dashboard/certificates.js index df5404f158..b25e50c0ba 100644 --- a/lms/static/js/instructor_dashboard/certificates.js +++ b/lms/static/js/instructor_dashboard/certificates.js @@ -81,23 +81,20 @@ var onCertificatesReady = null; success: function (data) { $btn_regenerating_certs.attr('disabled','disabled'); if(data.success){ - $certificate_regeneration_status.text(data.message). - removeClass('msg-error').addClass('msg-success'); + $certificate_regeneration_status.text(data.message).addClass("message"); } else{ - $certificate_regeneration_status.text(data.message). - removeClass('msg-success').addClass("msg-error"); + $certificate_regeneration_status.text(data.message).addClass("message"); } }, error: function(jqXHR) { try{ var response = JSON.parse(jqXHR.responseText); - $certificate_regeneration_status.text(gettext(response.message)). - removeClass('msg-success').addClass("msg-error"); + $certificate_regeneration_status.text(gettext(response.message)).addClass("message"); }catch(error){ $certificate_regeneration_status. text(gettext('Error while regenerating certificates. Please try again.')). - removeClass('msg-success').addClass("msg-error"); + addClass("message"); } } }); diff --git a/lms/static/js/spec/instructor_dashboard/certificates_exception_spec.js b/lms/static/js/spec/instructor_dashboard/certificates_exception_spec.js index 94bd191346..1207302d78 100644 --- a/lms/static/js/spec/instructor_dashboard/certificates_exception_spec.js +++ b/lms/static/js/spec/instructor_dashboard/certificates_exception_spec.js @@ -98,7 +98,7 @@ define([ { id: 1, user_id: 1, user_name: 'test1', user_email: 'test1@test.com', course_id: 'edX/test/course', created: "Thursday, October 29, 2015", - notes: 'test notes for test certificate exception' + notes: 'test notes for test certificate exception', certificate_generated: '' } ); @@ -106,7 +106,7 @@ define([ { id: 2, user_id: 2, user_name: 'test2', user_email: 'test2@test.com', course_id: 'edX/test/course', created: "Thursday, October 29, 2015", - notes: 'test notes for test certificate exception' + notes: 'test notes for test certificate exception', certificate_generated: '' } ); }); @@ -142,6 +142,7 @@ define([ user_email: "", created: "", notes: "test3 notes", + certificate_generated : '', new: true} ] }; diff --git a/lms/static/js/spec/instructor_dashboard/certificates_spec.js b/lms/static/js/spec/instructor_dashboard/certificates_spec.js index 9dd500fa1e..6db15104dd 100644 --- a/lms/static/js/spec/instructor_dashboard/certificates_spec.js +++ b/lms/static/js/spec/instructor_dashboard/certificates_spec.js @@ -17,8 +17,6 @@ define([ server_error_message: "Error while regenerating certificates. Please try again." }; var expected = { - error_class: 'msg-error', - success_class: 'msg-success', url: 'test/url/', postData : [], selected_statuses: ['downloadable', 'error'], @@ -27,29 +25,37 @@ define([ var select_options = function(option_values){ $.each(option_values, function(index, element){ - $("#certificate-statuses option[value=" + element + "]").attr('selected', 'selected'); + $("#certificate-regenerating-form input[value=" + element + "]").click(); }); }; beforeEach(function() { - var fixture = '

Regenerate Certificates

' + + var fixture = '
' + '
' + - '

Select one or more certificate statuses ' + - ' below using your mouse and ctrl or command key.

' + - ' ' + - ' ' + - ' ' + + '

To regenerate certificates for your course, ' + + ' chose the learners who will receive regenerated certificates and click
' + + ' Regenerate Certificates.' + + '

' + + '' + + '
' + + '' + + '
' + + '' + + '
' + + '' + '
' + - '
'; + '
' + + '
'; setFixtures(fixture); onCertificatesReady(); @@ -87,7 +93,6 @@ define([ $regenerate_certificates_button.click(); AjaxHelpers.respondWithError(requests, 500, {message: MESSAGES.server_error_message}); - expect($certificate_regeneration_status).toHaveClass(expected.error_class); expect($certificate_regeneration_status.text()).toEqual(MESSAGES.server_error_message); }); @@ -97,7 +102,6 @@ define([ $regenerate_certificates_button.click(); AjaxHelpers.respondWithError(requests, 400, {message: MESSAGES.error_message}); - expect($certificate_regeneration_status).toHaveClass(expected.error_class); expect($certificate_regeneration_status.text()).toEqual(MESSAGES.error_message); }); @@ -107,7 +111,6 @@ define([ $regenerate_certificates_button.click(); AjaxHelpers.respondWithJson(requests, {message: MESSAGES.success_message, success: true}); - expect($certificate_regeneration_status).toHaveClass(expected.success_class); expect($certificate_regeneration_status.text()).toEqual(MESSAGES.success_message); }); diff --git a/lms/static/sass/course/instructor/_instructor_2.scss b/lms/static/sass/course/instructor/_instructor_2.scss index 06e77e73ff..a3dd5d5cbd 100644 --- a/lms/static/sass/course/instructor/_instructor_2.scss +++ b/lms/static/sass/course/instructor/_instructor_2.scss @@ -2105,7 +2105,7 @@ input[name="subject"] { // -------------------- .instructor-dashboard-wrapper-2 section.idash-section#certificates { - %btn-blue { + .btn-blue { @extend %btn-primary-blue; padding: ($baseline/2.5) ($baseline/2); text-shadow: none; @@ -2118,6 +2118,46 @@ input[name="subject"] { border-top-style: groove; color: $black; } + .certificates-wrapper{ + .message{ + @extend %exception-message; + } + } + + p.under-heading { + margin: 12px 0 12px 0; + line-height: 23px; + } + + hr.section-divider{ + margin: 25px 0; + border-top: 7px solid #646464; + } + + .certificate-generation-history{ + table{ + thead{ + tr{ + td.task-name{ + width: 150px; + } + td.task-date{ + width: 200px; + } + } + } + + tbody{ + tr{ + td{ + padding: 5px; + vertical-align: middle; + text-align: left;; + } + } + } + } + } #certificate-white-list-editor { .certificate-exception-inputs { @@ -2134,10 +2174,6 @@ input[name="subject"] { .message { @extend %exception-message; } - - .button-blue { - @extend %btn-blue; - } } } @@ -2155,16 +2191,15 @@ input[name="subject"] { text-align: left; color: $gray; - &.date, &.email { - width: 230px; - } - - &.user-id { - width: 60px; + &.date{ + width: 150px; } &.user-name { - width: 150px; + width: 120px; + } + &.user-email { + width: 200px; } &.action { @@ -2211,10 +2246,6 @@ input[name="subject"] { } } - .button-blue { - @extend %btn-blue; - } - .message { @extend %exception-message; } @@ -2225,10 +2256,6 @@ input[name="subject"] { border-bottom: 1px groove black; display: inline-block; } - p.under-heading { - margin: 12px 0 12px 0; - line-height: 23px; - } } .bulk-white-list-exception { @@ -2250,10 +2277,6 @@ input[name="subject"] { .arrow { font-weight: bold; } - - .button-blue { - @extend %btn-blue; - } } } diff --git a/lms/templates/instructor/instructor_dashboard_2/certificate-white-list-editor.underscore b/lms/templates/instructor/instructor_dashboard_2/certificate-white-list-editor.underscore index 7940016b46..72ecf3b6fc 100644 --- a/lms/templates/instructor/instructor_dashboard_2/certificate-white-list-editor.underscore +++ b/lms/templates/instructor/instructor_dashboard_2/certificate-white-list-editor.underscore @@ -6,7 +6,7 @@
- +
diff --git a/lms/templates/instructor/instructor_dashboard_2/certificate-white-list.underscore b/lms/templates/instructor/instructor_dashboard_2/certificate-white-list.underscore index 6f2636e92d..fd59a3d42f 100644 --- a/lms/templates/instructor/instructor_dashboard_2/certificate-white-list.underscore +++ b/lms/templates/instructor/instructor_dashboard_2/certificate-white-list.underscore @@ -10,7 +10,7 @@ <%- gettext('Generate a Certificate for all users on the Exception list') %>

- +
<% if (certificates.length === 0) { %>

<%- gettext("No results") %>

@@ -18,9 +18,9 @@ - - + + @@ -30,9 +30,9 @@ %> - + diff --git a/lms/templates/instructor/instructor_dashboard_2/certificates.html b/lms/templates/instructor/instructor_dashboard_2/certificates.html index 20a0f142bb..66f971d77b 100644 --- a/lms/templates/instructor/instructor_dashboard_2/certificates.html +++ b/lms/templates/instructor/instructor_dashboard_2/certificates.html @@ -20,7 +20,7 @@ import json - + % endif @@ -40,7 +40,7 @@ import json % endif % endfor - + % endif @@ -53,13 +53,13 @@ import json - + % elif section_data['can_enable_for_course']: - + % else:

${_("You must successfully generate example certificates before you enable student-generated certificates.")}

@@ -68,7 +68,7 @@ import json % if section_data['instructor_generation_enabled'] and not (section_data['enabled_for_course'] and section_data['html_cert_enabled']): -
+

${_("Generate Certificates")}

@@ -77,7 +77,10 @@ import json

${_("Course certificate generation requires an activated web certificate configuration.")}

% else: - +

+ ${_("When you are ready to generate certificates for your course, click Generate Certificates. You do not need to do this
if you have set the certificate mode to on-demand generation.")} +

+ %endif
@@ -101,22 +104,58 @@ import json

${_("Regenerate Certificates")}

-

${_('Select one or more certificate statuses below using your mouse and ctrl or command key.')}

- - +

+ ${_('To regenerate certificates for your course, chose the learners who will receive regenerated certificates and click
Regenerate Certificates.')} +

- + +
+ +
+ +
+
+ + -
+
+ +
+
+

${_("Certificate Generation History")}

+
+
<%- gettext("Name") %><%- gettext("User ID") %> <%- gettext("User Email") %><%- gettext("Date Exception Granted") %><%- gettext("Exception Granted") %><%- gettext("Certificate Generated") %> <%- gettext("Notes") %> <%- gettext("Action") %>
<%- cert.get("user_name") %><%- cert.get("user_id") %> <%- cert.get("user_email") %> <%- cert.get("created") %><%- cert.get("certificate_generated") %> <%- cert.get("notes") %>
+ + + + + + + + + % for history in section_data['certificate_generation_history']: + + + + + + % endfor + +
${history.get_task_name().title()}${history.created.strftime("%B %d, %Y")}${history.get_certificate_generation_candidates()}
+ +
-
+

${_("SET CERTIFICATE EXCEPTIONS")}

${_("Set exceptions to generate certificates for learners who did not qualify for a certificate but have " \