EDUCATOR-1117 | Call celery task with kwargs in the correct fashion.

lms/djangoapps/certificates/signals.py

@@ -41,10 +41,7 @@ def _listen_for_certificate_whitelist_append(sender, instance, **kwargs):  # pyl
         if courses.get_course_by_id(instance.course_id, depth=0).self_paced:
             return
 
-    generate_certificate.apply_async(
-        student=instance.user,
-        course_key=instance.course_id,
-    )
+    fire_ungenerated_certificate_task(instance.user, instance.course_id)
     log.info(u'Certificate generation task initiated for {user} : {course} via whitelist'.format(
         user=instance.user.id,
         course=instance.course_id
@@ -73,10 +70,7 @@ def _listen_for_passing_grade(sender, user, course_id, **kwargs):  # pylint: dis
     if waffle.waffle().is_enabled(waffle.INSTRUCTOR_PACED_ONLY):
         if courses.get_course_by_id(course_id, depth=0).self_paced:
             return
-    if fire_ungenerated_certificate_task(
-        user=user,
-        course_id=course_id
-    ):
+    if fire_ungenerated_certificate_task(user, course_id):
         log.info(u'Certificate generation task initiated for {user} : {course} via passing grade'.format(
             user=user.id,
             course=course_id
@@ -98,23 +92,22 @@ def _listen_for_track_change(sender, user, **kwargs):  # pylint: disable=unused-
     grade_factory = CourseGradeFactory()
     for enrollment in user_enrollments:
         if grade_factory.read(user=user, course=enrollment.course).passed:
-            if fire_ungenerated_certificate_task(
-                user=user,
-                course_id=enrollment.course.id
-            ):
+            if fire_ungenerated_certificate_task(user, enrollment.course.id):
                 log.info(u'Certificate generation task initiated for {user} : {course} via track change'.format(
                     user=user.id,
                     course=enrollment.course.id
                 ))
 
 
-def fire_ungenerated_certificate_task(user, course_id):
+def fire_ungenerated_certificate_task(user, course_key):
     """
     Helper function to fire un-generated certificate tasks
+    :param user: A User object.
+    :param course_id: A CourseKey object.
     """
-    if GeneratedCertificate.certificate_for_student(user, course_id) is None:
-        generate_certificate.apply_async(
-            student=user,
-            course_key=course_id
-        )
+    if GeneratedCertificate.certificate_for_student(user, course_key) is None:
+        generate_certificate.apply_async(kwargs={
+            'student': unicode(user.id),
+            'course_key': unicode(course_key),
+        })
         return True

lms/djangoapps/certificates/tasks.py

@@ -3,6 +3,8 @@ from logging import getLogger
 
 from celery_utils.logged_task import LoggedTask
 from celery_utils.persist_on_failure import PersistOnFailureTask
+from django.contrib.auth.models import User
+from opaque_keys.edx.keys import CourseKey
 
 from .api import generate_user_certificates
 
@@ -21,6 +23,6 @@ def generate_certificate(**kwargs):
     """
     Generates a certificate for a single user.
     """
-    student = kwargs.pop('student')
-    course_key = kwargs.pop('course_key')
+    student = User.objects.get(id=kwargs.pop('student'))
+    course_key = CourseKey.from_string(kwargs.pop('course_key'))
     generate_user_certificates(student=student, course_key=course_key, **kwargs)

lms/djangoapps/certificates/tests/test_signals.py

@@ -83,10 +83,10 @@ class WhitelistGeneratedCertificatesTest(ModuleStoreTestCase):
                     user=self.user,
                     course_id=self.course.id
                 )
-                mock_generate_certificate_apply_async.assert_called_with(
-                    student=self.user,
-                    course_key=self.course.id,
-                )
+                mock_generate_certificate_apply_async.assert_called_with(kwargs={
+                    'student': unicode(self.user.id),
+                    'course_key': unicode(self.course.id),
+                })
 
     def test_cert_generation_on_whitelist_append_instructor_paced(self):
         """
@@ -108,10 +108,10 @@ class WhitelistGeneratedCertificatesTest(ModuleStoreTestCase):
                     user=self.user,
                     course_id=self.ip_course.id
                 )
-                mock_generate_certificate_apply_async.assert_called_with(
-                    student=self.user,
-                    course_key=self.ip_course.id
-                )
+                mock_generate_certificate_apply_async.assert_called_with(kwargs={
+                    'student': unicode(self.user.id),
+                    'course_key': unicode(self.ip_course.id),
+                })
 
 
 class PassingGradeCertsTest(ModuleStoreTestCase):
@@ -151,10 +151,10 @@ class PassingGradeCertsTest(ModuleStoreTestCase):
                 # Certs fired after passing
                 with mock_passing_grade():
                     grade_factory.update(self.user, self.course)
-                    mock_generate_certificate_apply_async.assert_called_with(
-                        student=self.user,
-                        course_key=self.course.id
-                    )
+                    mock_generate_certificate_apply_async.assert_called_with(kwargs={
+                        'student': unicode(self.user.id),
+                        'course_key': unicode(self.course.id),
+                    })
 
     def test_cert_generation_on_passing_instructor_paced(self):
         with mock.patch(
@@ -169,10 +169,10 @@ class PassingGradeCertsTest(ModuleStoreTestCase):
                 # Certs fired after passing
                 with mock_passing_grade():
                     grade_factory.update(self.user, self.ip_course)
-                    mock_generate_certificate_apply_async.assert_called_with(
-                        student=self.user,
-                        course_key=self.ip_course.id
-                    )
+                    mock_generate_certificate_apply_async.assert_called_with(kwargs={
+                        'student': unicode(self.user.id),
+                        'course_key': unicode(self.ip_course.id),
+                    })
 
     def test_cert_already_generated(self):
         with mock.patch(
@@ -231,10 +231,10 @@ class LearnerTrackChangeCertsTest(ModuleStoreTestCase):
                     status='submitted'
                 )
                 attempt.approve()
-                mock_generate_certificate_apply_async.assert_called_with(
-                    student=self.user_one,
-                    course_key=self.course_one.id
-                )
+                mock_generate_certificate_apply_async.assert_called_with(kwargs={
+                    'student': unicode(self.user_one.id),
+                    'course_key': unicode(self.course_one.id),
+                })
 
     def test_cert_generation_on_photo_verification_instructor_paced(self):
         with mock.patch(
@@ -252,7 +252,7 @@ class LearnerTrackChangeCertsTest(ModuleStoreTestCase):
                     status='submitted'
                 )
                 attempt.approve()
-                mock_generate_certificate_apply_async.assert_called_with(
-                    student=self.user_two,
-                    course_key=self.course_two.id
-                )
+                mock_generate_certificate_apply_async.assert_called_with(kwargs={
+                    'student': unicode(self.user_two.id),
+                    'course_key': unicode(self.course_two.id),
+                })

lms/djangoapps/certificates/tests/test_tasks.py

@@ -2,6 +2,7 @@ from unittest import TestCase
 
 import ddt
 from mock import patch
+from opaque_keys.edx.keys import CourseKey
 
 from lms.djangoapps.certificates.tasks import generate_certificate
 
@@ -9,13 +10,26 @@ from lms.djangoapps.certificates.tasks import generate_certificate
 @ddt.ddt
 class GenerateUserCertificateTest(TestCase):
     @patch('lms.djangoapps.certificates.tasks.generate_user_certificates')
-    def test_cert_task(self, generate_user_certs_mock):
-        generate_certificate(student='a', course_key='b', otherarg='c', otherotherarg='d')
-        generate_user_certs_mock.assert_called_with(student='a', course_key='b', otherarg='c', otherotherarg='d')
+    @patch('lms.djangoapps.certificates.tasks.User.objects.get')
+    def test_cert_task(self, user_get_mock, generate_user_certs_mock):
+        course_key = 'course-v1:edX+CS101+2017_T2'
+
+        generate_certificate(student='student-id', course_key=course_key, otherarg='c', otherotherarg='d')
+
+        expected_student = user_get_mock.return_value
+        generate_user_certs_mock.assert_called_with(
+            student=expected_student,
+            course_key=CourseKey.from_string(course_key),
+            otherarg='c',
+            otherotherarg='d'
+        )
+        user_get_mock.assert_called_once_with(id='student-id')
 
     @ddt.data('student', 'course_key')
     def test_cert_task_missing_args(self, missing_arg):
         kwargs = {'student': 'a', 'course_key': 'b', 'otherarg': 'c'}
         del kwargs[missing_arg]
-        with self.assertRaisesRegexp(KeyError, missing_arg):
-            generate_certificate(**kwargs)
+
+        with patch('lms.djangoapps.certificates.tasks.User.objects.get'):
+            with self.assertRaisesRegexp(KeyError, missing_arg):
+                generate_certificate(**kwargs)