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

feat: Send modified data of a learner's grade to the Credentials IDA

Browse Source 
[APER-1968]

We don't have a good way to understand if grade data in Credentials is out of sync with the LMS. Grades are sent to Credentials via a REST API call originating from an asynchronous Celery task on the LMS side. This PR updates our Celery task `send_grade_to_credentials` to include sending the `modified` DateTime value of a grade record to the Credentials IDA. Updates will be made on the Credentials side to accept and store this data as part of the UserGrade instance.

* Updates the `send_grade_to_credentials` task to include passing the grade's `modified` DateTime info as part of the request data to Credentials
* Updates the `CourseGradeBase` class to include an optional `last_updated` field. This will store the `modified` date of a PersistentCourseGrade instance when a grade is read through the CourseGradeFactory.
* Update existing log statement to use format strings where possible.
This commit is contained in:
Justin Hynes
2022-09-14 15:19:21 -04:00
parent 795fbbde8c
commit ef6de11467
5 changed files with 78 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
lms/djangoapps/grades/course_grade.py
View File

@@ -22,7 +22,16 @@ class CourseGradeBase:
"""
Base class for Course Grades.
"""
def __init__(self, user, course_data, percent=0.0, letter_grade=None, passed=False, force_update_subsections=False):
def __init__(
self,
user,
course_data,
percent=0.0,
letter_grade=None,
passed=False,
force_update_subsections=False,
last_updated=None
):
self.user = user
self.course_data = course_data
@@ -33,6 +42,8 @@ class CourseGradeBase:
self.letter_grade = letter_grade or None
self.force_update_subsections = force_update_subsections
self.last_updated = last_updated
def __str__(self):
return 'Course Grade: percent: {}, letter_grade: {}, passed: {}'.format(
str(self.percent),

3
lms/djangoapps/grades/course_grade_factory.py
View File

@@ -143,7 +143,8 @@ class CourseGradeFactory:
course_data,
persistent_grade.percent_grade,
persistent_grade.letter_grade,
persistent_grade.letter_grade != ''
persistent_grade.letter_grade != '',
last_updated=persistent_grade.modified
)
@staticmethod

3
lms/djangoapps/grades/tests/utils.py
View File

@@ -15,7 +15,7 @@ from xmodule.graders import ProblemScore # lint-amnesty, pylint: disable=wrong-
@contextmanager
def mock_passing_grade(letter_grade='Pass', percent=0.75, ):
def mock_passing_grade(letter_grade='Pass', percent=0.75, last_updated=None):
"""
Mock the grading function to always return a passing grade.
"""
@@ -24,6 +24,7 @@ def mock_passing_grade(letter_grade='Pass', percent=0.75, ):
percent=percent,
passed=letter_grade is not None,
attempted=True,
last_updated=last_updated,
)
with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_grade_read:
mock_grade_read.return_value = MagicMock(**passing_grade_fields)

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

