diff --git a/lms/djangoapps/certificates/api.py b/lms/djangoapps/certificates/api.py index 5c0b1b4329..48fe0f7ef1 100644 --- a/lms/djangoapps/certificates/api.py +++ b/lms/djangoapps/certificates/api.py @@ -874,17 +874,16 @@ def _course_uses_available_date(course): ) -def available_date_for_certificate(course, certificate, certificate_available_date=None): +def available_date_for_certificate(course, certificate): """ Returns the available date to use with a certificate Arguments: course (CourseOverview or course descriptor): The course we're checking certificate (GeneratedCertificate): The certificate we're getting the date for - certificate_available_date (datetime): An optional date to override the from the course overview. """ if _course_uses_available_date(course): - return certificate_available_date or course.certificate_available_date + return course.certificate_available_date return certificate.modified_date diff --git a/openedx/core/djangoapps/content/course_overviews/signals.py b/openedx/core/djangoapps/content/course_overviews/signals.py index e0d59ab6fd..d415ad360d 100644 --- a/openedx/core/djangoapps/content/course_overviews/signals.py +++ b/openedx/core/djangoapps/content/course_overviews/signals.py @@ -5,6 +5,7 @@ Signal handler for invalidating cached course overviews import logging +from django.db import transaction from django.dispatch import Signal from django.dispatch.dispatcher import receiver @@ -92,8 +93,11 @@ def _check_for_cert_availability_date_changes(previous_course_overview, updated_ f"{previous_course_overview.certificate_available_date} to " + f"{updated_course_overview.certificate_available_date}. Sending COURSE_CERT_DATE_CHANGE signal." ) - COURSE_CERT_DATE_CHANGE.send_robust( - sender=None, - course_key=updated_course_overview.id, - available_date=updated_course_overview.certificate_available_date - ) + + def _send_course_cert_date_change_signal(): + COURSE_CERT_DATE_CHANGE.send_robust( + sender=None, + course_key=updated_course_overview.id, + ) + + transaction.on_commit(_send_course_cert_date_change_signal) diff --git a/openedx/core/djangoapps/content/course_overviews/tests/test_signals.py b/openedx/core/djangoapps/content/course_overviews/tests/test_signals.py index 4b486f3616..9506a584bb 100644 --- a/openedx/core/djangoapps/content/course_overviews/tests/test_signals.py +++ b/openedx/core/djangoapps/content/course_overviews/tests/test_signals.py @@ -84,14 +84,16 @@ class CourseOverviewSignalsTestCase(ModuleStoreTestCase): ) # changing display name doesn't fire the signal - course.display_name = course.display_name + 'changed' - self.store.update_item(course, ModuleStoreEnum.UserID.test) + with self.captureOnCommitCallbacks(execute=True) as callbacks: + course.display_name = course.display_name + 'changed' + self.store.update_item(course, ModuleStoreEnum.UserID.test) assert not mock_signal.called # changing the given field fires the signal - for change in changes: - setattr(course, change.field_name, change.changed_value) - self.store.update_item(course, ModuleStoreEnum.UserID.test) + with self.captureOnCommitCallbacks(execute=True) as callbacks: + for change in changes: + setattr(course, change.field_name, change.changed_value) + self.store.update_item(course, ModuleStoreEnum.UserID.test) assert mock_signal.called @patch('openedx.core.djangoapps.content.course_overviews.signals.COURSE_START_DATE_CHANGED.send') diff --git a/openedx/core/djangoapps/programs/signals.py b/openedx/core/djangoapps/programs/signals.py index a548f597a6..ec9e3a5128 100644 --- a/openedx/core/djangoapps/programs/signals.py +++ b/openedx/core/djangoapps/programs/signals.py @@ -177,7 +177,7 @@ def handle_course_cert_revoked(sender, user, course_key, mode, status, **kwargs) @receiver(COURSE_CERT_DATE_CHANGE, dispatch_uid='course_certificate_date_change_handler') -def handle_course_cert_date_change(sender, course_key, available_date, **kwargs): # lint-amnesty, pylint: disable=unused-argument +def handle_course_cert_date_change(sender, course_key, **kwargs): # lint-amnesty, pylint: disable=unused-argument """ If course is updated and the certificate_available_date is changed, schedule a celery task to update visible_date for all certificates @@ -185,7 +185,6 @@ def handle_course_cert_date_change(sender, course_key, available_date, **kwargs) Args: course_key (CourseLocator): refers to the course whose certificate_available_date was updated. - available_date (datetime): the date to update the certificate's available date to Returns: None @@ -208,4 +207,4 @@ def handle_course_cert_date_change(sender, course_key, available_date, **kwargs) # import here, because signal is registered at startup, but items in tasks are not yet loaded from openedx.core.djangoapps.programs.tasks import update_certificate_visible_date_on_course_update - update_certificate_visible_date_on_course_update.delay(str(course_key), available_date) + update_certificate_visible_date_on_course_update.delay(str(course_key)) diff --git a/openedx/core/djangoapps/programs/tasks.py b/openedx/core/djangoapps/programs/tasks.py index e6d1355ee7..38f95756d4 100644 --- a/openedx/core/djangoapps/programs/tasks.py +++ b/openedx/core/djangoapps/programs/tasks.py @@ -1,7 +1,6 @@ """ This file contains celery tasks for programs-related functionality. """ -from datetime import datetime from urllib.parse import urljoin from celery import shared_task @@ -347,14 +346,16 @@ def update_credentials_course_certificate_configuration_available_date( certificate_available_date=None ): """ - This task will update the course certificate configuration's available date. This is different from the - "visable_date" attribute. This date will always either be the available date that is set in studio for - a given course, or it will be None. + This task will update the CourseCertificate configuration's available date + in Credentials. This is different from the "visible_date" attribute. This + date will always either be the available date that is set in Studio for a + given course, or it will be None. Arguments: course_run_key (str): The course run key to award the certificate for - certificate_available_date (str): A string representation of the datetime for when to make the certificate - available to the user. If not provided, it will be none. + certificate_available_date (str): A string representation of the + datetime for when to make the certificate available to the user. If + not provided, it will be None. """ LOGGER.info( f"Running task update_credentials_course_certificate_configuration_available_date for course {course_key} \ @@ -386,7 +387,7 @@ def update_credentials_course_certificate_configuration_available_date( @shared_task(bind=True, ignore_result=True) @set_code_owner_attribute -def award_course_certificate(self, username, course_run_key, certificate_available_date=None): +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. @@ -397,9 +398,6 @@ def award_course_certificate(self, username, course_run_key, certificate_availab Arguments: username (str): The user to award the Credentials course cert to course_run_key (str): The course run key to award the certificate for - certificate_available_date (str): A string representation of the datetime for when to make the certificate - available to the user. If not provided, it will calculate the date. - """ def _retry_with_custom_exception(username, course_run_key, reason, countdown): exception = MaxRetriesExceededError( @@ -460,23 +458,16 @@ def award_course_certificate(self, username, course_run_key, certificate_availab f"Task award_course_certificate was called without course overview data for course {course_key}" ) return + credentials_client = get_credentials_api_client( User.objects.get(username=settings.CREDENTIALS_SERVICE_USERNAME), ) - # Date is being passed via JSON and is encoded in the EMCA date time string format. The rest of the code - # expects a datetime. - if certificate_available_date: - certificate_available_date = datetime.strptime(certificate_available_date, DATE_FORMAT) - - # Even in the cases where this task is called with a certificate_available_date, we still need to retrieve - # the course overview because it's required to determine if we should use the certificate_available_date or - # the certs modified date visible_date = available_date_for_certificate( course_overview, certificate, - certificate_available_date=certificate_available_date ) + LOGGER.info( "Task award_course_certificate will award certificate for course " f"{course_key} with a visible date of {visible_date}" @@ -713,7 +704,7 @@ def revoke_program_certificates(self, username, course_key): # lint-amnesty, py @shared_task(bind=True, ignore_result=True) @set_code_owner_attribute -def update_certificate_visible_date_on_course_update(self, course_key, certificate_available_date): +def update_certificate_visible_date_on_course_update(self, course_key): """ This task is designed to be called whenever a course is updated with certificate_available_date so that visible_date is updated on credential @@ -728,8 +719,6 @@ def update_certificate_visible_date_on_course_update(self, course_key, certifica Arguments: course_key (str): The course identifier - certificate_available_date (str): The date to update the certificate availablity date to. It's a string - representation of a datetime object because task parameters must be JSON-able. Returns: None @@ -750,9 +739,11 @@ def update_certificate_visible_date_on_course_update(self, course_key, certifica f"Failed to update certificate availability date for course {course_key}. Reason: {error_msg}" ) raise self.retry(exc=exception, countdown=countdown, max_retries=MAX_RETRIES) - # update the course certificate with the new certificate available date if: - # - The course is not self paced - # - The certificates_display_behavior is not "end_with_date" + + # Update the CourseCertificate configuration in Credentials with the new + # certificate_available_date if: + # - The course is not self paced, AND + # - The certificates_display_behavior is "end_with_date" course_overview = CourseOverview.get_from_id(course_key) if ( course_overview.self_paced is False and @@ -760,8 +751,12 @@ def update_certificate_visible_date_on_course_update(self, course_key, certifica ): update_credentials_course_certificate_configuration_available_date.delay( str(course_key), - certificate_available_date + str(course_overview.certificate_available_date) ) + + # This code will update the visible_date in Credentials; we have moved away + # from relying on visible_date in favor of the above, but this still runs + # and visible_date is still updated users_with_certificates_in_course = GeneratedCertificate.eligible_available_certificates.filter( course_id=course_key ).values_list('user__username', flat=True) @@ -771,4 +766,4 @@ def update_certificate_visible_date_on_course_update(self, course_key, certifica f"for {len(users_with_certificates_in_course)} users in course {course_key}." ) for user in users_with_certificates_in_course: - award_course_certificate.delay(user, str(course_key), certificate_available_date=certificate_available_date) + award_course_certificate.delay(user, str(course_key)) diff --git a/openedx/core/djangoapps/signals/signals.py b/openedx/core/djangoapps/signals/signals.py index f995fd91e8..d11caaa24d 100644 --- a/openedx/core/djangoapps/signals/signals.py +++ b/openedx/core/djangoapps/signals/signals.py @@ -16,7 +16,7 @@ COURSE_GRADE_CHANGED = Signal() COURSE_CERT_CHANGED = Signal() COURSE_CERT_AWARDED = Signal() COURSE_CERT_REVOKED = Signal() -# providing_args=["course_key", "available_date"] +# providing_args=["course_key",] COURSE_CERT_DATE_CHANGE = Signal() # providing_args=['user', 'course_id', 'subsection_id', 'subsection_grade', ]