feat: notify_credentials of changed overrides
We use the `notify_credentials` management command to keep certificate- related data in the LMS and Credentials service in sync. We can run it with specific arguments (user_ids, course_keys, etc.) when we notice a data discrepancy; and it is run regularly by a Jenkins job with the `--auto` flag every ~4 hours to keep things up-to-date. Because we probably never want to notify credentials of of ALL the GeneratedCertificates, the celery task must be given some arguments to filter down to the relevant certificates. Running the management command with the `--auto` flag (as the Jenkins job does) adds `start_date` and `end_date` arguments of 4 hours ago and now, respectively. The handle_notify_credentials celery task then takes those arguments and looks for any GeneratedCertificates that have been modified within the given time range by checking the GeneratedCertificate modified_date. It will send the current data for those certificates to credentials. However, we also want to notify credentials about certificates that have an associated CertificateDateOverride that has changed within that time range: added, updated, or deleted. But changes to a CertificateDateOverride won’t affect the GeneratedCertificate’s modified date, and therefore wouldn’t be included in the list of certs cent to credentials. This commit adds a check for changed CertificateDateOverrides and includes their associated GeneratedCertificates in the list of certs. We use the CertificateDateOverride’s history model for this check so that we can include certificates whose override was deleted. MICROBA-1489
This commit is contained in:
@@ -32,6 +32,7 @@ from lms.djangoapps.certificates.config import AUTO_CERTIFICATE_GENERATION as _A
|
||||
from lms.djangoapps.certificates.data import CertificateStatuses
|
||||
from lms.djangoapps.certificates.models import (
|
||||
CertificateAllowlist,
|
||||
CertificateDateOverride,
|
||||
CertificateGenerationConfiguration,
|
||||
CertificateGenerationCourseSetting,
|
||||
CertificateInvalidation,
|
||||
@@ -203,9 +204,53 @@ def get_recently_modified_certificates(course_keys=None, start_date=None, end_da
|
||||
if user_ids:
|
||||
cert_filter_args['user__id__in'] = user_ids
|
||||
|
||||
# Include certificates with a CertificateDateOverride modified within the
|
||||
# given time range.
|
||||
if start_date or end_date:
|
||||
certs_with_modified_overrides = get_certs_with_modified_overrides(course_keys, start_date, end_date, user_ids)
|
||||
return GeneratedCertificate.objects.filter(
|
||||
**cert_filter_args
|
||||
).union(
|
||||
certs_with_modified_overrides
|
||||
).order_by(
|
||||
'modified_date'
|
||||
)
|
||||
|
||||
return GeneratedCertificate.objects.filter(**cert_filter_args).order_by('modified_date')
|
||||
|
||||
|
||||
def get_certs_with_modified_overrides(course_keys=None, start_date=None, end_date=None, user_ids=None):
|
||||
"""
|
||||
Returns a QuerySet of certificates with a CertificateDateOverride modified
|
||||
within the given start_date and end_date. Uses the history table to catch
|
||||
deleted overrides too.
|
||||
"""
|
||||
override_filter_args = {}
|
||||
if start_date:
|
||||
override_filter_args['history_date__gte'] = start_date
|
||||
if end_date:
|
||||
override_filter_args['history_date__lte'] = end_date
|
||||
|
||||
# Get the HistoricalCertificateDateOverrides that have entries within the
|
||||
# given date range. We check the history table to catch deleted overrides
|
||||
# along with those created / updated
|
||||
overrides = CertificateDateOverride.history.filter(**override_filter_args)
|
||||
|
||||
# Get the associated GeneratedCertificate ids.
|
||||
override_cert_ids = overrides.values_list('generated_certificate', flat=True)
|
||||
|
||||
# Build the args for the GeneratedCertificate query. First, filter by all
|
||||
# certs identified in override_cert_ids; then by the other arguments passed,
|
||||
# if present.
|
||||
cert_filter_args = {'pk__in': override_cert_ids}
|
||||
if course_keys:
|
||||
cert_filter_args['course_id__in'] = course_keys
|
||||
if user_ids:
|
||||
cert_filter_args['user__id__in'] = user_ids
|
||||
|
||||
return GeneratedCertificate.objects.filter(**cert_filter_args)
|
||||
|
||||
|
||||
def generate_certificate_task(user, course_key, generation_mode=None):
|
||||
"""
|
||||
Create a task to generate a certificate for this user in this course run, if the user is eligible and a certificate
|
||||
|
||||
@@ -95,6 +95,8 @@ def handle_notify_credentials(options, course_keys):
|
||||
logger.exception('No site configuration found for site %s', options['site'])
|
||||
return
|
||||
|
||||
# If a start_date or end_date are passed, this will include certificates
|
||||
# with a CertificateDateOverride modified within the time range
|
||||
certs = get_recently_modified_certificates(
|
||||
course_keys, options['start_date'], options['end_date'], options['user_ids']
|
||||
)
|
||||
|
||||
@@ -18,7 +18,7 @@ from lms.djangoapps.certificates.data import CertificateStatuses
|
||||
from lms.djangoapps.certificates.models import GeneratedCertificate
|
||||
from lms.djangoapps.grades.models import PersistentCourseGrade
|
||||
from lms.djangoapps.grades.tests.utils import mock_passing_grade
|
||||
from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory
|
||||
from lms.djangoapps.certificates.tests.factories import CertificateDateOverrideFactory, GeneratedCertificateFactory
|
||||
from openedx.core.djangoapps.catalog.tests.factories import CourseFactory, CourseRunFactory, ProgramFactory
|
||||
from openedx.core.djangoapps.credentials.helpers import is_learner_records_enabled
|
||||
from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory
|
||||
@@ -155,6 +155,17 @@ class TestHandleNotifyCredentialsTask(TestCase):
|
||||
with freeze_time(datetime(2017, 5, 1, 3)):
|
||||
cert2 = GeneratedCertificateFactory(user=self.user, course_id='course-v1:edX+Test+22')
|
||||
|
||||
# `auto` execution should include certificates with date overrides
|
||||
# modified within the time range. See the next test
|
||||
# (`test_certs_with_modified_date_overrides`) for more.
|
||||
with freeze_time(datetime(2017, 4, 30, 0)):
|
||||
cert3 = GeneratedCertificateFactory(user=self.user, course_id='course-v1:edX+Test+33')
|
||||
with freeze_time(datetime(2017, 5, 1, 2)):
|
||||
CertificateDateOverrideFactory(
|
||||
generated_certificate=cert3,
|
||||
overridden_by=self.user,
|
||||
)
|
||||
|
||||
with freeze_time(datetime(2017, 5, 1, 0)):
|
||||
grade1 = PersistentCourseGrade.objects.create(user_id=self.user.id, course_id='course-v1:edX+Test+11',
|
||||
percent_grade=1)
|
||||
@@ -171,12 +182,73 @@ class TestHandleNotifyCredentialsTask(TestCase):
|
||||
tasks.handle_notify_credentials(options=self.options, course_keys=[])
|
||||
|
||||
assert mock_send.called
|
||||
self.assertListEqual(list(mock_send.call_args[0][0]), [cert1, cert2])
|
||||
self.assertListEqual(list(mock_send.call_args[0][0]), [cert3, cert1, cert2])
|
||||
self.assertListEqual(list(mock_send.call_args[0][1]), [grade1, grade2])
|
||||
|
||||
assert len(list(mock_send.call_args[0][0])) <= len(total_certificates)
|
||||
assert len(list(mock_send.call_args[0][1])) <= len(total_grades)
|
||||
|
||||
@mock.patch(TASKS_MODULE + '.send_notifications')
|
||||
def test_certs_with_modified_date_overrides(self, mock_send):
|
||||
# Set up a time range for the call
|
||||
self.options['start_date'] = '2017-05-01T00:00:00'
|
||||
self.options['end_date'] = '2017-05-04T04:00:00'
|
||||
|
||||
# First, call the task without adding any overrides to verify that the
|
||||
# certs would not be included because of their modified_date
|
||||
tasks.handle_notify_credentials(options=self.options, course_keys=[])
|
||||
assert mock_send.called
|
||||
assert self.cert1 not in mock_send.call_args[0][0]
|
||||
assert self.cert2 not in mock_send.call_args[0][0]
|
||||
assert self.cert3 not in mock_send.call_args[0][0]
|
||||
|
||||
# ADD
|
||||
# cert1 was created January 1, 2017. We add a date override to it on
|
||||
# May 1, 2017.
|
||||
with freeze_time(datetime(2017, 5, 1, 1)):
|
||||
CertificateDateOverrideFactory(
|
||||
generated_certificate=self.cert1,
|
||||
overridden_by=self.user,
|
||||
)
|
||||
|
||||
# UPDATE
|
||||
# cert2 was created February 1, 2017. We add a date override to it on
|
||||
# April 1, 2017; but edit the date override on May 2, 2017.
|
||||
with freeze_time(datetime(2017, 4, 1)):
|
||||
cert2_override = CertificateDateOverrideFactory(
|
||||
generated_certificate=self.cert2,
|
||||
overridden_by=self.user,
|
||||
)
|
||||
|
||||
with freeze_time(datetime(2017, 5, 2, 1)):
|
||||
cert2_override.date = datetime(2018, 1, 1)
|
||||
cert2_override.save()
|
||||
|
||||
# DELETE
|
||||
# cert3 was created March 1, 2017. We add a date override to it on April
|
||||
# 2, 2017; but delete the override on May 3, 2017.
|
||||
with freeze_time(datetime(2017, 4, 2)):
|
||||
cert3_override = CertificateDateOverrideFactory(
|
||||
generated_certificate=self.cert3,
|
||||
overridden_by=self.user,
|
||||
)
|
||||
with freeze_time(datetime(2017, 5, 3, 1)):
|
||||
cert3_override.delete()
|
||||
|
||||
# None of the certs have modified dates within the time range, but they
|
||||
# each have date overrides that were added, updated, or deleted within
|
||||
# the time range.
|
||||
|
||||
tasks.handle_notify_credentials(options=self.options, course_keys=[])
|
||||
|
||||
# The three certs should now be included in the arguments because of the
|
||||
# the altered date overrides.
|
||||
assert mock_send.called
|
||||
self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert1, self.cert2, self.cert3])
|
||||
assert self.cert1 in mock_send.call_args[0][0]
|
||||
assert self.cert2 in mock_send.call_args[0][0]
|
||||
assert self.cert3 in mock_send.call_args[0][0]
|
||||
|
||||
@mock.patch(TASKS_MODULE + '.send_notifications')
|
||||
def test_date_args(self, mock_send):
|
||||
self.options['start_date'] = '2017-01-31T00:00:00Z'
|
||||
|
||||
Reference in New Issue
Block a user