@@ -48,7 +48,15 @@ MAX_RETRIES = 11
@shared_task(bind=True, ignore_result=True)
@set_code_owner_attribute
def send_grade_to_credentials(self, username, course_run_key, verified, letter_grade, percent_grade):
def send_grade_to_credentials(
self,
username,
course_run_key,
verified,
letter_grade,
percent_grade,
grade_last_updated
):
"""
Celery task to notify the Credentials IDA of a grade change via POST.
"""
@@ -66,10 +74,11 @@ def send_grade_to_credentials(self, username, course_run_key, verified, letter_g
api_url,
data={
'username': username,
'course_run': str(course_key),
'course_run': course_run_key,
'letter_grade': letter_grade,
'percent_grade': percent_grade,
'verified': verified,
'lms_last_updated_at': grade_last_updated
}
)
response.raise_for_status()
@@ -197,6 +206,7 @@ def send_notifications(
status,
grade.letter_grade,
grade.percent_grade,
grade_last_updated=grade.modified,
verbose=verbose
)
@@ -263,28 +273,26 @@ def gradestr(grade):
# 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):
def send_grade_if_interesting(
user,
course_run_key,
mode,
status,
letter_grade,
percent_grade,
grade_last_updated=None,
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
)
msg = (
f"Starting send_grade_if_interesting with_params: user [{getattr(user, 'username', None)}], "
f"course_run_key [{course_run_key}], mode [{mode}], status [{status}], letter_grade [{letter_grade}], "
f"percent_grade [{percent_grade}], grade_last_updated [{grade_last_updated}], 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:
@@ -311,10 +319,8 @@ def send_grade_if_interesting(user, course_run_key, mode, status, letter_grade,
# 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)
)
f"Skipping send grade: no cert for user [{getattr(user, 'username', None)}] & course_id "
f"[{course_run_key}]"
)
return
@@ -323,12 +329,7 @@ def send_grade_if_interesting(user, course_run_key, mode, status, letter_grade,
# 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
)
)
logger.info(f"Skipping send grade: mode/status uninteresting for mode [{mode}] & status [{status}]")
return
# If the course isn't in any program, don't bother telling Credentials about it. When Credentials grows support
@@ -336,26 +337,32 @@ def send_grade_if_interesting(user, course_run_key, mode, status, letter_grade,
if not is_course_run_in_a_program(course_run_key):
if verbose:
logger.info(
f"Skipping send grade: course run not in a program. [{str(course_run_key)}]"
f"Skipping send grade: course run not in a program. [{course_run_key}]"
)
return
# Grab grades if we don't have them in hand
if letter_grade is None or percent_grade is None:
# Grab grade data if we don't have them in hand
if letter_grade is None or percent_grade is None or grade_last_updated 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)
)
f"Skipping send grade: No grade found for user [{getattr(user, 'username', None)}] & course_id "
f"[{course_run_key}]"
)
return
letter_grade = grade.letter_grade
percent_grade = grade.percent
grade_last_updated = grade.last_updated
send_grade_to_credentials.delay(user.username, str(course_run_key), True, letter_grade, percent_grade)
send_grade_to_credentials.delay(
user.username,
str(course_run_key),
True,
letter_grade,
percent_grade,
grade_last_updated
)
def is_course_run_in_a_program(course_run_key):

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

@@ -51,7 +51,16 @@ class TestSendGradeToCredentialTask(TestCase):
api_client = mock.MagicMock()
mock_get_api_client.return_value = api_client
tasks.send_grade_to_credentials.delay('user', 'course-v1:org+course+run', True, 'A', 1.0).get()
last_updated = datetime.now()
tasks.send_grade_to_credentials.delay(
'user',
'course-v1:org+course+run',
True,
'A',
1.0,
last_updated
).get()
assert mock_get_api_client.call_count == 1
assert mock_get_api_client.call_args[0] == (self.user,)
@@ -63,6 +72,7 @@ class TestSendGradeToCredentialTask(TestCase):
'letter_grade': 'A',
'percent_grade': 1.0,
'verified': True,
'lms_last_updated_at': last_updated.isoformat(),
})
def test_retry(self, mock_get_api_client):
@@ -71,7 +81,7 @@ class TestSendGradeToCredentialTask(TestCase):
"""
mock_get_api_client.side_effect = boom
task = tasks.send_grade_to_credentials.delay('user', 'course-v1:org+course+run', True, 'A', 1.0)
task = tasks.send_grade_to_credentials.delay('user', 'course-v1:org+course+run', True, 'A', 1.0, None)
pytest.raises(Exception, task.get)
assert mock_get_api_client.call_count == (tasks.MAX_RETRIES + 1)
@@ -482,10 +492,13 @@ class TestSendGradeIfInteresting(TestCase):
_mock_is_learner_issuance_enabled):
mock_is_course_run_in_a_program.return_value = True
with mock_passing_grade('B', 0.81):
last_updated = datetime.now()
with mock_passing_grade('B', 0.81, last_updated):
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)
assert mock_send_grade_to_credentials.delay.call_args[0] == (
self.user.username, str(self.key), True, 'B', 0.81, last_updated
)
mock_send_grade_to_credentials.delay.reset_mock()
def test_send_grade_without_issuance_enabled(self, _mock_is_course_run_in_a_program,