fix: Pass date in cert date update signal

Because the available date update to the CourseOverview happens inside a
view's signal and we have atomic requests on, the read that was
happening inside the task happened *before* the write was commited to
the database. To avoid the unknown bugs that would come from disabling
atomic transactions for that view (since it's large), this passes the
date we want down through the signals and tasks so we can skip the DB
read at the end.
This commit is contained in:
Matt Tuchfarber
2021-03-12 13:57:50 -05:00
parent 027704adae
commit 7dd4a2b6fd
5 changed files with 48 additions and 18 deletions

View File

@@ -81,9 +81,17 @@ def _course_uses_available_date(course):
return can_show_certificate_available_date_field(course) and course.certificate_available_date
def available_date_for_certificate(course, certificate):
def available_date_for_certificate(course, certificate, certificate_available_date=None):
"""
Returns the available date to use with a certificate
Arguments:
course (CourseOverview): 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 course.certificate_available_date
return certificate_available_date or course.certificate_available_date
return certificate.modified_date

View File

@@ -95,4 +95,8 @@ 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)
COURSE_CERT_DATE_CHANGE.send_robust(
sender=None,
course_key=updated_course_overview.id,
available_date=updated_course_overview.certificate_available_date
)

View File

@@ -182,15 +182,15 @@ 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, **kwargs): # lint-amnesty, pylint: disable=unused-argument
def handle_course_cert_date_change(sender, course_key, available_date, **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
within course.
Args:
course_key:
refers to the course whose certificate_available_date was updated.
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
@@ -216,4 +216,4 @@ def handle_course_cert_date_change(sender, course_key, **kwargs): # lint-amnest
# 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))
update_certificate_visible_date_on_course_update.delay(str(course_key), available_date)

View File

@@ -1,7 +1,7 @@
"""
This file contains celery tasks for programs-related functionality.
"""
from datetime import datetime
from celery import shared_task
from celery.exceptions import MaxRetriesExceededError
@@ -300,10 +300,17 @@ def post_course_certificate(client, username, certificate, visible_date):
@shared_task(bind=True, ignore_result=True)
@set_code_owner_attribute
def award_course_certificate(self, username, course_run_key):
def award_course_certificate(self, username, course_run_key, certificate_available_date=None):
"""
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.
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(
@@ -368,10 +375,19 @@ def award_course_certificate(self, username, course_run_key):
username=settings.CREDENTIALS_SERVICE_USERNAME),
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,
# endpoints, celery tasks, and jenkins jobs that needed to be written for this functionality
visible_date = available_date_for_certificate(course_overview, certificate)
# Date is being passed via JSON and is encoded in the EMCA date time string format. The rest of the code
# expects a datetime.
certificate_available_date = datetime.strptime(certificate_available_date, VISIBLE_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}"
@@ -588,7 +604,7 @@ def revoke_program_certificates(self, username, course_key):
@shared_task(bind=True, ignore_result=True)
@set_code_owner_attribute
def update_certificate_visible_date_on_course_update(self, course_key):
def update_certificate_visible_date_on_course_update(self, course_key, certificate_available_date):
"""
This task is designed to be called whenever a course is updated with
certificate_available_date so that visible_date is updated on credential
@@ -598,8 +614,10 @@ def update_certificate_visible_date_on_course_update(self, course_key):
the credentials API to update all these certificates visible_date value
to keep certificates in sync on both sides.
Args:
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
@@ -626,8 +644,8 @@ def update_certificate_visible_date_on_course_update(self, course_key):
).values_list('user__username', flat=True)
LOGGER.info(
"Task update_certificate_visible_date_on_course_update resending course certificates"
"Task update_certificate_visible_date_on_course_update resending course certificates "
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))
award_course_certificate.delay(user, str(course_key), certificate_available_date=certificate_available_date)

View File

@@ -14,7 +14,7 @@ COURSE_GRADE_CHANGED = Signal(providing_args=["user", "course_grade", "course_ke
COURSE_CERT_CHANGED = Signal(providing_args=["user", "course_key", "mode", "status"])
COURSE_CERT_AWARDED = Signal(providing_args=["user", "course_key", "mode", "status"])
COURSE_CERT_REVOKED = Signal(providing_args=["user", "course_key", "mode", "status"])
COURSE_CERT_DATE_CHANGE = Signal(providing_args=["course_key"])
COURSE_CERT_DATE_CHANGE = Signal(providing_args=["course_key", "available_date"])
COURSE_ASSESSMENT_GRADE_CHANGED = Signal(