semantic task_input for certificate generation ECOM-3505

lms/djangoapps/certificates/views/support.py

@@ -260,5 +260,10 @@ def generate_certificate_for_user(request):
             return HttpResponseBadRequest(msg)
 
         # Attempt to generate certificate
-        generate_certificates_for_students(request, params["course_key"], students=[params["user"]])
+        generate_certificates_for_students(
+            request,
+            params["course_key"],
+            student_set="specific_student",
+            specific_student_id=params["user"].id
+        )
         return HttpResponse(200)

lms/djangoapps/instructor/tests/test_certificates.py

@@ -720,7 +720,6 @@ class GenerateCertificatesInstructorApiTest(SharedModuleStoreTestCase):
 
         response = self.client.post(
             url,
-            data=json.dumps([self.certificate_exception]),
             content_type='application/json'
         )
         # Assert Success
@@ -736,24 +735,49 @@ class GenerateCertificatesInstructorApiTest(SharedModuleStoreTestCase):
             u"Certificate generation started for white listed students."
         )
 
-    def test_generate_certificate_exceptions_invalid_user_list_error(self):
+    def test_generate_certificate_exceptions_whitelist_not_generated(self):
         """
-        Test generate certificates exceptions api endpoint returns error
-        when called with certificate exceptions with empty 'user_id' field
+        Test generate certificates exceptions api endpoint returns success
+        when calling with new certificate exception.
         """
         url = reverse(
             'generate_certificate_exceptions',
             kwargs={'course_id': unicode(self.course.id), 'generate_for': 'new'}
         )
 
-        # assign empty user_id
-        self.certificate_exception.update({'user_id': ''})
+        response = self.client.post(
+            url,
+            content_type='application/json'
+        )
+
+        # Assert Success
+        self.assertEqual(response.status_code, 200)
+
+        res_json = json.loads(response.content)
+
+        # Assert Request is successful
+        self.assertTrue(res_json['success'])
+        # Assert Message
+        self.assertEqual(
+            res_json['message'],
+            u"Certificate generation started for white listed students."
+        )
+
+    def test_generate_certificate_exceptions_generate_for_incorrect_value(self):
+        """
+        Test generate certificates exceptions api endpoint returns error
+        when calling with generate_for without 'new' or 'all' value.
+        """
+        url = reverse(
+            'generate_certificate_exceptions',
+            kwargs={'course_id': unicode(self.course.id), 'generate_for': ''}
+        )
 
         response = self.client.post(
             url,
-            data=json.dumps([self.certificate_exception]),
             content_type='application/json'
         )
+
         # Assert Failure
         self.assertEqual(response.status_code, 400)
 
@@ -764,7 +788,7 @@ class GenerateCertificatesInstructorApiTest(SharedModuleStoreTestCase):
         # Assert Message
         self.assertEqual(
             res_json['message'],
-            u"Invalid data, user_id must be present for all certificate exceptions."
+            u'Invalid data, generate_for must be "new" or "all".'
         )
 
 

lms/djangoapps/instructor/views/api.py

@@ -3032,41 +3032,24 @@ def generate_certificate_exceptions(request, course_id, generate_for=None):
     """
     course_key = CourseKey.from_string(course_id)
 
-    try:
-        certificate_white_list = json.loads(request.body)
-    except ValueError:
-        return JsonResponse({
-            'success': False,
-            'message': _('Invalid Json data, Please refresh the page and then try again.')
-        }, status=400)
-
-    users = [exception.get('user_id', False) for exception in certificate_white_list]
-
     if generate_for == 'all':
         # Generate Certificates for all white listed students
-        students = User.objects.filter(
-            certificatewhitelist__course_id=course_key,
-            certificatewhitelist__whitelist=True
-        )
-    elif not all(users):
-        # Invalid data, user_id must be present for all certificate exceptions
+        students = 'all_whitelisted'
+
+    elif generate_for == 'new':
+        students = 'whitelisted_not_generated'
+
+    else:
+        # Invalid data, generate_for must be present for all certificate exceptions
         return JsonResponse(
             {
                 'success': False,
-                'message': _('Invalid data, user_id must be present for all certificate exceptions.'),
+                'message': _('Invalid data, generate_for must be "new" or "all".'),
             },
             status=400
         )
-    else:
-        students = User.objects.filter(
-            id__in=users,
-            certificatewhitelist__course_id=course_key,
-            certificatewhitelist__whitelist=True
-        )
 
-    if students:
-        # generate certificates for students if 'students' list is not empty
-        instructor_task.api.generate_certificates_for_students(request, course_key, students=students)
+    instructor_task.api.generate_certificates_for_students(request, course_key, student_set=students)
 
     response_payload = {
         'success': True,
@@ -3275,8 +3258,10 @@ def re_validate_certificate(request, course_key, generated_certificate):
         certificate_invalidation.deactivate()
 
     # We need to generate certificate only for a single student here
-    students = [certificate_invalidation.generated_certificate.user]
-    instructor_task.api.generate_certificates_for_students(request, course_key, students=students)
+    student = certificate_invalidation.generated_certificate.user
+    instructor_task.api.generate_certificates_for_students(
+        request, course_key, student_set="specific_student", specific_student_id=student.id
+    )
 
 
 def validate_request_data_and_get_certificate(certificate_invalidation, course_key):

lms/djangoapps/instructor_task/api.py

@@ -45,6 +45,13 @@ from bulk_email.models import CourseEmail
 from util import milestones_helpers
 
 
+class SpecificStudentIdMissingError(Exception):
+    """
+    Exception indicating that a student id was not provided when generating a certificate for a specific student.
+    """
+    pass
+
+
 def get_running_instructor_tasks(course_id):
     """
     Returns a query of InstructorTask objects of running tasks for a given course.
