Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions lms/djangoapps/certificates/generation_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def generate_allowlist_certificate_task(user, course_key, generation_mode=None):
Create a task to generate an allowlist certificate for this user in this course run.
"""
enrollment_mode = _get_enrollment_mode(user, course_key)
course_grade = _get_course_grade(user, course_key)
course_grade = _get_course_grade(user, course_key, send_grade_signals=False)
if _can_generate_allowlist_certificate(user, course_key, enrollment_mode):
return _generate_certificate_task(user=user, course_key=course_key, enrollment_mode=enrollment_mode,
course_grade=course_grade, generation_mode=generation_mode)
Expand All @@ -68,7 +68,7 @@ def _generate_regular_certificate_task(user, course_key, generation_mode=None):
eligible and a certificate can be generated.
"""
enrollment_mode = _get_enrollment_mode(user, course_key)
course_grade = _get_course_grade(user, course_key)
course_grade = _get_course_grade(user, course_key, send_grade_signals=False)
if _can_generate_regular_certificate(user, course_key, enrollment_mode, course_grade):
return _generate_certificate_task(user=user, course_key=course_key, enrollment_mode=enrollment_mode,
course_grade=course_grade, generation_mode=generation_mode)
Expand Down Expand Up @@ -371,11 +371,14 @@ def _get_grade_value(course_grade):
return ''


def _get_course_grade(user, course_key):
def _get_course_grade(user, course_key, send_grade_signals=True):
"""
Get the user's course grade in this course run. Note that this may be None.

Use send_grade_signals=False to avoid firing the course grade signals recursively.
See details in lms/djangoapps/grades/course_grade_factory.py _update method.
"""
return CourseGradeFactory().read(user, course_key=course_key)
return CourseGradeFactory().read(user, course_key=course_key, send_grade_signals=send_grade_signals)


def _get_enrollment_mode(user, course_key):
Expand Down
55 changes: 30 additions & 25 deletions lms/djangoapps/grades/course_grade_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ def read(
course_structure=None,
course_key=None,
create_if_needed=True,
send_grade_signals=True,
):
"""
Returns the CourseGrade for the given user in the course.
Expand All @@ -51,7 +52,7 @@ def read(
if assume_zero_if_absent(course_data.course_key):
return self._create_zero(user, course_data)
elif create_if_needed:
return self._update(user, course_data)
return self._update(user, course_data, send_grade_signals=send_grade_signals)
else:
return None

Expand Down Expand Up @@ -161,13 +162,16 @@ def _read(user, course_data):
)

@staticmethod
def _update(user, course_data, force_update_subsections=False):
def _update(user, course_data, force_update_subsections=False, send_grade_signals=True):
"""
Computes, saves, and returns a CourseGrade object for the
given user and course.
Sends a COURSE_GRADE_CHANGED signal to listeners and
COURSE_GRADE_NOW_PASSED if learner has passed course or
COURSE_GRADE_NOW_FAILED if learner is now failing course
Computes, saves, and returns a CourseGrade object for the given user and course.

send_grade_signals defines if signals should be sent. Use it to avoid recursion issues in
cases when the signal listener trying to get grades but Persistent Grades are disabled.
If True - sends:
COURSE_GRADE_CHANGED signal to listeners and
COURSE_GRADE_NOW_PASSED if learner has passed course or
COURSE_GRADE_NOW_FAILED if learner is now failing course
"""
should_persist = should_persist_grades(course_data.course_key)
if should_persist and force_update_subsections:
Expand All @@ -194,26 +198,27 @@ def _update(user, course_data, force_update_subsections=False):
passed=course_grade.passed,
)

COURSE_GRADE_CHANGED.send_robust(
sender=None,
user=user,
course_grade=course_grade,
course_key=course_data.course_key,
deadline=course_data.course.end,
)
if course_grade.passed:
COURSE_GRADE_NOW_PASSED.send(
sender=CourseGradeFactory,
user=user,
course_id=course_data.course_key,
)
else:
COURSE_GRADE_NOW_FAILED.send(
sender=CourseGradeFactory,
if send_grade_signals:
COURSE_GRADE_CHANGED.send_robust(
sender=None,
user=user,
course_id=course_data.course_key,
grade=course_grade,
course_grade=course_grade,
course_key=course_data.course_key,
deadline=course_data.course.end,
)
if course_grade.passed:
COURSE_GRADE_NOW_PASSED.send(
sender=CourseGradeFactory,
user=user,
course_id=course_data.course_key,
)
else:
COURSE_GRADE_NOW_FAILED.send(
sender=CourseGradeFactory,
user=user,
course_id=course_data.course_key,
grade=course_grade,
)

log.info(
'Grades: Update, %s, User: %s, %s, persisted: %s',
Expand Down
38 changes: 35 additions & 3 deletions lms/djangoapps/grades/tests/test_course_grade_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,21 @@
Tests for the CourseGradeFactory class.
"""
import itertools
from unittest.mock import patch
from unittest.mock import patch, Mock

import ddt
from django.conf import settings
from edx_toggles.toggles.testutils import override_waffle_switch
import pytest

from common.djangoapps.student.tests.factories import UserFactory
from lms.djangoapps.certificates.config import AUTO_CERTIFICATE_GENERATION
from lms.djangoapps.courseware.access import has_access
from lms.djangoapps.grades.config.tests.utils import persistent_grades_feature_flags
from openedx.core.djangoapps.content.block_structure.factory import BlockStructureFactory
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
from openedx.core.djangoapps.signals.signals import COURSE_GRADE_NOW_PASSED
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order

from ..config.waffle import ASSUME_ZERO_GRADE_IF_ABSENT, waffle_switch
from ..course_grade import CourseGrade, ZeroCourseGrade
Expand Down Expand Up @@ -73,6 +76,35 @@ def test_course_grade_feature_gating(self, feature_flag, course_setting):
grade_factory.read(self.request.user, self.course)
assert mock_read_grade.called == (feature_flag and course_setting)

@patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False})
def test_no_recursion_without_persistent_grades(self):
"""
Course grade signals should not be fired recursively when persistent grades are disabled.
"""
self.mock_process_signal = Mock() # pylint: disable=attribute-defined-outside-init

def handler(**kwargs):
"""
Mock signal receiver.
"""
self.mock_process_signal()

with persistent_grades_feature_flags(
global_flag=False,
enabled_for_all_courses=False,
course_id=self.course.id,
enabled_for_course=False
):
with override_waffle_switch(AUTO_CERTIFICATE_GENERATION, active=True), mock_get_score(2, 2):
COURSE_GRADE_NOW_PASSED.connect(handler)
try:
CourseGradeFactory().update(self.request.user, self.course)
except RecursionError:
pytest.fail("The COURSE_GRADE_NOW_PASSED signal fired recursively.")

self.mock_process_signal.assert_called_once()
COURSE_GRADE_NOW_PASSED.disconnect(handler)

def test_read_and_update(self):
grade_factory = CourseGradeFactory()

Expand Down