diff --git a/openedx/core/djangoapps/programs/signals.py b/openedx/core/djangoapps/programs/signals.py index 25e3c4cf03644362adf87ef9adf0b18ed39f5eec..ecbae8860652a3ee9945a0b6d250b8cf397e2162 100644 --- a/openedx/core/djangoapps/programs/signals.py +++ b/openedx/core/djangoapps/programs/signals.py @@ -94,4 +94,4 @@ def handle_course_cert_changed(sender, user, course_key, mode, status, **kwargs) ) # import here, because signal is registered at startup, but items in tasks are not yet able to be loaded from openedx.core.djangoapps.programs.tasks.v1.tasks import award_course_certificate - award_course_certificate.delay(user.username, course_key) + award_course_certificate.delay(user.username, str(course_key)) diff --git a/openedx/core/djangoapps/programs/tasks/v1/tasks.py b/openedx/core/djangoapps/programs/tasks/v1/tasks.py index 67cee1283f955aa924045d4d7e24aafb8cd9c2ff..70187157623b98450967ea4417ed06cf17e13f19 100644 --- a/openedx/core/djangoapps/programs/tasks/v1/tasks.py +++ b/openedx/core/djangoapps/programs/tasks/v1/tasks.py @@ -7,6 +7,7 @@ from django.conf import settings from django.contrib.auth.models import User from django.contrib.sites.models import Site from edx_rest_api_client import exceptions +from opaque_keys.edx.keys import CourseKey from course_modes.models import CourseMode from lms.djangoapps.certificates.models import GeneratedCertificate @@ -16,7 +17,6 @@ from openedx.core.djangoapps.credentials.models import CredentialsApiConfig from openedx.core.djangoapps.credentials.utils import get_credentials, get_credentials_api_client from openedx.core.djangoapps.programs.utils import ProgramProgressMeter - LOGGER = get_task_logger(__name__) # Under cms the following setting is not defined, leading to errors during tests. ROUTING_KEY = getattr(settings, 'CREDENTIALS_GENERATION_ROUTING_KEY', None) @@ -221,7 +221,6 @@ def award_course_certificate(self, username, course_run_key): This task is designed to be called whenever a student GeneratedCertificate is updated. It can be called independently for a username and a course_run, but is invoked on each GeneratedCertificate.save. """ - LOGGER.info('Running task award_course_certificate for username %s', username) countdown = 2 ** self.request.retries @@ -238,6 +237,7 @@ def award_course_certificate(self, username, course_run_key): raise self.retry(countdown=countdown, max_retries=MAX_RETRIES) try: + course_key = CourseKey.from_string(course_run_key) try: user = User.objects.get(username=username) except User.DoesNotExist: @@ -248,27 +248,27 @@ def award_course_certificate(self, username, course_run_key): try: certificate = GeneratedCertificate.eligible_certificates.get( user=user.id, - course_id=course_run_key + course_id=course_key ) except GeneratedCertificate.DoesNotExist: LOGGER.exception( 'Task award_course_certificate was called without Certificate found for %s to user %s', - course_run_key, + course_key, username ) return if certificate.mode in CourseMode.VERIFIED_MODES + CourseMode.CREDIT_MODES: try: - course_overview = CourseOverview.get_from_id(course_run_key) + course_overview = CourseOverview.get_from_id(course_key) except (CourseOverview.DoesNotExist, IOError): LOGGER.exception( 'Task award_course_certificate was called without course overview data for course %s', - course_run_key + course_key ) return credentials_client = get_credentials_api_client(User.objects.get( username=settings.CREDENTIALS_SERVICE_USERNAME), - org=course_run_key.org, + org=course_key.org, ) # FIXME This may result in visible dates that do not update alongside the Course Overview if that changes # This is a known limitation of this implementation and was chosen to reduce the amount of replication, @@ -276,7 +276,7 @@ def award_course_certificate(self, username, course_run_key): visible_date = display_date_for_certificate(course_overview, certificate) post_course_certificate(credentials_client, username, certificate, visible_date) - LOGGER.info('Awarded certificate for course %s to user %s', course_run_key, username) + LOGGER.info('Awarded certificate for course %s to user %s', course_key, username) except Exception as exc: LOGGER.exception('Failed to determine course certificates to be awarded for user %s', username) raise self.retry(exc=exc, countdown=countdown, max_retries=MAX_RETRIES) diff --git a/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py b/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py index 728db00f2cb8da431e41d7401d4b31e508b1394d..0a0dbedde0d36db4eff9e709bd419132edd0867a 100644 --- a/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py +++ b/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py @@ -392,7 +392,7 @@ class AwardCourseCertificatesTestCase(CredentialsApiConfigMixin, TestCase): """ Tests the API POST method is called with appropriate params when configured properly """ - tasks.award_course_certificate.delay(self.student.username, self.course.id).get() + tasks.award_course_certificate.delay(self.student.username, str(self.course.id)).get() call_args, _ = mock_post_course_certificate.call_args self.assertEqual(call_args[1], self.student.username) self.assertEqual(call_args[2], self.certificate) @@ -404,7 +404,7 @@ class AwardCourseCertificatesTestCase(CredentialsApiConfigMixin, TestCase): self.create_credentials_config(enabled=False) with mock.patch(TASKS_MODULE + '.LOGGER.warning') as mock_warning: with self.assertRaises(MaxRetriesExceededError): - tasks.award_course_certificate.delay(self.student.username, self.course.id).get() + tasks.award_course_certificate.delay(self.student.username, str(self.course.id)).get() self.assertTrue(mock_warning.called) self.assertFalse(mock_post_course_certificate.called) @@ -414,7 +414,7 @@ class AwardCourseCertificatesTestCase(CredentialsApiConfigMixin, TestCase): """ with mock.patch(TASKS_MODULE + '.LOGGER.exception') as mock_exception: # Use a random username here since this user won't be found in the DB - tasks.award_course_certificate.delay('random_username', self.course.id).get() + tasks.award_course_certificate.delay('random_username', str(self.course.id)).get() self.assertTrue(mock_exception.called) self.assertFalse(mock_post_course_certificate.called) @@ -424,7 +424,7 @@ class AwardCourseCertificatesTestCase(CredentialsApiConfigMixin, TestCase): """ self.certificate.delete() with mock.patch(TASKS_MODULE + '.LOGGER.exception') as mock_exception: - tasks.award_course_certificate.delay(self.student.username, self.course.id).get() + tasks.award_course_certificate.delay(self.student.username, str(self.course.id)).get() self.assertTrue(mock_exception.called) self.assertFalse(mock_post_course_certificate.called) @@ -435,7 +435,7 @@ class AwardCourseCertificatesTestCase(CredentialsApiConfigMixin, TestCase): self.course.delete() with mock.patch(TASKS_MODULE + '.LOGGER.exception') as mock_exception: # Use the certificate course id here since the course will be deleted - tasks.award_course_certificate.delay(self.student.username, self.certificate.course_id).get() + tasks.award_course_certificate.delay(self.student.username, str(self.certificate.course_id)).get() self.assertTrue(mock_exception.called) self.assertFalse(mock_post_course_certificate.called) @@ -449,5 +449,5 @@ class AwardCourseCertificatesTestCase(CredentialsApiConfigMixin, TestCase): self.certificate.save() self.create_credentials_config() - tasks.award_course_certificate.delay(self.student.username, self.certificate.course_id).get() + tasks.award_course_certificate.delay(self.student.username, str(self.certificate.course_id)).get() self.assertFalse(mock_post_course_certificate.called)