diff --git a/lms/djangoapps/certificates/signals.py b/lms/djangoapps/certificates/signals.py index 572bd3cf58..e8ccff4844 100644 --- a/lms/djangoapps/certificates/signals.py +++ b/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 diff --git a/lms/djangoapps/certificates/tasks.py b/lms/djangoapps/certificates/tasks.py index 9ccd36bb3e..c297d2f516 100644 --- a/lms/djangoapps/certificates/tasks.py +++ b/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) diff --git a/lms/djangoapps/certificates/tests/test_signals.py b/lms/djangoapps/certificates/tests/test_signals.py index 8bbe406478..b006e8c349 100644 --- a/lms/djangoapps/certificates/tests/test_signals.py +++ b/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), + }) diff --git a/lms/djangoapps/certificates/tests/test_tasks.py b/lms/djangoapps/certificates/tests/test_tasks.py index 9572e279a2..2f391b682e 100644 --- a/lms/djangoapps/certificates/tests/test_tasks.py +++ b/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)