diff --git a/lms/djangoapps/instructor_task/tasks_helper.py b/lms/djangoapps/instructor_task/tasks_helper.py index b9aa42be56..fa625cd739 100644 --- a/lms/djangoapps/instructor_task/tasks_helper.py +++ b/lms/djangoapps/instructor_task/tasks_helper.py @@ -40,7 +40,8 @@ from django.utils.translation import ugettext as _ from certificates.models import ( CertificateWhitelist, certificate_info_for_user, - CertificateStatuses + CertificateStatuses, + GeneratedCertificate ) from certificates.api import generate_user_certificates from courseware.courses import get_course_by_id, get_problems_in_section @@ -1419,6 +1420,12 @@ def generate_students_certificates( statuses_to_regenerate = task_input.get('statuses_to_regenerate', []) students_require_certs = students_require_certificate(course_id, enrolled_students, statuses_to_regenerate) + if statuses_to_regenerate: + # Mark existing generated certificates as 'unavailable' before regenerating + # We need to call this method after "students_require_certificate" otherwise "students_require_certificate" + # would return no results. + invalidate_generated_certificates(course_id, enrolled_students, statuses_to_regenerate) + task_progress.skipped = task_progress.total - len(students_require_certs) current_step = {'step': 'Generating Certificates'} @@ -1542,10 +1549,12 @@ def students_require_certificate(course_id, enrolled_students, statuses_to_regen if statuses_to_regenerate: # Return Students that have Generated Certificates and the generated certificate status # lies in 'statuses_to_regenerate' - return User.objects.filter( + students_require_certificates = enrolled_students.filter( generatedcertificate__course_id=course_id, generatedcertificate__status__in=statuses_to_regenerate ) + # Fetch results otherwise subsequent operations on table cause wrong data fetch + return list(students_require_certificates) else: # compute those students whose certificates are already generated students_already_have_certs = User.objects.filter( @@ -1554,3 +1563,32 @@ def students_require_certificate(course_id, enrolled_students, statuses_to_regen # Return all the enrolled student skipping the ones whose certificates have already been generated return list(set(enrolled_students) - set(students_already_have_certs)) + + +def invalidate_generated_certificates(course_id, enrolled_students, certificate_statuses): # pylint: disable=invalid-name + """ + Invalidate generated certificates for all enrolled students in the given course having status in + 'certificate_statuses'. + + Generated Certificates are invalidated by marking its status 'unavailable' and updating verify_uuid, download_uuid, + download_url and grade with empty string. + + :param course_id: Course Key for the course whose generated certificates need to be removed + :param enrolled_students: (queryset or list) students enrolled in the course + :param certificate_statuses: certificates statuses for whom to remove generated certificate + """ + certificates = GeneratedCertificate.objects.filter( + user__in=enrolled_students, + course_id=course_id, + status__in=certificate_statuses, + ) + + # Mark generated certificates as 'unavailable' and update download_url, download_uui, verify_uuid and + # grade with empty string for each row + certificates.update( + status=CertificateStatuses.unavailable, + verify_uuid='', + download_uuid='', + download_url='', + grade='', + ) diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index 51a81887e5..4e7d87e19a 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -16,7 +16,7 @@ from django.core.urlresolvers import reverse from django.test.utils import override_settings from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory -from certificates.models import CertificateStatuses +from certificates.models import CertificateStatuses, GeneratedCertificate from certificates.tests.factories import GeneratedCertificateFactory, CertificateWhitelistFactory from course_modes.models import CourseMode from courseware.tests.factories import InstructorFactory @@ -1708,6 +1708,9 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): Verify that certificates are regenerated for all eligible students enrolled in a course whose generated certificate statuses lies in the list 'statuses_to_regenerate' given in task_input. """ + # Default grade for students + default_grade = '-1' + # create 10 students students = [self.create_student(username='student_{}'.format(i), email='student_{}@example.com'.format(i)) for i in xrange(1, 11)] @@ -1718,7 +1721,8 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): user=student, course_id=self.course.id, status=CertificateStatuses.downloadable, - mode='honor' + mode='honor', + grade=default_grade ) # mark 3 students to have certificates generated with status 'error' @@ -1727,7 +1731,8 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): user=student, course_id=self.course.id, status=CertificateStatuses.error, - mode='honor' + mode='honor', + grade=default_grade ) # mark 6th students to have certificates generated with status 'deleted' @@ -1736,7 +1741,8 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): user=student, course_id=self.course.id, status=CertificateStatuses.deleted, - mode='honor' + mode='honor', + grade=default_grade ) # mark rest of the 4 students with having generated certificates with status 'generating' @@ -1748,7 +1754,8 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): user=student, course_id=self.course.id, status=CertificateStatuses.generating, - mode='honor' + mode='honor', + grade=default_grade ) # white-list 7 students @@ -1781,3 +1788,138 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): }, result ) + generated_certificates = GeneratedCertificate.objects.filter( + user__in=students, + course_id=self.course.id, + mode='honor' + ) + certificate_statuses = [generated_certificate.status for generated_certificate in generated_certificates] + certificate_grades = [generated_certificate.grade for generated_certificate in generated_certificates] + + # Verify from results from database + # Certificates are being generated for 2 white-listed students that had statuses in 'deleted'' and 'generating' + self.assertEqual(certificate_statuses.count(CertificateStatuses.generating), 2) + # 5 students are skipped that had Certificate Status 'downloadable' and 'error' + self.assertEqual(certificate_statuses.count(CertificateStatuses.downloadable), 2) + self.assertEqual(certificate_statuses.count(CertificateStatuses.error), 3) + + # grades will be '0.0' as students are either white-listed or ending in error + self.assertEqual(certificate_grades.count('0.0'), 5) + # grades will be '-1' for students that were skipped + self.assertEqual(certificate_grades.count(default_grade), 5) + + def test_certificate_regeneration_with_existing_unavailable_status(self): + """ + Verify that certificates are regenerated for all eligible students enrolled in a course whose generated + certificate status lies in the list 'statuses_to_regenerate' given in task_input. but the 'unavailable' + status is not touched if it is not in the 'statuses_to_regenerate' list. + """ + # Default grade for students + default_grade = '-1' + + # create 10 students + students = [self.create_student(username='student_{}'.format(i), email='student_{}@example.com'.format(i)) + for i in xrange(1, 11)] + + # mark 2 students to have certificates generated already + for student in students[:2]: + GeneratedCertificateFactory.create( + user=student, + course_id=self.course.id, + status=CertificateStatuses.downloadable, + mode='honor', + grade=default_grade + ) + + # mark 3 students to have certificates generated with status 'error' + for student in students[2:5]: + GeneratedCertificateFactory.create( + user=student, + course_id=self.course.id, + status=CertificateStatuses.error, + mode='honor', + grade=default_grade + ) + + # mark 2 students to have generated certificates with status 'unavailable' + for student in students[5:7]: + GeneratedCertificateFactory.create( + user=student, + course_id=self.course.id, + status=CertificateStatuses.unavailable, + mode='honor', + grade=default_grade + ) + + # mark 3 students to have generated certificates with status 'generating' + for student in students[7:]: + GeneratedCertificateFactory.create( + user=student, + course_id=self.course.id, + status=CertificateStatuses.generating, + mode='honor', + grade=default_grade + ) + + # white-list all students + for student in students[:]: + CertificateWhitelistFactory.create(user=student, course_id=self.course.id, whitelist=True) + + current_task = Mock() + current_task.update_state = Mock() + + # Regenerated certificates for students having generated certificates with status + # 'downloadable', 'error' or 'generating' + task_input = { + 'statuses_to_regenerate': [ + CertificateStatuses.downloadable, + CertificateStatuses.error, + CertificateStatuses.generating + ] + } + + with patch('instructor_task.tasks_helper._get_current_task') as mock_current_task: + mock_current_task.return_value = current_task + with patch('capa.xqueue_interface.XQueueInterface.send_to_queue') as mock_queue: + mock_queue.return_value = (0, "Successfully queued") + result = generate_students_certificates( + None, None, self.course.id, task_input, 'certificates generated' + ) + + self.assertDictContainsSubset( + { + 'action_name': 'certificates generated', + 'total': 10, + 'attempted': 8, + 'succeeded': 8, + 'failed': 0, + 'skipped': 2 + }, + result + ) + + generated_certificates = GeneratedCertificate.objects.filter( + user__in=students, + course_id=self.course.id, + mode='honor' + ) + certificate_statuses = [generated_certificate.status for generated_certificate in generated_certificates] + certificate_grades = [generated_certificate.grade for generated_certificate in generated_certificates] + + # Verify from results from database + # Certificates are being generated for 8 students that had statuses in 'downloadable', 'error' and 'generating' + self.assertEqual(certificate_statuses.count(CertificateStatuses.generating), 8) + # 2 students are skipped that had Certificate Status 'unavailable' + self.assertEqual(certificate_statuses.count(CertificateStatuses.unavailable), 2) + + # grades will be '0.0' as students are white-listed and have not completed any tasks + self.assertEqual(certificate_grades.count('0.0'), 8) + # grades will be '-1' for students that have not been processed + self.assertEqual(certificate_grades.count(default_grade), 2) + + # Verify that students with status 'unavailable were skipped + unavailable_certificates = \ + [cert for cert in generated_certificates + if cert.status == CertificateStatuses.unavailable and cert.grade == default_grade] + + self.assertEquals(len(unavailable_certificates), 2)