diff --git a/lms/djangoapps/certificates/api.py b/lms/djangoapps/certificates/api.py index c88c656391..8939e6e805 100644 --- a/lms/djangoapps/certificates/api.py +++ b/lms/djangoapps/certificates/api.py @@ -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 diff --git a/openedx/core/djangoapps/credentials/tasks/v1/tasks.py b/openedx/core/djangoapps/credentials/tasks/v1/tasks.py index 0587f3ce4a..c3b767c815 100644 --- a/openedx/core/djangoapps/credentials/tasks/v1/tasks.py +++ b/openedx/core/djangoapps/credentials/tasks/v1/tasks.py @@ -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'] ) diff --git a/openedx/core/djangoapps/credentials/tests/test_tasks.py b/openedx/core/djangoapps/credentials/tests/test_tasks.py index fbc15280e5..18c9403b16 100644 --- a/openedx/core/djangoapps/credentials/tests/test_tasks.py +++ b/openedx/core/djangoapps/credentials/tests/test_tasks.py @@ -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'