diff --git a/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py b/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py index ea8f90c8d1e021405a4b200cd515f226308e1bb3..17cefccbee20cf35ea57d8aa3a3172897a92d508 100644 --- a/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py +++ b/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py @@ -13,6 +13,7 @@ signals.) import logging import math +import shlex import sys import time @@ -28,8 +29,9 @@ from six.moves import range from lms.djangoapps.certificates.api import get_recently_modified_certificates from lms.djangoapps.grades.api import get_recently_modified_grades from openedx.core.djangoapps.credentials.models import NotifyCredentialsConfig +from lms.djangoapps.certificates.models import CertificateStatuses from openedx.core.djangoapps.credentials.signals import handle_cert_change, send_grade_if_interesting -from openedx.core.djangoapps.programs.signals import handle_course_cert_changed +from openedx.core.djangoapps.programs.signals import handle_course_cert_changed, handle_course_cert_awarded from openedx.core.djangoapps.site_configuration.models import SiteConfiguration log = logging.getLogger(__name__) @@ -153,6 +155,11 @@ class Command(BaseCommand): action='store_true', help='Run grade/cert change signal in verbose mode', ) + parser.add_argument( + '--notify_programs', + action='store_true', + help='Send program award notifications with course notification tasks', + ) def get_args_from_database(self): """ Returns an options dictionary from the current NotifyCredentialsConfig model. """ @@ -160,9 +167,9 @@ class Command(BaseCommand): if not config.enabled: raise CommandError('NotifyCredentialsConfig is disabled, but --args-from-database was requested.') - # We don't need fancy shell-style whitespace/quote handling - none of our arguments are complicated - argv = config.arguments.split() - + # This split will allow for quotes to wrap datetimes, like "2020-10-20 04:00:00" and other + # arguments as if it were the command line + argv = shlex.split(config.arguments) parser = self.create_parser('manage.py', 'notify_credentials') return parser.parse_args(argv).__dict__ # we want a dictionary, not a non-iterable Namespace object @@ -176,13 +183,14 @@ class Command(BaseCommand): log.info( u"notify_credentials starting, dry-run=%s, site=%s, delay=%d seconds, page_size=%d, " - u"from=%s, to=%s, execution=%s", + u"from=%s, to=%s, notify_programs=%s, execution=%s", options['dry_run'], options['site'], options['delay'], options['page_size'], options['start_date'] if options['start_date'] else 'NA', options['end_date'] if options['end_date'] else 'NA', + options['notify_programs'], 'auto' if options['auto'] else 'manual', ) @@ -198,18 +206,28 @@ class Command(BaseCommand): certs = get_recently_modified_certificates(course_keys, options['start_date'], options['end_date']) grades = get_recently_modified_grades(course_keys, options['start_date'], options['end_date']) + log.info('notify_credentials Sending notifications for {certs} certificates and {grades} grades'.format( + certs=certs.count(), + grades=grades.count() + )) if options['dry_run']: self.print_dry_run(certs, grades) else: - self.send_notifications(certs, grades, - site_config=site_config, - delay=options['delay'], - page_size=options['page_size'], - verbose=options['verbose']) + self.send_notifications( + certs, + grades, + site_config=site_config, + delay=options['delay'], + page_size=options['page_size'], + verbose=options['verbose'], + notify_programs=options['notify_programs'] + ) log.info('notify_credentials finished') - def send_notifications(self, certs, grades, site_config=None, delay=0, page_size=0, verbose=False): + def send_notifications( + self, certs, grades, site_config=None, delay=0, page_size=0, verbose=False, notify_programs=False + ): """ Run actual handler commands for the provided certs and grades. """ course_cert_info = {} @@ -240,6 +258,8 @@ class Command(BaseCommand): course_cert_info[(cert.user.id, str(cert.course_id))] = data handle_course_cert_changed(**signal_args) + if notify_programs and CertificateStatuses.is_passing_status(cert.status): + handle_course_cert_awarded(**signal_args) # Then do grades for i, grade in paged_query(grades, delay, page_size): diff --git a/openedx/core/djangoapps/credentials/management/commands/tests/test_notify_credentials.py b/openedx/core/djangoapps/credentials/management/commands/tests/test_notify_credentials.py index a4d309561aece2a3f99459d3b4c226fea8305a5b..b2b7b37a6940a1a5567859c8241db5ebd469149a 100644 --- a/openedx/core/djangoapps/credentials/management/commands/tests/test_notify_credentials.py +++ b/openedx/core/djangoapps/credentials/management/commands/tests/test_notify_credentials.py @@ -13,7 +13,7 @@ from django.test import TestCase, override_settings from freezegun import freeze_time from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory -from lms.djangoapps.certificates.models import GeneratedCertificate +from lms.djangoapps.certificates.models import GeneratedCertificate, CertificateStatuses from lms.djangoapps.grades.models import PersistentCourseGrade from openedx.core.djangoapps.credentials.models import NotifyCredentialsConfig from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory @@ -36,10 +36,14 @@ class TestNotifyCredentials(TestCase): with freeze_time(datetime(2017, 1, 1)): self.cert1 = GeneratedCertificateFactory(user=self.user, course_id='course-v1:edX+Test+1') - with freeze_time(datetime(2017, 2, 1)): + with freeze_time(datetime(2017, 2, 1, 0)): self.cert2 = GeneratedCertificateFactory(user=self.user, course_id='course-v1:edX+Test+2') with freeze_time(datetime(2017, 3, 1)): self.cert3 = GeneratedCertificateFactory(user=self.user, course_id='course-v1:testX+Test+3') + with freeze_time(datetime(2017, 2, 1, 5)): + self.cert4 = GeneratedCertificateFactory( + user=self.user, course_id='course-v1:edX+Test+4', status=CertificateStatuses.downloadable + ) print(('self.cert1.modified_date', self.cert1.modified_date)) # No factory for these @@ -52,6 +56,9 @@ class TestNotifyCredentials(TestCase): with freeze_time(datetime(2017, 3, 1)): self.grade3 = PersistentCourseGrade.objects.create(user_id=self.user.id, course_id='course-v1:testX+Test+3', percent_grade=1) + with freeze_time(datetime(2017, 2, 1, 5)): + self.grade4 = PersistentCourseGrade.objects.create(user_id=self.user.id, course_id='course-v1:edX+Test+4', + percent_grade=1) print(('self.grade1.modified', self.grade1.modified)) @mock.patch(COMMAND_MODULE + '.Command.send_notifications') @@ -94,20 +101,26 @@ class TestNotifyCredentials(TestCase): def test_date_args(self, mock_send): call_command(Command(), '--start-date', '2017-01-31') self.assertTrue(mock_send.called) - self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert2, self.cert3]) - self.assertListEqual(list(mock_send.call_args[0][1]), [self.grade2, self.grade3]) + self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert2, self.cert4, self.cert3]) + self.assertListEqual(list(mock_send.call_args[0][1]), [self.grade2, self.grade4, self.grade3]) mock_send.reset_mock() call_command(Command(), '--start-date', '2017-02-01', '--end-date', '2017-02-02') self.assertTrue(mock_send.called) - self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert2]) - self.assertListEqual(list(mock_send.call_args[0][1]), [self.grade2]) + self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert2, self.cert4]) + self.assertListEqual(list(mock_send.call_args[0][1]), [self.grade2, self.grade4]) mock_send.reset_mock() call_command(Command(), '--end-date', '2017-02-02') self.assertTrue(mock_send.called) - self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert1, self.cert2]) - self.assertListEqual(list(mock_send.call_args[0][1]), [self.grade1, self.grade2]) + self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert1, self.cert2, self.cert4]) + self.assertListEqual(list(mock_send.call_args[0][1]), [self.grade1, self.grade2, self.grade4]) + mock_send.reset_mock() + + call_command(Command(), '--start-date', "2017-02-01 00:00:00", '--end-date', '2017-02-01 04:00:00') + self.assertTrue(mock_send.called) + self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert2]) + self.assertListEqual(list(mock_send.call_args[0][1]), [self.grade2]) @mock.patch(COMMAND_MODULE + '.Command.send_notifications') def test_no_args(self, mock_send): @@ -120,12 +133,22 @@ class TestNotifyCredentials(TestCase): call_command(Command(), '--dry-run', '--start-date', '2017-02-01') self.assertFalse(mock_send.called) + @mock.patch(COMMAND_MODULE + '.handle_course_cert_awarded') @mock.patch(COMMAND_MODULE + '.send_grade_if_interesting') @mock.patch(COMMAND_MODULE + '.handle_course_cert_changed') - def test_hand_off(self, mock_grade_interesting, mock_program_changed): + def test_hand_off(self, mock_grade_interesting, mock_program_changed, mock_program_awarded): call_command(Command(), '--start-date', '2017-02-01') - self.assertEqual(mock_grade_interesting.call_count, 2) - self.assertEqual(mock_program_changed.call_count, 2) + self.assertEqual(mock_grade_interesting.call_count, 3) + self.assertEqual(mock_program_changed.call_count, 3) + self.assertEqual(mock_program_awarded.call_count, 0) + mock_grade_interesting.reset_mock() + mock_program_changed.reset_mock() + mock_program_awarded.reset_mock() + + call_command(Command(), '--start-date', '2017-02-01', '--notify_programs') + self.assertEqual(mock_grade_interesting.call_count, 3) + self.assertEqual(mock_program_changed.call_count, 3) + self.assertEqual(mock_program_awarded.call_count, 1) @mock.patch(COMMAND_MODULE + '.time') def test_delay(self, mock_time): @@ -145,7 +168,7 @@ class TestNotifyCredentials(TestCase): reset_queries() call_command(Command(), '--start-date', '2017-01-01', '--page-size=1') - self.assertEqual(len(connection.queries), baseline + 4) # two extra page queries each for certs & grades + self.assertEqual(len(connection.queries), baseline + 6) # two extra page queries each for certs & grades reset_queries() call_command(Command(), '--start-date', '2017-01-01', '--page-size=2') @@ -168,13 +191,13 @@ class TestNotifyCredentials(TestCase): # Add a config config = NotifyCredentialsConfig.current() - config.arguments = '--start-date 2017-03-01' + config.arguments = '--start-date "2017-03-01 00:00:00"' config.enabled = True config.save() # Not told to use config, should ignore it call_command(Command(), '--start-date', '2017-01-01') - self.assertEqual(len(mock_send.call_args[0][0]), 3) + self.assertEqual(len(mock_send.call_args[0][0]), 4) # Number of certs expected # Told to use it, and enabled. Should use config in preference of command line call_command(Command(), '--start-date', '2017-01-01', '--args-from-database')