@@ -437,17 +444,34 @@ def submit_export_ora2_data(request, course_key):
     return submit_task(request, task_type, task_class, course_key, task_input, task_key)
 
 
-def generate_certificates_for_students(request, course_key, students=None):  # pylint: disable=invalid-name
+def generate_certificates_for_students(request, course_key, student_set=None, specific_student_id=None):  # pylint: disable=invalid-name
     """
-    Submits a task to generate certificates for given students enrolled in the course or
-    all students if argument 'students' is None
+    Submits a task to generate certificates for given students enrolled in the course.
+
+     Arguments:
+        course_key  : Course Key
+        student_set : Semantic for student collection for certificate generation.
+                      Options are:
+                      'all_whitelisted': All Whitelisted students.
+                      'whitelisted_not_generated': Whitelisted students which does not got certificates yet.
+                      'specific_student': Single student for certificate generation.
+        specific_student_id : Student ID when student_set is 'specific_student'
 
     Raises AlreadyRunningError if certificates are currently being generated.
-    """
-    if students:
-        task_type = 'generate_certificates_certain_student'
-        students = [student.id for student in students]
-        task_input = {'students': students}
+    Raises SpecificStudentIdMissingError if student_set is 'specific_student' and specific_student_id is 'None'
+    """
+    if student_set:
+        task_type = 'generate_certificates_student_set'
+        task_input = {'student_set': student_set}
+
+        if student_set == 'specific_student':
+            task_type = 'generate_certificates_certain_student'
+            if specific_student_id is None:
+                raise SpecificStudentIdMissingError(
+                    "Attempted to generate certificate for a single student, "
+                    "but no specific student id provided"
+                )
+            task_input.update({'specific_student_id': specific_student_id})
     else:
         task_type = 'generate_certificates_all_student'
         task_input = {}
@@ -466,22 +490,16 @@ def generate_certificates_for_students(request, course_key, students=None):  # p
     return instructor_task
 
 
-def regenerate_certificates(request, course_key, statuses_to_regenerate, students=None):
+def regenerate_certificates(request, course_key, statuses_to_regenerate):
     """
-    Submits a task to regenerate certificates for given students enrolled in the course or
-    all students if argument 'students' is None.
+    Submits a task to regenerate certificates for given students enrolled in the course.
     Regenerate Certificate only if the status of the existing generated certificate is in 'statuses_to_regenerate'
     list passed in the arguments.
 
     Raises AlreadyRunningError if certificates are currently being generated.
     """
-    if students:
-        task_type = 'regenerate_certificates_certain_student'
-        students = [student.id for student in students]
-        task_input = {'students': students}
-    else:
-        task_type = 'regenerate_certificates_all_student'
-        task_input = {}
+    task_type = 'regenerate_certificates_all_student'
+    task_input = {}
 
     task_input.update({"statuses_to_regenerate": statuses_to_regenerate})
     task_class = generate_certificates

lms/djangoapps/instructor_task/tasks_helper.py

@@ -1409,30 +1409,56 @@ def generate_students_certificates(
     json column, otherwise generate certificates for all enrolled students.
     """
     start_time = time()
