diff --git a/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py b/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py index e4ee12bf01..078588400b 100644 --- a/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py +++ b/openedx/core/djangoapps/credentials/management/commands/notify_credentials.py @@ -12,40 +12,23 @@ signals.) import logging -import math import shlex import sys -import time from datetime import datetime, timedelta import dateutil.parser -from django.contrib.auth import get_user_model from django.core.management.base import BaseCommand, CommandError from MySQLdb import OperationalError from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from pytz import UTC -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 send_grade_if_interesting # lint-amnesty, pylint: disable=unused-import -from openedx.core.djangoapps.programs.signals import handle_course_cert_changed, handle_course_cert_awarded -from openedx.core.djangoapps.site_configuration.models import SiteConfiguration +from openedx.core.djangoapps.credentials.tasks.v1.tasks import handle_notify_credentials -User = get_user_model() log = logging.getLogger(__name__) -def certstr(cert): - return f'{cert.course_id} for user {cert.user.username}' - - -def gradestr(grade): - return f'{grade.course_id} for user {grade.user_id}' - - def parsetime(timestr): dt = dateutil.parser.parse(timestr) if dt.tzinfo is None: @@ -53,41 +36,6 @@ def parsetime(timestr): return dt -def paged_query(queryset, delay, page_size): - """ - A generator that iterates through a queryset but only resolves chunks of it at once, to avoid overwhelming memory - with a giant query. Also adds an optional delay between yields, to help with load. - """ - count = queryset.count() - pages = int(math.ceil(count / page_size)) - - for page in range(pages): - page_start = page * page_size - page_end = page_start + page_size - subquery = queryset[page_start:page_end] - - if delay and page: - time.sleep(delay) - - index = 0 - try: - for item in subquery.iterator(): - index += 1 - yield page_start + index, item - except OperationalError: - # When running the notify_credentials command locally there is an - # OperationalError thrown by MySQL when there are no more results - # available for the queryset iterator. This change catches that exception, - # checks state, and then logs according to that state. This code runs in - # production without issue. This changes allows for the code to be run - # locally without a separate code path. - if index == count: - log.info('OperationalError Exception caught, all known results processed in paged_query') - else: - log.warning('OperationalError Exception caught, it is possible some results were missed') - continue - - class Command(BaseCommand): """ Example usage: @@ -104,9 +52,9 @@ class Command(BaseCommand): DRY-RUN: This command would have handled changes for... 3 Certificates: - course-v1:edX+RecordsSelfPaced+1 for user records_one_cert - course-v1:edX+RecordsSelfPaced+1 for user records - course-v1:edX+RecordsSelfPaced+1 for user records_unverified + course-v1:edX+RecordsSelfPaced+1 for user 14 + course-v1:edX+RecordsSelfPaced+1 for user 17 + course-v1:edX+RecordsSelfPaced+1 for user 18 3 Grades: course-v1:edX+RecordsSelfPaced+1 for user 14 course-v1:edX+RecordsSelfPaced+1 for user 17 @@ -216,110 +164,11 @@ class Command(BaseCommand): 'auto' if options['auto'] else 'manual', ) - try: - site_config = SiteConfiguration.objects.get(site__domain=options['site']) if options['site'] else None - except SiteConfiguration.DoesNotExist: - log.error('No site configuration found for site %s', options['site']) - course_keys = self.get_course_keys(options['courses']) if not (course_keys or options['start_date'] or options['end_date'] or options['user_ids']): raise CommandError('You must specify a filter (e.g. --courses= or --start-date or --user_ids)') - certs = get_recently_modified_certificates( - course_keys, options['start_date'], options['end_date'], options['user_ids'] - ) - - users = None - if options['user_ids']: - users = User.objects.filter(id__in=options['user_ids']) - grades = get_recently_modified_grades( - course_keys, options['start_date'], options['end_date'], users - ) - - 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'], - 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, notify_programs=False - ): - """ Run actual handler commands for the provided certs and grades. """ - - course_cert_info = {} - - # First, do certs - for i, cert in paged_query(certs, delay, page_size): - if site_config and not site_config.has_org(cert.course_id.org): - log.info("Skipping credential changes %d for certificate %s", i, certstr(cert)) - continue - - log.info( - "Handling credential changes %d for certificate %s", - i, certstr(cert), - ) - - signal_args = { - 'sender': None, - 'user': cert.user, - 'course_key': cert.course_id, - 'mode': cert.mode, - 'status': cert.status, - 'verbose': verbose, - } - - data = { - 'mode': cert.mode, - 'status': cert.status - } - - 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): - if site_config and not site_config.has_org(grade.course_id.org): - log.info("Skipping grade changes %d for grade %s", i, gradestr(grade)) - continue - - log.info( - "Handling grade changes %d for grade %s", - i, gradestr(grade), - ) - - user = User.objects.get(id=grade.user_id) - - # Grab mode/status from cert call - key = (user.id, str(grade.course_id)) - cert_info = course_cert_info.get(key, {}) - mode = cert_info.get('mode', None) - status = cert_info.get('status', None) - - send_grade_if_interesting( - user, - grade.course_id, - mode, - status, - grade.letter_grade, - grade.percent_grade, - verbose=verbose - ) + handle_notify_credentials.delay(options, course_keys) def get_course_keys(self, courses=None): """ @@ -339,26 +188,11 @@ class Command(BaseCommand): log.info("%d courses specified: %s", len(courses), ", ".join(courses)) for course_id in courses: try: - course_keys.append(CourseKey.from_string(course_id)) + # Use CourseKey to check if the course_id is parsable, but just + # keep the string; the celery task needs JSON serializable data. + course_keys.append(str(CourseKey.from_string(course_id))) except InvalidKeyError: log.fatal("%s is not a parseable CourseKey", course_id) sys.exit(1) return course_keys - - def print_dry_run(self, certs, grades): - """Give a preview of what certs/grades we will handle.""" - print("DRY-RUN: This command would have handled changes for...") - ITEMS_TO_SHOW = 10 - - print(certs.count(), "Certificates:") - for cert in certs[:ITEMS_TO_SHOW]: - print(" ", certstr(cert)) - if certs.count() > ITEMS_TO_SHOW: - print(" (+ {} more)".format(certs.count() - ITEMS_TO_SHOW)) - - print(grades.count(), "Grades:") - for grade in grades[:ITEMS_TO_SHOW]: - print(" ", gradestr(grade)) - if grades.count() > ITEMS_TO_SHOW: - print(" (+ {} more)".format(grades.count() - ITEMS_TO_SHOW)) 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 c6d7e58a1a..1abc35a661 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 @@ -8,13 +8,9 @@ from unittest import mock from django.core.management import call_command from django.core.management.base import CommandError -from django.db import connection, reset_queries 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, 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 from openedx.core.djangolib.testing.utils import skip_unless_lms @@ -22,7 +18,7 @@ from common.djangoapps.student.tests.factories import UserFactory from ..notify_credentials import Command -COMMAND_MODULE = 'openedx.core.djangoapps.credentials.management.commands.notify_credentials' +NOTIFY_CREDENTIALS_TASK = 'openedx.core.djangoapps.credentials.tasks.v1.tasks.handle_notify_credentials.run' @skip_unless_lms @@ -34,203 +30,181 @@ class TestNotifyCredentials(TestCase): super().setUp() self.user = UserFactory.create() self.user2 = UserFactory.create() + self.expected_options = { + 'args_from_database': False, + 'auto': False, + 'courses': None, + 'delay': 0, + 'dry_run': False, + 'end_date': None, + 'force_color': False, + 'no_color': False, + 'notify_programs': False, + 'page_size': 100, + 'pythonpath': None, + 'settings': None, + 'site': None, + 'start_date': None, + 'traceback': False, + 'user_ids': None, + 'verbose': False, + 'verbosity': 1, + 'skip_checks': True, + } - 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, 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.user2, course_id='course-v1:edX+Test+4', status=CertificateStatuses.downloadable - ) - print(('self.cert1.modified_date', self.cert1.modified_date)) + @mock.patch(NOTIFY_CREDENTIALS_TASK) + def test_course_args(self, mock_task): + course_1_id = 'course-v1:edX+Test+1' + course_2_id = 'course-v1:edX+Test+2' + self.expected_options['courses'] = [course_1_id, course_2_id] - # No factory for these - with freeze_time(datetime(2017, 1, 1)): - self.grade1 = PersistentCourseGrade.objects.create(user_id=self.user.id, course_id='course-v1:edX+Test+1', - percent_grade=1) - with freeze_time(datetime(2017, 2, 1)): - self.grade2 = PersistentCourseGrade.objects.create(user_id=self.user.id, course_id='course-v1:edX+Test+2', - percent_grade=1) - 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.user2.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') - def test_course_args(self, mock_send): - call_command(Command(), '--course', 'course-v1:edX+Test+1', 'course-v1:edX+Test+2') - assert mock_send.called - assert list(mock_send.call_args[0][0]) == [self.cert1, self.cert2] - assert list(mock_send.call_args[0][1]) == [self.grade1, self.grade2] + call_command(Command(), '--course', course_1_id, course_2_id) + assert mock_task.called + assert mock_task.call_args[0][0] == self.expected_options @freeze_time(datetime(2017, 5, 1, 4)) - @mock.patch(COMMAND_MODULE + '.Command.send_notifications') - def test_auto_execution(self, mock_send): - cert_filter_args = {} - - with freeze_time(datetime(2017, 5, 1, 0)): - cert1 = GeneratedCertificateFactory(user=self.user, course_id='course-v1:edX+Test+11') - with freeze_time(datetime(2017, 5, 1, 3)): - cert2 = GeneratedCertificateFactory(user=self.user, course_id='course-v1:edX+Test+22') - - 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) - with freeze_time(datetime(2017, 5, 1, 3)): - grade2 = PersistentCourseGrade.objects.create(user_id=self.user.id, course_id='course-v1:edX+Test+22', - percent_grade=1) - - total_certificates = GeneratedCertificate.objects.filter(**cert_filter_args).order_by('modified_date') # pylint: disable=no-member - total_grades = PersistentCourseGrade.objects.all() + @mock.patch(NOTIFY_CREDENTIALS_TASK) + def test_auto_execution(self, mock_task): + self.expected_options['auto'] = True + self.expected_options['start_date'] = '2017-05-01T00:00:00' + self.expected_options['end_date'] = '2017-05-01T04:00:00' call_command(Command(), '--auto') + assert mock_task.called + assert mock_task.call_args[0][0] == self.expected_options - assert mock_send.called - self.assertListEqual(list(mock_send.call_args[0][0]), [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(COMMAND_MODULE + '.Command.send_notifications') - def test_date_args(self, mock_send): + @mock.patch(NOTIFY_CREDENTIALS_TASK) + def test_date_args(self, mock_task): + self.expected_options['start_date'] = '2017-01-31T00:00:00Z' call_command(Command(), '--start-date', '2017-01-31') - assert mock_send.called - 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() + assert mock_task.called + assert mock_task.call_args[0][0] == self.expected_options + mock_task.reset_mock() + self.expected_options['start_date'] = '2017-02-01T00:00:00Z' + self.expected_options['end_date'] = '2017-02-02T00:00:00Z' call_command(Command(), '--start-date', '2017-02-01', '--end-date', '2017-02-02') - assert mock_send.called - 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() + assert mock_task.called + assert mock_task.call_args[0][0] == self.expected_options + mock_task.reset_mock() + self.expected_options['start_date'] = None + self.expected_options['end_date'] = '2017-02-02T00:00:00Z' call_command(Command(), '--end-date', '2017-02-02') - assert mock_send.called - 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() + assert mock_task.called + assert mock_task.call_args[0][0] == self.expected_options + mock_task.reset_mock() + self.expected_options['start_date'] = '2017-02-01T00:00:00Z' + self.expected_options['end_date'] = '2017-02-01T04:00:00Z' call_command(Command(), '--start-date', "2017-02-01 00:00:00", '--end-date', '2017-02-01 04:00:00') - assert 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]) + assert mock_task.called + assert mock_task.call_args[0][0] == self.expected_options - @mock.patch(COMMAND_MODULE + '.Command.send_notifications') - def test_username_arg(self, mock_send): + @mock.patch(NOTIFY_CREDENTIALS_TASK) + def test_username_arg(self, mock_task): + self.expected_options['start_date'] = '2017-02-01T00:00:00Z' + self.expected_options['end_date'] = '2017-02-02T00:00:00Z' + self.expected_options['user_ids'] = [str(self.user2.id)] call_command( - Command(), '--start-date', '2017-02-01', '--end-date', '2017-02-02', '--user_ids', self.user2.id + 'notify_credentials', '--start-date', '2017-02-01', '--end-date', '2017-02-02', '--user_ids', self.user2.id ) - assert mock_send.called - self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert4]) - self.assertListEqual(list(mock_send.call_args[0][1]), [self.grade4]) - mock_send.reset_mock() + assert mock_task.called + assert mock_task.call_args[0][0] == self.expected_options + mock_task.reset_mock() + self.expected_options['start_date'] = None + self.expected_options['end_date'] = None + self.expected_options['user_ids'] = [str(self.user2.id)] call_command( - Command(), '--user_ids', self.user2.id + 'notify_credentials', '--user_ids', self.user2.id ) - assert mock_send.called - self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert4]) - self.assertListEqual(list(mock_send.call_args[0][1]), [self.grade4]) - mock_send.reset_mock() + assert mock_task.called + assert mock_task.call_args[0][0] == self.expected_options + mock_task.reset_mock() + self.expected_options['start_date'] = '2017-02-01T00:00:00Z' + self.expected_options['end_date'] = '2017-02-02T00:00:00Z' + self.expected_options['user_ids'] = [str(self.user.id)] call_command( - Command(), '--start-date', '2017-02-01', '--end-date', '2017-02-02', '--user_ids', self.user.id + 'notify_credentials', '--start-date', '2017-02-01', '--end-date', '2017-02-02', '--user_ids', self.user.id ) - assert 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_send.reset_mock() + assert mock_task.called + assert mock_task.call_args[0][0] == self.expected_options + mock_task.reset_mock() + self.expected_options['start_date'] = None + self.expected_options['end_date'] = None + self.expected_options['user_ids'] = [str(self.user.id)] call_command( - Command(), '--user_ids', self.user.id + 'notify_credentials', '--user_ids', self.user.id ) - assert mock_send.called - self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert1, self.cert2, self.cert3]) - self.assertListEqual(list(mock_send.call_args[0][1]), [self.grade1, self.grade2, self.grade3]) - mock_send.reset_mock() + assert mock_task.called + assert mock_task.call_args[0][0] == self.expected_options + mock_task.reset_mock() + self.expected_options['start_date'] = None + self.expected_options['end_date'] = None + self.expected_options['user_ids'] = [str(self.user.id), str(self.user2.id)] call_command( - Command(), '--user_ids', self.user.id, self.user2.id + 'notify_credentials', '--user_ids', self.user.id, self.user2.id ) - assert mock_send.called - self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert1, self.cert2, self.cert4, self.cert3]) - self.assertListEqual(list(mock_send.call_args[0][1]), [self.grade1, self.grade2, self.grade4, self.grade3]) - mock_send.reset_mock() + assert mock_task.called + assert mock_task.call_args[0][0] == self.expected_options + mock_task.reset_mock() - @mock.patch(COMMAND_MODULE + '.Command.send_notifications') - def test_no_args(self, mock_send): + @mock.patch(NOTIFY_CREDENTIALS_TASK) + def test_no_args(self, mock_task): with self.assertRaisesRegex(CommandError, 'You must specify a filter.*'): call_command(Command()) - assert not mock_send.called + assert not mock_task.called - @mock.patch(COMMAND_MODULE + '.Command.send_notifications') - def test_dry_run(self, mock_send): + @mock.patch(NOTIFY_CREDENTIALS_TASK) + def test_dry_run(self, mock_task): + self.expected_options['start_date'] = '2017-02-01T00:00:00Z' + self.expected_options['dry_run'] = True call_command(Command(), '--dry-run', '--start-date', '2017-02-01') - assert not 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, mock_program_awarded): - call_command(Command(), '--start-date', '2017-02-01') - assert mock_grade_interesting.call_count == 3 - assert mock_program_changed.call_count == 3 - assert mock_program_awarded.call_count == 0 - mock_grade_interesting.reset_mock() - mock_program_changed.reset_mock() - mock_program_awarded.reset_mock() + assert mock_task.called + assert mock_task.call_args[0][0] == self.expected_options + @mock.patch(NOTIFY_CREDENTIALS_TASK) + def test_hand_off(self, mock_task): + self.expected_options['start_date'] = '2017-02-01T00:00:00Z' + self.expected_options['notify_programs'] = True call_command(Command(), '--start-date', '2017-02-01', '--notify_programs') - assert mock_grade_interesting.call_count == 3 - assert mock_program_changed.call_count == 3 - assert mock_program_awarded.call_count == 1 + assert mock_task.called + assert mock_task.call_args[0][0] == self.expected_options - @mock.patch(COMMAND_MODULE + '.time') - def test_delay(self, mock_time): - call_command(Command(), '--start-date', '2017-01-01', '--page-size=2') - assert mock_time.sleep.call_count == 0 - mock_time.sleep.reset_mock() + @mock.patch(NOTIFY_CREDENTIALS_TASK) + def test_delay(self, mock_task): + self.expected_options['start_date'] = '2017-02-01T00:00:00Z' + self.expected_options['delay'] = 0.2 + call_command(Command(), '--start-date', '2017-02-01', '--delay', '0.2') + assert mock_task.called + assert mock_task.call_args[0][0] == self.expected_options - call_command(Command(), '--start-date', '2017-01-01', '--page-size=2', '--delay', '0.2') - assert mock_time.sleep.call_count == 2 - # Between each page, twice (2 pages, for certs and grades) - assert mock_time.sleep.call_args[0][0] == 0.2 + @mock.patch(NOTIFY_CREDENTIALS_TASK) + def test_page_size(self, mock_task): + self.expected_options['start_date'] = '2017-02-01T00:00:00Z' + self.expected_options['page_size'] = 2 + call_command(Command(), '--start-date', '2017-02-01', '--page-size=2') + assert mock_task.called + assert mock_task.call_args[0][0] == self.expected_options - @override_settings(DEBUG=True) - def test_page_size(self): - reset_queries() - call_command(Command(), '--start-date', '2017-01-01') - baseline = len(connection.queries) - - reset_queries() - call_command(Command(), '--start-date', '2017-01-01', '--page-size=1') - assert 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') - assert len(connection.queries) == (baseline + 2) - # one extra page query each for certs & grades - - @mock.patch(COMMAND_MODULE + '.send_grade_if_interesting') - def test_site(self, mock_grade_interesting): + @mock.patch(NOTIFY_CREDENTIALS_TASK) + def test_site(self, mock_task): site_config = SiteConfigurationFactory.create( site_values={'course_org_filter': ['testX']} ) + self.expected_options['site'] = site_config.site.domain + self.expected_options['start_date'] = '2017-01-01T00:00:00Z' call_command(Command(), '--site', site_config.site.domain, '--start-date', '2017-01-01') - assert mock_grade_interesting.call_count == 1 + assert mock_task.called + assert mock_task.call_args[0][0] == self.expected_options - @mock.patch(COMMAND_MODULE + '.Command.send_notifications') - def test_args_from_database(self, mock_send): + @mock.patch(NOTIFY_CREDENTIALS_TASK) + def test_args_from_database(self, mock_task): # Nothing in the database, should default to disabled with self.assertRaisesRegex(CommandError, 'NotifyCredentialsConfig is disabled.*'): call_command(Command(), '--start-date', '2017-01-01', '--args-from-database') @@ -242,13 +216,17 @@ class TestNotifyCredentials(TestCase): config.save() # Not told to use config, should ignore it + self.expected_options['start_date'] = '2017-01-01T00:00:00Z' call_command(Command(), '--start-date', '2017-01-01') - assert len(mock_send.call_args[0][0]) == 4 - # Number of certs expected + assert mock_task.called + assert mock_task.call_args[0][0] == self.expected_options # Told to use it, and enabled. Should use config in preference of command line + self.expected_options['start_date'] = '2017-03-01T00:00:00Z' + del self.expected_options['skip_checks'] call_command(Command(), '--start-date', '2017-01-01', '--args-from-database') - assert len(mock_send.call_args[0][0]) == 1 + assert mock_task.called + assert mock_task.call_args[0][0] == self.expected_options config.enabled = False config.save() diff --git a/openedx/core/djangoapps/credentials/signals.py b/openedx/core/djangoapps/credentials/signals.py index b390fab95d..2fdc8d9c6d 100644 --- a/openedx/core/djangoapps/credentials/signals.py +++ b/openedx/core/djangoapps/credentials/signals.py @@ -3,141 +3,7 @@ This file contains signal handlers for credentials-related functionality. """ -from logging import getLogger - -from django.contrib.sites.models import Site - -from common.djangoapps.course_modes.models import CourseMode -from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate -from lms.djangoapps.grades.api import CourseGradeFactory -from openedx.core.djangoapps.catalog.utils import get_programs -from openedx.core.djangoapps.credentials.models import CredentialsApiConfig - -from .helpers import is_learner_records_enabled_for_org # lint-amnesty, pylint: disable=unused-import -from .tasks.v1.tasks import send_grade_to_credentials - -log = getLogger(__name__) - - -# "interesting" here means "credentials will want to know about it" -INTERESTING_MODES = CourseMode.CERTIFICATE_RELEVANT_MODES -INTERESTING_STATUSES = [ - CertificateStatuses.notpassing, - CertificateStatuses.downloadable, -] - - -# These handlers have Credentials business logic that has bled into the LMS. But we want to filter here in order to -# not flood our task queue with a bunch of signals. So we put up with it. - -def is_course_run_in_a_program(course_run_key): - """ Returns true if the given course key is in any program at all. """ - - # We don't have an easy way to go from course_run_key to a specific site that owns it. So just search each site. - sites = Site.objects.all() - str_key = str(course_run_key) - for site in sites: - for program in get_programs(site): - for course in program['courses']: - for course_run in course['course_runs']: - if str_key == course_run['key']: - return True - return False - - -def send_grade_if_interesting(user, course_run_key, mode, status, letter_grade, percent_grade, verbose=False): - """ Checks if grade is interesting to Credentials and schedules a Celery task if so. """ - - if verbose: - msg = "Starting send_grade_if_interesting with params: "\ - "user [{username}], "\ - "course_run_key [{key}], "\ - "mode [{mode}], "\ - "status [{status}], "\ - "letter_grade [{letter_grade}], "\ - "percent_grade [{percent_grade}], "\ - "verbose [{verbose}]"\ - .format( - username=getattr(user, 'username', None), - key=str(course_run_key), - mode=mode, - status=status, - letter_grade=letter_grade, - percent_grade=percent_grade, - verbose=verbose - ) - log.info(msg) - # Avoid scheduling new tasks if certification is disabled. (Grades are a part of the records/cert story) - if not CredentialsApiConfig.current().is_learner_issuance_enabled: - if verbose: - log.info("Skipping send grade: is_learner_issuance_enabled False") - return - - # Avoid scheduling new tasks if learner records are disabled for this site. - if not is_learner_records_enabled_for_org(course_run_key.org): - if verbose: - log.info( - "Skipping send grade: ENABLE_LEARNER_RECORDS False for org [{org}]".format( - org=course_run_key.org - ) - ) - return - - # Grab mode/status if we don't have them in hand - if mode is None or status is None: - try: - cert = GeneratedCertificate.objects.get(user=user, course_id=course_run_key) # pylint: disable=no-member - mode = cert.mode - status = cert.status - except GeneratedCertificate.DoesNotExist: - # We only care about grades for which there is a certificate. - if verbose: - log.info( - "Skipping send grade: no cert for user [{username}] & course_id [{course_id}]".format( - username=getattr(user, 'username', None), - course_id=str(course_run_key) - ) - ) - return - - # Don't worry about whether it's available as well as awarded. Just awarded is good enough to record a verified - # attempt at a course. We want even the grades that didn't pass the class because Credentials wants to know about - # those too. - if mode not in INTERESTING_MODES or status not in INTERESTING_STATUSES: - if verbose: - log.info( - "Skipping send grade: mode/status uninteresting for mode [{mode}] & status [{status}]".format( - mode=mode, - status=status - ) - ) - return - - # If the course isn't in any program, don't bother telling Credentials about it. When Credentials grows support - # for course records as well as program records, we'll need to open this up. - if not is_course_run_in_a_program(course_run_key): - if verbose: - log.info( - "Skipping send grade: course run not in a program. [{course_id}]".format(course_id=str(course_run_key)) - ) - return - - # Grab grades if we don't have them in hand - if letter_grade is None or percent_grade is None: - grade = CourseGradeFactory().read(user, course_key=course_run_key, create_if_needed=False) - if grade is None: - if verbose: - log.info( - "Skipping send grade: No grade found for user [{username}] & course_id [{course_id}]".format( - username=getattr(user, 'username', None), - course_id=str(course_run_key) - ) - ) - return - letter_grade = grade.letter_grade - percent_grade = grade.percent - - send_grade_to_credentials.delay(user.username, str(course_run_key), True, letter_grade, percent_grade) +from .tasks.v1.tasks import send_grade_if_interesting def handle_grade_change(user, course_grade, course_key, **kwargs): diff --git a/openedx/core/djangoapps/credentials/tasks/v1/tasks.py b/openedx/core/djangoapps/credentials/tasks/v1/tasks.py index 6b35f6043c..b9885a85ca 100644 --- a/openedx/core/djangoapps/credentials/tasks/v1/tasks.py +++ b/openedx/core/djangoapps/credentials/tasks/v1/tasks.py @@ -2,19 +2,40 @@ This file contains celery tasks for credentials-related functionality. """ +import math +import time from celery import shared_task from celery.exceptions import MaxRetriesExceededError from celery.utils.log import get_task_logger +from celery_utils.logged_task import LoggedTask from django.conf import settings from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user +from django.contrib.sites.models import Site from edx_django_utils.monitoring import set_code_owner_attribute from opaque_keys.edx.keys import CourseKey +from MySQLdb import OperationalError +from common.djangoapps.course_modes.models import CourseMode +from lms.djangoapps.certificates.api import get_recently_modified_certificates +from lms.djangoapps.grades.api import CourseGradeFactory, get_recently_modified_grades +from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate +from openedx.core.djangoapps.catalog.utils import get_programs +from openedx.core.djangoapps.credentials.helpers import is_learner_records_enabled_for_org +from openedx.core.djangoapps.credentials.models import CredentialsApiConfig from openedx.core.djangoapps.credentials.utils import get_credentials_api_client +from openedx.core.djangoapps.programs.signals import handle_course_cert_changed, handle_course_cert_awarded +from openedx.core.djangoapps.site_configuration.models import SiteConfiguration logger = get_task_logger(__name__) +# "interesting" here means "credentials will want to know about it" +INTERESTING_MODES = CourseMode.CERTIFICATE_RELEVANT_MODES +INTERESTING_STATUSES = [ + CertificateStatuses.notpassing, + CertificateStatuses.downloadable, +] + # Maximum number of retries before giving up. # For reference, 11 retries with exponential backoff yields a maximum waiting # time of 2047 seconds (about 30 minutes). Setting this to None could yield @@ -54,3 +75,288 @@ def send_grade_to_credentials(self, username, course_run_key, verified, letter_g f"Failed to send grade to credentials. Reason: {error_msg}" ) raise self.retry(exc=exception, countdown=countdown, max_retries=MAX_RETRIES) + + +@shared_task(base=LoggedTask, ignore_result=True) +@set_code_owner_attribute +def handle_notify_credentials(options, course_keys): + """ + Celery task to handle the notify_credentials management command. Finds the + relevant cert and grade records, then starts other celery tasks to send the + data. + """ + + try: + site_config = SiteConfiguration.objects.get(site__domain=options['site']) if options['site'] else None + except SiteConfiguration.DoesNotExist: + logger.exception('No site configuration found for site %s', options['site']) + return + + certs = get_recently_modified_certificates( + course_keys, options['start_date'], options['end_date'], options['user_ids'] + ) + + users = None + if options['user_ids']: + users = User.objects.filter(id__in=options['user_ids']) + + grades = get_recently_modified_grades( + course_keys, options['start_date'], options['end_date'], users + ) + + logger.info('notify_credentials Sending notifications for {certs} certificates and {grades} grades'.format( + certs=certs.count(), + grades=grades.count() + )) + + if options['dry_run']: + log_dry_run(certs, grades) + else: + send_notifications( + certs, + grades, + site_config=site_config, + delay=options['delay'], + page_size=options['page_size'], + verbose=options['verbose'], + notify_programs=options['notify_programs'] + ) + + logger.info('notify_credentials finished') + + +def send_notifications( + certs, grades, site_config=None, delay=0, page_size=100, verbose=False, notify_programs=False +): + """ Run actual handler commands for the provided certs and grades. """ + course_cert_info = {} + # First, do certs + for i, cert in paged_query(certs, delay, page_size): + if site_config and not site_config.has_org(cert.course_id.org): + logger.info("Skipping credential changes %d for certificate %s", i, certstr(cert)) + continue + + logger.info( + "Handling credential changes %d for certificate %s", + i, certstr(cert), + ) + + signal_args = { + 'sender': None, + 'user': cert.user, + 'course_key': cert.course_id, + 'mode': cert.mode, + 'status': cert.status, + 'verbose': verbose, + } + + data = { + 'mode': cert.mode, + 'status': cert.status + } + + 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): + if site_config and not site_config.has_org(grade.course_id.org): + logger.info("Skipping grade changes %d for grade %s", i, gradestr(grade)) + continue + + logger.info( + "Handling grade changes %d for grade %s", + i, gradestr(grade), + ) + + user = User.objects.get(id=grade.user_id) + + # Grab mode/status from cert call + key = (user.id, str(grade.course_id)) + cert_info = course_cert_info.get(key, {}) + mode = cert_info.get('mode', None) + status = cert_info.get('status', None) + + send_grade_if_interesting( + user, + grade.course_id, + mode, + status, + grade.letter_grade, + grade.percent_grade, + verbose=verbose + ) + + +def paged_query(queryset, delay, page_size): + """ + A generator that iterates through a queryset but only resolves chunks of it at once, to avoid overwhelming memory + with a giant query. Also adds an optional delay between yields, to help with load. + """ + count = queryset.count() + pages = int(math.ceil(count / page_size)) + + for page in range(pages): + page_start = page * page_size + page_end = page_start + page_size + subquery = queryset[page_start:page_end] + + if delay and page: + time.sleep(delay) + index = 0 + try: + for item in subquery.iterator(): + index += 1 + yield page_start + index, item + except OperationalError: + # When running the notify_credentials command locally there is an + # OperationalError thrown by MySQL when there are no more results + # available for the queryset iterator. This change catches that exception, + # checks state, and then logs according to that state. This code runs in + # production without issue. This changes allows for the code to be run + # locally without a separate code path. + if index == count: + logger.info('OperationalError Exception caught, all known results processed in paged_query') + else: + logger.warning('OperationalError Exception caught, it is possible some results were missed') + continue + + +def log_dry_run(certs, grades): + """Give a preview of what certs/grades we will handle.""" + logger.info("DRY-RUN: This task would have handled changes for...") + ITEMS_TO_SHOW = 10 + + logger.info(f"{certs.count()} Certificates:") + for cert in certs[:ITEMS_TO_SHOW]: + logger.info(f" {certstr(cert)}") + if certs.count() > ITEMS_TO_SHOW: + logger.info(f" (+ {certs.count() - ITEMS_TO_SHOW} more)") + + logger.info(f"{grades.count()} Grades:") + for grade in grades[:ITEMS_TO_SHOW]: + logger.info(f" {gradestr(grade)}") + if grades.count() > ITEMS_TO_SHOW: + logger.info(f" (+ {grades.count() - ITEMS_TO_SHOW} more)") + + +def certstr(cert): + return f'{cert.course_id} for user {cert.user.id}' + + +def gradestr(grade): + return f'{grade.course_id} for user {grade.user_id}' + + +# This has Credentials business logic that has bled into the LMS. But we want to filter here in order to +# not flood our task queue with a bunch of signals. So we put up with it. +def send_grade_if_interesting(user, course_run_key, mode, status, letter_grade, percent_grade, verbose=False): + """ Checks if grade is interesting to Credentials and schedules a Celery task if so. """ + + if verbose: + msg = "Starting send_grade_if_interesting with params: "\ + "user [{username}], "\ + "course_run_key [{key}], "\ + "mode [{mode}], "\ + "status [{status}], "\ + "letter_grade [{letter_grade}], "\ + "percent_grade [{percent_grade}], "\ + "verbose [{verbose}]"\ + .format( + username=getattr(user, 'username', None), + key=str(course_run_key), + mode=mode, + status=status, + letter_grade=letter_grade, + percent_grade=percent_grade, + verbose=verbose + ) + logger.info(msg) + # Avoid scheduling new tasks if certification is disabled. (Grades are a part of the records/cert story) + if not CredentialsApiConfig.current().is_learner_issuance_enabled: + if verbose: + logger.info("Skipping send grade: is_learner_issuance_enabled False") + return + + # Avoid scheduling new tasks if learner records are disabled for this site. + if not is_learner_records_enabled_for_org(course_run_key.org): + if verbose: + logger.info( + "Skipping send grade: ENABLE_LEARNER_RECORDS False for org [{org}]".format( + org=course_run_key.org + ) + ) + return + + # Grab mode/status if we don't have them in hand + if mode is None or status is None: + try: + cert = GeneratedCertificate.objects.get(user=user, course_id=course_run_key) # pylint: disable=no-member + mode = cert.mode + status = cert.status + except GeneratedCertificate.DoesNotExist: + # We only care about grades for which there is a certificate. + if verbose: + logger.info( + "Skipping send grade: no cert for user [{username}] & course_id [{course_id}]".format( + username=getattr(user, 'username', None), + course_id=str(course_run_key) + ) + ) + return + + # Don't worry about whether it's available as well as awarded. Just awarded is good enough to record a verified + # attempt at a course. We want even the grades that didn't pass the class because Credentials wants to know about + # those too. + if mode not in INTERESTING_MODES or status not in INTERESTING_STATUSES: + if verbose: + logger.info( + "Skipping send grade: mode/status uninteresting for mode [{mode}] & status [{status}]".format( + mode=mode, + status=status + ) + ) + return + + # If the course isn't in any program, don't bother telling Credentials about it. When Credentials grows support + # for course records as well as program records, we'll need to open this up. + if not is_course_run_in_a_program(course_run_key): + if verbose: + logger.info( + "Skipping send grade: course run not in a program. [{course_id}]".format(course_id=str(course_run_key)) + ) + return + + # Grab grades if we don't have them in hand + if letter_grade is None or percent_grade is None: + grade = CourseGradeFactory().read(user, course_key=course_run_key, create_if_needed=False) + if grade is None: + if verbose: + logger.info( + "Skipping send grade: No grade found for user [{username}] & course_id [{course_id}]".format( + username=getattr(user, 'username', None), + course_id=str(course_run_key) + ) + ) + return + letter_grade = grade.letter_grade + percent_grade = grade.percent + + send_grade_to_credentials.delay(user.username, str(course_run_key), True, letter_grade, percent_grade) + + +def is_course_run_in_a_program(course_run_key): + """ Returns true if the given course key is in any program at all. """ + + # We don't have an easy way to go from course_run_key to a specific site that owns it. So just search each site. + sites = Site.objects.all() + str_key = str(course_run_key) + for site in sites: + for program in get_programs(site): + for course in program['courses']: + for course_run in course['course_runs']: + if str_key == course_run['key']: + return True + return False diff --git a/openedx/core/djangoapps/credentials/tests/test_signals.py b/openedx/core/djangoapps/credentials/tests/test_signals.py index 49e56aa911..c73625803d 100644 --- a/openedx/core/djangoapps/credentials/tests/test_signals.py +++ b/openedx/core/djangoapps/credentials/tests/test_signals.py @@ -3,18 +3,11 @@ from unittest import mock -import ddt from django.conf import settings from django.test import TestCase, override_settings -from opaque_keys.edx.keys import CourseKey from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory -from lms.djangoapps.grades.tests.utils import mock_passing_grade -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.credentials.signals import is_course_run_in_a_program, send_grade_if_interesting -from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory from openedx.core.djangolib.testing.utils import skip_unless_lms from common.djangoapps.student.tests.factories import UserFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -23,142 +16,6 @@ from xmodule.modulestore.tests.factories import CourseFactory as XModuleCourseFa SIGNALS_MODULE = 'openedx.core.djangoapps.credentials.signals' -@ddt.ddt -@skip_unless_lms -@mock.patch( - 'openedx.core.djangoapps.credentials.models.CredentialsApiConfig.is_learner_issuance_enabled', - new_callable=mock.PropertyMock, - return_value=True, -) -@mock.patch(SIGNALS_MODULE + '.send_grade_to_credentials') -@mock.patch(SIGNALS_MODULE + '.is_course_run_in_a_program') -class TestCredentialsSignalsSendGrade(TestCase): - """ Tests for send_grade_if_interesting, the main utility function that sends a grade """ - - def setUp(self): - super().setUp() - self.user = UserFactory() - self.key = CourseKey.from_string(CourseRunFactory()['key']) - - @ddt.data( - [True, 'verified', 'downloadable'], - [True, 'professional', 'downloadable'], - [True, 'no-id-professional', 'downloadable'], - [True, 'credit', 'downloadable'], - [True, 'verified', 'notpassing'], - [True, 'masters', 'downloadable'], - [True, 'masters', 'notpassing'], - [False, 'audit', 'downloadable'], - [False, 'professional', 'generating'], - [False, 'no-id-professional', 'generating'], - ) - @ddt.unpack - def test_send_grade_if_right_cert(self, called, mode, status, mock_is_course_run_in_a_program, - mock_send_grade_to_credentials, _mock_is_learner_issuance_enabled): - mock_is_course_run_in_a_program.return_value = True - - # Test direct send - send_grade_if_interesting(self.user, self.key, mode, status, 'A', 1.0) - assert mock_send_grade_to_credentials.delay.called is called - mock_send_grade_to_credentials.delay.reset_mock() - - # Test query - GeneratedCertificateFactory( - user=self.user, - course_id=self.key, - status=status, - mode=mode - ) - send_grade_if_interesting(self.user, self.key, None, None, 'A', 1.0) - assert mock_send_grade_to_credentials.delay.called is called - - def test_send_grade_missing_cert(self, _, mock_send_grade_to_credentials, _mock_is_learner_issuance_enabled): - send_grade_if_interesting(self.user, self.key, None, None, 'A', 1.0) - assert not mock_send_grade_to_credentials.delay.called - - @ddt.data([True], [False]) - @ddt.unpack - def test_send_grade_if_in_a_program(self, in_program, mock_is_course_run_in_a_program, - mock_send_grade_to_credentials, _mock_is_learner_issuance_enabled): - mock_is_course_run_in_a_program.return_value = in_program - send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', 'A', 1.0) - assert mock_send_grade_to_credentials.delay.called is in_program - - def test_send_grade_queries_grade(self, mock_is_course_run_in_a_program, mock_send_grade_to_credentials, - _mock_is_learner_issuance_enabled): - mock_is_course_run_in_a_program.return_value = True - - with mock_passing_grade('B', 0.81): - send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', None, None) - assert mock_send_grade_to_credentials.delay.called - assert mock_send_grade_to_credentials.delay.call_args[0] == (self.user.username, str(self.key), True, 'B', 0.81) - mock_send_grade_to_credentials.delay.reset_mock() - - @mock.patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) - def test_send_grade_without_grade(self, mock_is_course_run_in_a_program, mock_send_grade_to_credentials, - _mock_is_learner_issuance_enabled): - mock_is_course_run_in_a_program.return_value = True - send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', None, None) - assert not mock_send_grade_to_credentials.delay.called - - def test_send_grade_without_issuance_enabled(self, _mock_is_course_run_in_a_program, - mock_send_grade_to_credentials, mock_is_learner_issuance_enabled): - mock_is_learner_issuance_enabled.return_value = False - send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', None, None) - assert mock_is_learner_issuance_enabled.called - assert not mock_send_grade_to_credentials.delay.called - - def test_send_grade_records_enabled(self, _mock_is_course_run_in_a_program, mock_send_grade_to_credentials, - _mock_is_learner_issuance_enabled): - site_config = SiteConfigurationFactory.create( - site_values={'course_org_filter': [self.key.org]} - ) - - # Correctly sent - send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', None, None) - assert mock_send_grade_to_credentials.delay.called - mock_send_grade_to_credentials.delay.reset_mock() - - # Correctly not sent - site_config.site_values['ENABLE_LEARNER_RECORDS'] = False - site_config.save() - send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', None, None) - assert not mock_send_grade_to_credentials.delay.called - - def test_send_grade_records_disabled_globally( - self, _mock_is_course_run_in_a_program, mock_send_grade_to_credentials, - _mock_is_learner_issuance_enabled - ): - assert is_learner_records_enabled() - with override_settings(FEATURES={"ENABLE_LEARNER_RECORDS": False}): - assert not is_learner_records_enabled() - send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', None, None) - assert not mock_send_grade_to_credentials.delay.called - - -@skip_unless_lms -@mock.patch(SIGNALS_MODULE + '.get_programs') -class TestCredentialsSignalsUtils(TestCase): - """ Tests helper utility functions in our signal handling. """ - - def setUp(self): - super().setUp() - self.site = SiteFactory() - self.course_run = CourseRunFactory() - course = CourseFactory(course_runs=[self.course_run]) - self.data = [ProgramFactory(courses=[course])] - - def test_is_course_run_in_a_program_success(self, mock_get_programs): - mock_get_programs.return_value = self.data - assert is_course_run_in_a_program(self.course_run['key']) - assert mock_get_programs.call_args[0] == (self.site,) - - def test_is_course_run_in_a_program_failure(self, mock_get_programs): - mock_get_programs.return_value = self.data - course_run2 = CourseRunFactory() - assert not is_course_run_in_a_program(course_run2['key']) - - @skip_unless_lms @mock.patch(SIGNALS_MODULE + '.send_grade_if_interesting') class TestCredentialsSignalsEmissions(ModuleStoreTestCase): diff --git a/openedx/core/djangoapps/credentials/tests/test_tasks.py b/openedx/core/djangoapps/credentials/tests/test_tasks.py index 8585f2069a..6a2578dca9 100644 --- a/openedx/core/djangoapps/credentials/tests/test_tasks.py +++ b/openedx/core/djangoapps/credentials/tests/test_tasks.py @@ -3,13 +3,25 @@ Test credentials tasks """ from unittest import mock +from datetime import datetime +import ddt import pytest from django.conf import settings +from django.db import connection, reset_queries from django.test import TestCase, override_settings +from freezegun import freeze_time +from opaque_keys.edx.keys import CourseKey -from openedx.core.djangolib.testing.utils import skip_unless_lms from common.djangoapps.student.tests.factories import UserFactory +from lms.djangoapps.certificates.models import GeneratedCertificate, CertificateStatuses +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 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 +from openedx.core.djangolib.testing.utils import skip_unless_lms from openedx.core.djangoapps.credentials.tasks.v1 import tasks @@ -63,3 +75,407 @@ class TestSendGradeToCredentialTask(TestCase): pytest.raises(Exception, task.get) assert mock_get_api_client.call_count == (tasks.MAX_RETRIES + 1) + + +@skip_unless_lms +class TestHandleNotifyCredentialsTask(TestCase): + """ + Tests for the 'handle_notify_credentials' task. + """ + def setUp(self): + super().setUp() + self.user = UserFactory.create() + self.user2 = UserFactory.create() + + 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, 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.user2, course_id='course-v1:edX+Test+4', status=CertificateStatuses.downloadable + ) + print(('self.cert1.modified_date', self.cert1.modified_date)) + + # No factory for these + with freeze_time(datetime(2017, 1, 1)): + self.grade1 = PersistentCourseGrade.objects.create(user_id=self.user.id, course_id='course-v1:edX+Test+1', + percent_grade=1) + with freeze_time(datetime(2017, 2, 1)): + self.grade2 = PersistentCourseGrade.objects.create(user_id=self.user.id, course_id='course-v1:edX+Test+2', + percent_grade=1) + 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.user2.id, course_id='course-v1:edX+Test+4', + percent_grade=1) + print(('self.grade1.modified', self.grade1.modified)) + + self.options = { + 'args_from_database': False, + 'auto': False, + 'courses': None, + 'delay': 0, + 'dry_run': False, + 'end_date': None, + 'force_color': False, + 'no_color': False, + 'notify_programs': False, + 'page_size': 100, + 'pythonpath': None, + 'settings': None, + 'site': None, + 'start_date': None, + 'traceback': False, + 'user_ids': None, + 'verbose': False, + 'verbosity': 1, + 'skip_checks': True, + } + + @mock.patch(TASKS_MODULE + '.send_notifications') + def test_course_args(self, mock_send): + course_keys = ['course-v1:edX+Test+1', 'course-v1:edX+Test+2'] + tasks.handle_notify_credentials(options=self.options, course_keys=course_keys) + assert mock_send.called + assert list(mock_send.call_args[0][0]) == [self.cert1, self.cert2] + assert list(mock_send.call_args[0][1]) == [self.grade1, self.grade2] + + @freeze_time(datetime(2017, 5, 1, 4)) + @mock.patch(TASKS_MODULE + '.send_notifications') + def test_auto_execution(self, mock_send): + cert_filter_args = {} + + with freeze_time(datetime(2017, 5, 1, 0)): + cert1 = GeneratedCertificateFactory(user=self.user, course_id='course-v1:edX+Test+11') + with freeze_time(datetime(2017, 5, 1, 3)): + cert2 = GeneratedCertificateFactory(user=self.user, course_id='course-v1:edX+Test+22') + + 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) + with freeze_time(datetime(2017, 5, 1, 3)): + grade2 = PersistentCourseGrade.objects.create(user_id=self.user.id, course_id='course-v1:edX+Test+22', + percent_grade=1) + + total_certificates = GeneratedCertificate.objects.filter(**cert_filter_args).order_by('modified_date') # pylint: disable=no-member + total_grades = PersistentCourseGrade.objects.all() + + self.options['auto'] = True + self.options['start_date'] = '2017-05-01T00:00:00' + self.options['end_date'] = '2017-05-01T04:00:00' + 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][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_date_args(self, mock_send): + self.options['start_date'] = '2017-01-31T00:00:00Z' + tasks.handle_notify_credentials(options=self.options, course_keys=[]) + assert mock_send.called + 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() + + self.options['start_date'] = '2017-02-01T00:00:00Z' + self.options['end_date'] = '2017-02-02T00:00:00Z' + tasks.handle_notify_credentials(options=self.options, course_keys=[]) + assert mock_send.called + 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() + + self.options['start_date'] = None + self.options['end_date'] = '2017-02-02T00:00:00Z' + tasks.handle_notify_credentials(options=self.options, course_keys=[]) + assert mock_send.called + 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() + + self.options['start_date'] = '2017-02-01T00:00:00Z' + self.options['end_date'] = '2017-02-01T04:00:00Z' + tasks.handle_notify_credentials(options=self.options, course_keys=[]) + assert 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(TASKS_MODULE + '.send_notifications') + def test_username_arg(self, mock_send): + self.options['start_date'] = '2017-02-01T00:00:00Z' + self.options['end_date'] = '2017-02-02T00:00:00Z' + self.options['user_ids'] = [str(self.user2.id)] + tasks.handle_notify_credentials(options=self.options, course_keys=[]) + assert mock_send.called + self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert4]) + self.assertListEqual(list(mock_send.call_args[0][1]), [self.grade4]) + mock_send.reset_mock() + + self.options['start_date'] = None + self.options['end_date'] = None + self.options['user_ids'] = [str(self.user2.id)] + tasks.handle_notify_credentials(options=self.options, course_keys=[]) + assert mock_send.called + self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert4]) + self.assertListEqual(list(mock_send.call_args[0][1]), [self.grade4]) + mock_send.reset_mock() + + self.options['start_date'] = '2017-02-01T00:00:00Z' + self.options['end_date'] = '2017-02-02T00:00:00Z' + self.options['user_ids'] = [str(self.user.id)] + tasks.handle_notify_credentials(options=self.options, course_keys=[]) + assert 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_send.reset_mock() + + self.options['start_date'] = None + self.options['end_date'] = None + self.options['user_ids'] = [str(self.user.id)] + tasks.handle_notify_credentials(options=self.options, course_keys=[]) + assert mock_send.called + self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert1, self.cert2, self.cert3]) + self.assertListEqual(list(mock_send.call_args[0][1]), [self.grade1, self.grade2, self.grade3]) + mock_send.reset_mock() + + self.options['start_date'] = None + self.options['end_date'] = None + self.options['user_ids'] = [str(self.user.id), str(self.user2.id)] + tasks.handle_notify_credentials(options=self.options, course_keys=[]) + assert mock_send.called + self.assertListEqual(list(mock_send.call_args[0][0]), [self.cert1, self.cert2, self.cert4, self.cert3]) + self.assertListEqual(list(mock_send.call_args[0][1]), [self.grade1, self.grade2, self.grade4, self.grade3]) + mock_send.reset_mock() + + @mock.patch(TASKS_MODULE + '.send_notifications') + def test_dry_run(self, mock_send): + self.options['start_date'] = '2017-02-01T00:00:00Z' + self.options['dry_run'] = True + tasks.handle_notify_credentials(options=self.options, course_keys=[]) + assert not mock_send.called + + @mock.patch(TASKS_MODULE + '.handle_course_cert_awarded') + @mock.patch(TASKS_MODULE + '.send_grade_if_interesting') + @mock.patch(TASKS_MODULE + '.handle_course_cert_changed') + def test_hand_off(self, mock_grade_interesting, mock_program_changed, mock_program_awarded): + self.options['start_date'] = '2017-02-01T00:00:00Z' + tasks.handle_notify_credentials(options=self.options, course_keys=[]) + assert mock_grade_interesting.call_count == 3 + assert mock_program_changed.call_count == 3 + assert mock_program_awarded.call_count == 0 + mock_grade_interesting.reset_mock() + mock_program_changed.reset_mock() + mock_program_awarded.reset_mock() + + self.options['start_date'] = '2017-02-01T00:00:00Z' + self.options['notify_programs'] = True + tasks.handle_notify_credentials(options=self.options, course_keys=[]) + assert mock_grade_interesting.call_count == 3 + assert mock_program_changed.call_count == 3 + assert mock_program_awarded.call_count == 1 + + @mock.patch(TASKS_MODULE + '.time') + def test_delay(self, mock_time): + self.options['start_date'] = '2017-02-01T00:00:00Z' + self.options['page_size'] = 2 + tasks.handle_notify_credentials(options=self.options, course_keys=[]) + assert mock_time.sleep.call_count == 0 + mock_time.sleep.reset_mock() + + self.options['start_date'] = '2017-02-01T00:00:00Z' + self.options['page_size'] = 2 + self.options['delay'] = 0.2 + tasks.handle_notify_credentials(options=self.options, course_keys=[]) + assert mock_time.sleep.call_count == 2 + # Between each page, twice (2 pages, for certs and grades) + assert mock_time.sleep.call_args[0][0] == 0.2 + + @override_settings(DEBUG=True) + def test_page_size(self): + self.options['start_date'] = '2017-01-01T00:00:00Z' + reset_queries() + tasks.handle_notify_credentials(options=self.options, course_keys=[]) + baseline = len(connection.queries) + + self.options['start_date'] = '2017-01-01T00:00:00Z' + self.options['page_size'] = 1 + reset_queries() + tasks.handle_notify_credentials(options=self.options, course_keys=[]) + assert len(connection.queries) == (baseline + 6) + # two extra page queries each for certs & grades + + self.options['start_date'] = '2017-01-01T00:00:00Z' + self.options['page_size'] = 2 + reset_queries() + tasks.handle_notify_credentials(options=self.options, course_keys=[]) + assert len(connection.queries) == (baseline + 2) + # one extra page query each for certs & grades + + @mock.patch(TASKS_MODULE + '.send_grade_if_interesting') + def test_site(self, mock_grade_interesting): + site_config = SiteConfigurationFactory.create( + site_values={'course_org_filter': ['testX']} + ) + + self.options['start_date'] = '2017-01-01T00:00:00Z' + self.options['site'] = site_config.site.domain + tasks.handle_notify_credentials(options=self.options, course_keys=[]) + assert mock_grade_interesting.call_count == 1 + + @mock.patch(TASKS_MODULE + '.send_notifications') + def test_send_notifications_failure(self, mock_send): + self.options['start_date'] = '2017-01-31T00:00:00Z' + mock_send.side_effect = boom + with pytest.raises(Exception): + tasks.handle_notify_credentials(options=self.options, course_keys=[]) + + @mock.patch(TASKS_MODULE + '.send_grade_if_interesting') + def test_send_grade_failure(self, mock_send_grade): + self.options['start_date'] = '2017-01-31T00:00:00Z' + mock_send_grade.side_effect = boom + with pytest.raises(Exception): + tasks.handle_notify_credentials(options=self.options, course_keys=[]) + + +@ddt.ddt +@skip_unless_lms +@mock.patch( + 'openedx.core.djangoapps.credentials.models.CredentialsApiConfig.is_learner_issuance_enabled', + new_callable=mock.PropertyMock, + return_value=True, +) +@mock.patch(TASKS_MODULE + '.send_grade_to_credentials') +@mock.patch(TASKS_MODULE + '.is_course_run_in_a_program') +class TestSendGradeIfInteresting(TestCase): + """ Tests for send_grade_if_interesting, the main utility function that sends a grade """ + + def setUp(self): + super().setUp() + self.user = UserFactory() + self.key = CourseKey.from_string(CourseRunFactory()['key']) + + @ddt.data( + [True, 'verified', 'downloadable'], + [True, 'professional', 'downloadable'], + [True, 'no-id-professional', 'downloadable'], + [True, 'credit', 'downloadable'], + [True, 'verified', 'notpassing'], + [True, 'masters', 'downloadable'], + [True, 'masters', 'notpassing'], + [False, 'audit', 'downloadable'], + [False, 'professional', 'generating'], + [False, 'no-id-professional', 'generating'], + ) + @ddt.unpack + def test_send_grade_if_right_cert(self, called, mode, status, mock_is_course_run_in_a_program, + mock_send_grade_to_credentials, _mock_is_learner_issuance_enabled): + mock_is_course_run_in_a_program.return_value = True + + # Test direct send + tasks.send_grade_if_interesting(self.user, self.key, mode, status, 'A', 1.0) + assert mock_send_grade_to_credentials.delay.called is called + mock_send_grade_to_credentials.delay.reset_mock() + + # Test query + GeneratedCertificateFactory( + user=self.user, + course_id=self.key, + status=status, + mode=mode + ) + tasks.send_grade_if_interesting(self.user, self.key, None, None, 'A', 1.0) + assert mock_send_grade_to_credentials.delay.called is called + + def test_send_grade_missing_cert(self, _, mock_send_grade_to_credentials, _mock_is_learner_issuance_enabled): + tasks.send_grade_if_interesting(self.user, self.key, None, None, 'A', 1.0) + assert not mock_send_grade_to_credentials.delay.called + + @ddt.data([True], [False]) + @ddt.unpack + def test_send_grade_if_in_a_program(self, in_program, mock_is_course_run_in_a_program, + mock_send_grade_to_credentials, _mock_is_learner_issuance_enabled): + mock_is_course_run_in_a_program.return_value = in_program + tasks.send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', 'A', 1.0) + assert mock_send_grade_to_credentials.delay.called is in_program + + def test_send_grade_queries_grade(self, mock_is_course_run_in_a_program, mock_send_grade_to_credentials, + _mock_is_learner_issuance_enabled): + mock_is_course_run_in_a_program.return_value = True + + with mock_passing_grade('B', 0.81): + tasks.send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', None, None) + assert mock_send_grade_to_credentials.delay.called + assert mock_send_grade_to_credentials.delay.call_args[0] == (self.user.username, str(self.key), True, 'B', 0.81) + mock_send_grade_to_credentials.delay.reset_mock() + + @mock.patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) + def test_send_grade_without_grade(self, mock_is_course_run_in_a_program, mock_send_grade_to_credentials, + _mock_is_learner_issuance_enabled): + mock_is_course_run_in_a_program.return_value = True + tasks.send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', None, None) + assert not mock_send_grade_to_credentials.delay.called + + def test_send_grade_without_issuance_enabled(self, _mock_is_course_run_in_a_program, + mock_send_grade_to_credentials, mock_is_learner_issuance_enabled): + mock_is_learner_issuance_enabled.return_value = False + tasks.send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', None, None) + assert mock_is_learner_issuance_enabled.called + assert not mock_send_grade_to_credentials.delay.called + + def test_send_grade_records_enabled(self, _mock_is_course_run_in_a_program, mock_send_grade_to_credentials, + _mock_is_learner_issuance_enabled): + site_config = SiteConfigurationFactory.create( + site_values={'course_org_filter': [self.key.org]} + ) + + # Correctly sent + tasks.send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', None, None) + assert mock_send_grade_to_credentials.delay.called + mock_send_grade_to_credentials.delay.reset_mock() + + # Correctly not sent + site_config.site_values['ENABLE_LEARNER_RECORDS'] = False + site_config.save() + tasks.send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', None, None) + assert not mock_send_grade_to_credentials.delay.called + + def test_send_grade_records_disabled_globally( + self, _mock_is_course_run_in_a_program, mock_send_grade_to_credentials, + _mock_is_learner_issuance_enabled + ): + assert is_learner_records_enabled() + with override_settings(FEATURES={"ENABLE_LEARNER_RECORDS": False}): + assert not is_learner_records_enabled() + tasks.send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', None, None) + assert not mock_send_grade_to_credentials.delay.called + + +@skip_unless_lms +@mock.patch(TASKS_MODULE + '.get_programs') +class TestIsCourseRunInAProgramUtil(TestCase): + """ Tests helper utility functions in our signal handling. """ + + def setUp(self): + super().setUp() + self.site = SiteFactory() + self.course_run = CourseRunFactory() + course = CourseFactory(course_runs=[self.course_run]) + self.data = [ProgramFactory(courses=[course])] + + def test_is_course_run_in_a_program_success(self, mock_get_programs): + mock_get_programs.return_value = self.data + assert tasks.is_course_run_in_a_program(self.course_run['key']) + assert mock_get_programs.call_args[0] == (self.site,) + + def test_is_course_run_in_a_program_failure(self, mock_get_programs): + mock_get_programs.return_value = self.data + course_run2 = CourseRunFactory() + assert not tasks.is_course_run_in_a_program(course_run2['key'])