Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: add ability for notify_credentials to revoke program certs

Browse Source 
[APER-2504]

This PR adds additional functionality to the tasks kicked off when the `notify_credentials` management command is run.

I have added a new keyword arg (revoke_program_certs) that, if True, will check to see if we need to revoke a program certificate. This functionality was introduced to help automate fixing cases where a learner still has access to a Program Certificate even if they have been awarded all of the course certificates in a Program (see APER-2499).

The functionality has to be committed in two separate PRs because of our blue/green deployments. The task changes will come first, then we will update the management command to be able to set/pass the new settings. New settings were added as keyword args (defaulting to False) in order to ensure that we won't trip up our workers.
This commit is contained in:
Justin Hynes
2023-06-14 15:05:55 +00:00
parent 3fab0aec65
commit d427d404da
2 changed files with 306 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files

186
openedx/core/djangoapps/credentials/tasks/v1/tasks.py
View File

@@ -1,7 +1,6 @@
"""
This file contains celery tasks for credentials-related functionality.
This file contains Celery tasks and utility functions supporting the Credentials IDA.
"""
import math
import time
from urllib.parse import urljoin
@@ -11,7 +10,7 @@ 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.auth import get_user_model
from django.contrib.sites.models import Site
from edx_django_utils.monitoring import set_code_owner_attribute
from MySQLdb import OperationalError
@@ -27,22 +26,25 @@ from openedx.core.djangoapps.content.course_overviews.models import CourseOvervi
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_base_url, get_credentials_api_client
from openedx.core.djangoapps.programs.signals import handle_course_cert_awarded, handle_course_cert_changed
from openedx.core.djangoapps.programs.signals import (
handle_course_cert_awarded,
handle_course_cert_changed,
handle_course_cert_revoked,
)
from openedx.core.djangoapps.site_configuration.models import SiteConfiguration
User = get_user_model()
logger = get_task_logger(__name__)
# "interesting" here means "credentials will want to know about it"
# "Interesting" means "The Credentials IDA 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
# unwanted behavior: infinite retries.
# 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 unwanted behavior: infinite retries.
MAX_RETRIES = 11
@@ -58,7 +60,15 @@ def send_grade_to_credentials(
grade_last_updated
):
"""
Celery task to notify the Credentials IDA of a grade change via POST.
Celery task to notify the Credentials IDA of an "interesting" grade change via an API call.
Args:
username (string): The username of the learner we are currently processing
course_run_key (string): String identifier of the course run associated with the grade update
verified (bool): Boolean determining if the course run is in a "verified" track
letter_grade (string): String identifier describing the "letter" grade the learner has earned
percent_grade (float): Number representing the learner's grade in this course run
grade_last_updated (string): String describing the last time this grade was modified in the LMS
"""
logger.info(f"Running task send_grade_to_credentials for username {username} and course {course_run_key}")
@@ -82,16 +92,12 @@ def send_grade_to_credentials(
}
)
response.raise_for_status()
logger.info(f"Sent grade for course {course_run_key} to user {username}")
logger.info(f"Sent grade for course {course_run_key} for user {username}")
except Exception: # lint-amnesty, pylint: disable=W0703
grade_str = f'(percent: {percent_grade} letter: {letter_grade})'
error_msg = f'Failed to send grade{grade_str} for course {course_run_key} to user {username}.'
error_msg = f'Failed to send grade {grade_str} for course {course_run_key} for user {username}.'
logger.exception(error_msg)
exception = MaxRetriesExceededError(
f"Failed to send grade to credentials. Reason: {error_msg}"
)
exception = MaxRetriesExceededError(f"Failed to send grade to credentials. Reason: {error_msg}")
raise self.retry(exc=exception, countdown=countdown, max_retries=MAX_RETRIES) # pylint: disable=raise-missing-from
@@ -99,21 +105,26 @@ def send_grade_to_credentials(
@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.
"""
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.
Args:
options (dict): Dictionary containing settings for this run of the `handle_notify_credentials` task
course_keys (list[string]): List of course run keys for this run of the `handle_notify_credentials` task
"""
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
# If a start_date or end_date are passed, this will include certificates
# with a CertificateDateOverride modified within the time range
# If a `start_date` or `end_date` are included in the options we should also include certificates with a
# CertificateDateOveride modified within the time range
certs = get_recently_modified_certificates(
course_keys, options['start_date'], options['end_date'], options['user_ids']
course_keys,
options['start_date'],
options['end_date'],
options['user_ids']
)
users = None
@@ -121,14 +132,15 @@ def handle_notify_credentials(options, course_keys):
users = User.objects.filter(id__in=options['user_ids'])
grades = get_recently_modified_grades(
course_keys, options['start_date'], options['end_date'], users
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()
))
logger.info(
f"[notify_credentials] Sending notifications for {certs.count()} certificates and {grades.count()} grades"
)
if options['dry_run']:
log_dry_run(certs, grades)
else:
@@ -142,13 +154,38 @@ def handle_notify_credentials(options, course_keys):
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
certs,
grades,
site_config=None,
delay=0,
page_size=100,
verbose=False,
notify_programs=False,
revoke_program_certs=False
):
""" Run actual handler commands for the provided certs and grades. """
"""
Utility function responsible for bootstrapping certificate and grade updates to the Credentials IDA. We do this by
invoking existing code paths that should force the LMS to (re-)send data to Credentials.
First, we directly call our Django signal handlers responsible for updating the status of certificates throughout
the system. This will trigger attempts to send certificate status information to the Credentials IDA.
Then, we call a utility function (`send_grade_if_interesting`) that is responsible for determining if the
Credentials IDA cares about this particular grade update for a learner.
Args:
certs (QuerySet): A collection of course certificates associated with the users we need to update
grades (QuerySet): A collection of grades associated with the users we need to update
site_config (SiteConfiguration): Optional, may be None. The site associated with the users we need to update.
delay (int): Delay (in seconds) to wait between queries
page_size (int): Number of records to process from the certs or grades QuerySet at once. Used to handle load.
verbose (bool): Used to determine level of logging output during grade updates
notify_programs (bool): Used to determine if an update should be sent to Credentials
revoke_program_certs (bool): Used to determine if the system should attempt revoking program certificates during
this run of the `notify_credentials` management command
"""
course_cert_info = {}
# First, do certs
for i, cert in paged_query(certs, delay, page_size):
@@ -156,11 +193,7 @@ def send_notifications(
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),
)
logger.info(f"Handling credential change {i} for certificate {certstr(cert)}")
signal_args = {
'sender': None,
'user': cert.user,
@@ -174,23 +207,23 @@ def send_notifications(
'mode': cert.mode,
'status': cert.status
}
course_cert_info[(cert.user.id, str(cert.course_id))] = data
# handles awarding course certificates in Credentials
handle_course_cert_changed(**signal_args)
# handles awarding program certificates in Credentials
if notify_programs and CertificateStatuses.is_passing_status(cert.status):
handle_course_cert_awarded(**signal_args)
# handles revoking program certificates in Credentials
if revoke_program_certs and notify_programs and not CertificateStatuses.is_passing_status(cert.status):
handle_course_cert_revoked(**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))
logger.info(f"Skipping grade change {i} for grade in {gradestr(grade)}")
continue
logger.info(
"Handling grade changes %d for grade %s",
i, gradestr(grade),
)
logger.info(f"Handling grade change {i} for grade in {gradestr(grade)}")
user = User.objects.get(id=grade.user_id)
# Grab mode/status from cert call
@@ -252,15 +285,15 @@ def log_dry_run(certs, grades):
logger.info(f"{certs.count()} Certificates:")
for cert in certs[:ITEMS_TO_SHOW]:
logger.info(f" {certstr(cert)}")
logger.info(f"\t{certstr(cert)}")
if certs.count() > ITEMS_TO_SHOW:
logger.info(f" (+ {certs.count() - ITEMS_TO_SHOW} more)")
logger.info(f"\t(+ {certs.count() - ITEMS_TO_SHOW} more)")
logger.info(f"{grades.count()} Grades:")
for grade in grades[:ITEMS_TO_SHOW]:
logger.info(f" {gradestr(grade)}")
logger.info(f"\t{gradestr(grade)}")
if grades.count() > ITEMS_TO_SHOW:
logger.info(f" (+ {grades.count() - ITEMS_TO_SHOW} more)")
logger.info(f"\t(+ {grades.count() - ITEMS_TO_SHOW} more)")
def certstr(cert):
@@ -271,8 +304,6 @@ 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,
@@ -283,8 +314,22 @@ def send_grade_if_interesting(
grade_last_updated=None,
verbose=False
):
""" Checks if grade is interesting to Credentials and schedules a Celery task if so. """
"""
Checks if a grade is interesting to Credentials and schedules a Celery task if so. This is Credentials business
logic that has bled into the LMS. We want to filter here in order to not flood our task queue with a bunch of
signals, so we put up with it.
Args:
user (User): User associated with this grade update
course_run_key (CourseLocator): The course run key associated with this grade update
mode (string): The "mode" for the specific course run (e.g. "verified", "audit", etc.)
status (string): The status of the Certificate associated with this grade update
letter_grade (string): The letter grade associated with this grade update (e.g. "A", "B", or "pass" or "fail")
percent_grade (float): A number representing the learner's grade in this course run
grade_last_updated (DateTime): DateTime object representing the last time the (percent) grade was updated in the
LMS.
verbose (bool): A value determining the logging level desired for this grade update
"""
if verbose:
msg = (
f"Starting send_grade_if_interesting with_params: user [{getattr(user, 'username', None)}], "
@@ -366,9 +411,17 @@ def send_grade_if_interesting(
def is_course_run_in_a_program(course_run_key):
""" Returns true if the given course key is in any program at all. """
"""
Returns true if the given course key is in any program at all. This functionality depends on data to be present in
the program cache. We don't have an easy way to determine if a course run is in a program, so we must search through
each program (on a site-by-site basis).
# 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.
Args:
course_run_key (CourseLocator): The course run key used we are using to verify program membership
Returns:
A boolean describing if the course run is part of a program
"""
sites = Site.objects.all()
str_key = str(course_run_key)
for site in sites:
@@ -384,10 +437,10 @@ def is_course_run_in_a_program(course_run_key):
@set_code_owner_attribute
def backfill_date_for_all_course_runs():
"""
This task will update the course certificate configuration's certificate_available_date
in credentials for all course runs. This is different from the "visable_date" attribute.
This date will always either be the available date that is set in studio for a given course, or it will be None.
This will exclude any course runs that do not have a certificate_available_date or are self paced.
This task will update the course certificate configuration's certificate_available_date in credentials for all
course runs. This is different from the "visable_date" attribute. This date will either be the available date that
is set in studio for a given course, or it will be None. This will exclude any course runs that do not have a
certificate_available_date or are self paced.
"""
course_run_list = CourseOverview.objects.exclude(self_paced=True).exclude(certificate_available_date=None)
for index, course_run in enumerate(course_run_list):
@@ -435,10 +488,9 @@ def backfill_date_for_all_course_runs():
@set_code_owner_attribute
def clean_certificate_available_date():
"""
This task will clean out the misconfigured certificate available date. When courses Change their
certificates_display_behavior, the certificate_available_date was not updating properly. This is
command is meant to be ran one time to clean up any courses that were not supposed to have
certificate_available_date
Historically, when courses changed their certificates_display_behavior, the certificate_available_date was not
updating properly. This task will clean out course runs that have a misconfigured certificate available date in the
Credentials IDA.
"""
course_run_list = CourseOverview.objects.exclude(
self_paced=0,
@@ -446,17 +498,13 @@ def clean_certificate_available_date():
certificate_available_date__isnull=False
)
for index, course_run in enumerate(course_run_list):
logger.info(
f"removing certificate_available_date for course {course_run.id}"
)
logger.info(f"removing certificate_available_date for course {course_run.id}")
course_key = str(course_run.id)
course_modes = CourseMode.objects.filter(course_id=course_key)
# There should only ever be one certificate relevant mode per course run
modes = [mode.slug for mode in course_modes if mode.slug in CourseMode.CERTIFICATE_RELEVANT_MODES]
if len(modes) != 1:
logger.exception(
f'Either course {course_key} has no certificate mode or multiple modes. Task failed.'
)
logger.exception(f'Either course {course_key} has no certificate mode or multiple modes. Task failed.')
# if there is only one relevant mode, post to credentials
else:
try:

193
openedx/core/djangoapps/credentials/tests/test_tasks.py
View File

@@ -8,15 +8,18 @@ from datetime import datetime
import ddt
import pytest
from django.conf import settings
from django.contrib.auth import get_user_model
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 common.djangoapps.student.tests.factories import UserFactory
from lms.djangoapps.certificates.api import get_recently_modified_certificates
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.models_api import get_recently_modified_grades
from lms.djangoapps.grades.tests.utils import mock_passing_grade
from lms.djangoapps.certificates.tests.factories import CertificateDateOverrideFactory, GeneratedCertificateFactory
from openedx.core.djangoapps.catalog.tests.factories import CourseFactory, CourseRunFactory, ProgramFactory
@@ -26,6 +29,8 @@ from openedx.core.djangolib.testing.utils import skip_unless_lms
from openedx.core.djangoapps.credentials.tasks.v1 import tasks
User = get_user_model()
TASKS_MODULE = 'openedx.core.djangoapps.credentials.tasks.v1.tasks'
@@ -107,8 +112,6 @@ class TestHandleNotifyCredentialsTask(TestCase):
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',
@@ -122,8 +125,6 @@ class TestHandleNotifyCredentialsTask(TestCase):
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,
@@ -557,3 +558,187 @@ class TestIsCourseRunInAProgramUtil(TestCase):
mock_get_programs.return_value = self.data
course_run2 = CourseRunFactory()
assert not tasks.is_course_run_in_a_program(course_run2['key'])
@ddt.ddt
@skip_unless_lms
@mock.patch('openedx.core.djangoapps.credentials.tasks.v1.tasks.handle_course_cert_changed')
@mock.patch('openedx.core.djangoapps.credentials.tasks.v1.tasks.handle_course_cert_awarded')
@mock.patch('openedx.core.djangoapps.credentials.tasks.v1.tasks.handle_course_cert_revoked')
@mock.patch('openedx.core.djangoapps.credentials.tasks.v1.tasks.send_grade_if_interesting')
class TestSendNotifications(TestCase):
"""
Unit Tests for the `send_notifications` function in the `tasks.py` file.
"""
def setUp(self):
super().setUp()
self.user = UserFactory()
self.course_run = CourseRunFactory()
self.course_key = CourseKey.from_string(self.course_run['key'])
self.course = CourseFactory(course_runs=[self.course_run])
self.site = SiteConfigurationFactory.create(
site_values={'course_org_filter': [self.course_key.org]},
)
self.certificate = GeneratedCertificateFactory(
user=self.user,
course_id=self.course_run['key'],
status=CertificateStatuses.downloadable
)
self.grade = PersistentCourseGrade.objects.create(
user_id=self.user.id,
course_id=self.course_run['key'],
percent_grade=1.0
)
def _get_certs_qs(self):
"""
The function under test expects a QuerySet containing certificate information, this retrieves the data needed
to invoke the `send_notifications` function in the form that is expected. We leverage a utility function that
the product code uses.
"""
return get_recently_modified_certificates([self.course_run['key']], None, None, [self.user.id])
def _get_grades_qs(self):
"""
The function under test expects a QuerySet containing grade information, this retrieves the data needed
to invoke the `send_notifications` function in the form that is expected. We leverage a utility function that
the product code uses.
"""
users = User.objects.filter(id__in=[self.user.id])
return get_recently_modified_grades([self.course_run['key']], None, None, users)
def _build_expected_signal_args(self):
return {
'sender': None,
'user': self.user,
'course_key': self.certificate.course_id,
'mode': self.certificate.mode,
'status': self.certificate.status,
'verbose': False
}
def test_send_notifications(self, mock_interesting, mock_revoked, mock_awarded, mock_changed):
"""
A test case that verifies the `send_notifications` functions behavior when the `notify_programs` flag is False.
"""
certs = self._get_certs_qs()
grades = self._get_grades_qs()
tasks.send_notifications(
certs,
grades,
self.site
)
expected_signal_args = self._build_expected_signal_args()
mock_changed.assert_called_once_with(**expected_signal_args)
mock_interesting.assert_called_once_with(
self.user,
self.grade.course_id,
self.certificate.mode,
self.certificate.status,
self.grade.letter_grade,
self.grade.percent_grade,
grade_last_updated=self.grade.modified,
verbose=False
)
mock_revoked.assert_not_called()
mock_awarded.assert_not_called()
@ddt.data(
['downloadable', True],
['unavailable', False]
)
@ddt.unpack
def test_send_notifications_notify_programs(
self,
cert_status,
award_expected,
mock_interesting,
mock_revoked,
mock_awarded,
mock_changed
):
"""
Test cases that verify the `send_notifications` functions behavior when the `notify_programs` flag is True.
"""
certs = self._get_certs_qs()
grades = self._get_grades_qs()
self.certificate.status = cert_status
self.certificate.save()
tasks.send_notifications(
certs,
grades,
self.site,
notify_programs=True
)
expected_signal_args = self._build_expected_signal_args()
mock_changed.assert_called_once_with(**expected_signal_args)
mock_interesting.assert_called_once_with(
self.user,
self.grade.course_id,
self.certificate.mode,
self.certificate.status,
self.grade.letter_grade,
self.grade.percent_grade,
grade_last_updated=self.grade.modified,
verbose=False
)
mock_revoked.assert_not_called()
if award_expected:
mock_awarded.assert_called_once_with(**expected_signal_args)
else:
mock_awarded.assert_not_called()
@ddt.data(
['downloadable', False],
['unavailable', True]
)
@ddt.unpack
def test_send_notifications_revoke_programs(
self,
cert_status,
revoke_expected,
mock_interesting,
mock_revoked,
mock_awarded,
mock_changed
):
"""
Test cases that verify the `send_notifications` functions behavior when the `revoke_program_certs` flag is True.
"""
certs = self._get_certs_qs()
grades = self._get_grades_qs()
self.certificate.status = cert_status
self.certificate.save()
tasks.send_notifications(
certs,
grades,
self.site,
notify_programs=True,
revoke_program_certs=True
)
expected_signal_args = self._build_expected_signal_args()
mock_changed.assert_called_once_with(**expected_signal_args)
mock_interesting.assert_called_once_with(
self.user,
self.grade.course_id,
self.certificate.mode,
self.certificate.status,
self.grade.letter_grade,
self.grade.percent_grade,
grade_last_updated=self.grade.modified,
verbose=False
)
if revoke_expected:
mock_revoked.assert_called_once_with(**expected_signal_args)
mock_awarded.mock_not_called()
else:
mock_revoked.assert_not_called()
mock_awarded.assert_called_once_with(**expected_signal_args)