-    enrolled_students = CourseEnrollment.objects.users_enrolled_in(course_id)
+    students_to_generate_certs_for = CourseEnrollment.objects.users_enrolled_in(course_id)
+
+    student_set = task_input.get('student_set')
+    if student_set == 'all_whitelisted':
+        # Generate Certificates for all white listed students.
+        students_to_generate_certs_for = students_to_generate_certs_for.filter(
+            certificatewhitelist__course_id=course_id,
+            certificatewhitelist__whitelist=True
+        )
 
-    students = task_input.get('students', None)
+    elif student_set == 'whitelisted_not_generated':
+        # All Whitelisted students
+        students_to_generate_certs_for = students_to_generate_certs_for.filter(
+            certificatewhitelist__course_id=course_id,
+            certificatewhitelist__whitelist=True
+        )
 
-    if students is not None:
-        enrolled_students = enrolled_students.filter(id__in=students)
+        # Whitelisted students which got certificates already.
+        certificate_generated_students = GeneratedCertificate.objects.filter(  # pylint: disable=no-member
+            course_id=course_id,
+        )
+        certificate_generated_students_ids = set(certificate_generated_students.values_list('user_id', flat=True))
 
-    task_progress = TaskProgress(action_name, enrolled_students.count(), start_time)
+        students_to_generate_certs_for = students_to_generate_certs_for.exclude(
+            id__in=certificate_generated_students_ids
+        )
+
+    elif student_set == "specific_student":
+        specific_student_id = task_input.get('specific_student_id')
+        students_to_generate_certs_for = students_to_generate_certs_for.filter(id=specific_student_id)
+
+    task_progress = TaskProgress(action_name, students_to_generate_certs_for.count(), start_time)
 
     current_step = {'step': 'Calculating students already have certificates'}
     task_progress.update_task_state(extra_meta=current_step)
 
     statuses_to_regenerate = task_input.get('statuses_to_regenerate', [])
-    if students is not None and not statuses_to_regenerate:
+    if student_set is not None and not statuses_to_regenerate:
         # We want to skip 'filtering students' only when students are given and statuses to regenerate are not
-        students_require_certs = enrolled_students
+        students_require_certs = students_to_generate_certs_for
     else:
-        students_require_certs = students_require_certificate(course_id, enrolled_students, statuses_to_regenerate)
+        students_require_certs = students_require_certificate(
+            course_id, students_to_generate_certs_for, 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)
+        invalidate_generated_certificates(course_id, students_to_generate_certs_for, statuses_to_regenerate)
 
     task_progress.skipped = task_progress.total - len(students_require_certs)
 

lms/djangoapps/instructor_task/tests/test_api.py

@@ -24,6 +24,7 @@ from instructor_task.api import (
     generate_certificates_for_students,
     regenerate_certificates,
     submit_export_ora2_data,
+    SpecificStudentIdMissingError,
 )
 
 from instructor_task.api_helper import AlreadyRunningError
@@ -295,6 +296,18 @@ class InstructorTaskCourseSubmitTest(TestReportMixin, InstructorTaskCourseTestCa
             )
         self._test_resubmission(api_call)
 
+    def test_certificate_generation_no_specific_student_id(self):
+        """
+        Raises ValueError when student_set is 'specific_student' and 'specific_student_id' is None.
+        """
+        with self.assertRaises(SpecificStudentIdMissingError):
+            generate_certificates_for_students(
+                self.create_task_request(self.instructor),
+                self.course.id,
+                student_set='specific_student',
+                specific_student_id=None
+            )
+
     def test_certificate_generation_history(self):
         """
         Tests that a new record is added whenever certificate generation/regeneration task is submitted.

lms/templates/instructor/instructor_dashboard_2/certificate-white-list.underscore

@@ -2,7 +2,8 @@
 <p class="under-heading">
 <label>
     <input type='radio' name='generate-exception-certificates-radio' checked="checked" value='new' aria-describedby='generate-exception-certificates-radio-new-tip'>
-    <span id='generate-exception-certificates-radio-new-tip'><%- gettext('Generate a Certificate for all ') %><strong><%- gettext('New') %></strong> <%- gettext('additions to the Exception list') %></span>
+    <span id='generate-exception-certificates-radio-new-tip'><%- gettext('Generate certificates for all users on the Exception list for whom certificates have not yet been run') %></span>
+
 </label>
 <br/>
 <label>