Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Remove V2 certificate checks from the certificates app #28102

Merged
merged 1 commit into from
Jul 8, 2021
Merged

fix: Remove V2 certificate checks from the certificates app #28102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
119 changes: 15 additions & 104 deletions lms/djangoapps/certificates/signals.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,9 @@
from django.dispatch import receiver

from common.djangoapps.course_modes import api as modes_api
from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.signals import ENROLLMENT_TRACK_UPDATED
from lms.djangoapps.certificates.generation_handler import (
can_generate_certificate_task,
generate_allowlist_certificate_task,
generate_certificate_task,
is_on_certificate_allowlist
Expand All @@ -23,8 +21,6 @@
CertificateStatuses,
GeneratedCertificate
)
from lms.djangoapps.certificates.tasks import CERTIFICATE_DELAY_SECONDS, generate_certificate
from lms.djangoapps.grades.api import CourseGradeFactory
from lms.djangoapps.verify_student.services import IDVerificationService
from openedx.core.djangoapps.certificates.api import auto_certificate_generation_enabled
from openedx.core.djangoapps.content.course_overviews.signals import COURSE_PACING_CHANGED
Expand Down Expand Up @@ -65,12 +61,6 @@ def _listen_for_certificate_allowlist_append(sender, instance, **kwargs): # pyl
f'made to generate an allowlist certificate.')
return generate_allowlist_certificate_task(instance.user, instance.course_id)

if _fire_ungenerated_certificate_task(instance.user, instance.course_id):
log.info('Certificate generation task initiated for {user} : {course} via allowlist'.format(
user=instance.user.id,
course=instance.course_id
))


@receiver(COURSE_GRADE_NOW_PASSED, dispatch_uid="new_passing_learner")
def listen_for_passing_grade(sender, user, course_id, **kwargs): # pylint: disable=unused-argument
Expand All @@ -82,21 +72,14 @@ def listen_for_passing_grade(sender, user, course_id, **kwargs): # pylint: disa
if not auto_certificate_generation_enabled():
return

if can_generate_certificate_task(user, course_id):
cert = GeneratedCertificate.certificate_for_student(user, course_id)
if cert is not None and CertificateStatuses.is_passing_status(cert.status):
log.info(f'{course_id} is using V2 certificates, and the cert status is already passing for user '
f'{user.id}. Passing grade signal will be ignored.')
return
log.info(f'{course_id} is using V2 certificates. Attempt will be made to generate a V2 certificate for '
f'{user.id} as a passing grade was received.')
return generate_certificate_task(user, course_id)

if _fire_ungenerated_certificate_task(user, course_id):
log.info('Certificate generation task initiated for {user} : {course} via passing grade'.format(
user=user.id,
course=course_id
))
cert = GeneratedCertificate.certificate_for_student(user, course_id)
if cert is not None and CertificateStatuses.is_passing_status(cert.status):
log.info(f'The cert status is already passing for user {user.id} : {course_id}. Passing grade signal will be '
f'ignored.')
return
log.info(f'Attempt will be made to generate a course certificate for {user.id} : {course_id} as a passing grade '
f'was received.')
return generate_certificate_task(user, course_id)


@receiver(COURSE_GRADE_NOW_FAILED, dispatch_uid="new_failing_learner")
Expand Down Expand Up @@ -126,33 +109,18 @@ def _listen_for_failing_grade(sender, user, course_id, grade, **kwargs): # pyli
def _listen_for_id_verification_status_changed(sender, user, **kwargs): # pylint: disable=unused-argument
"""
Listen for a signal indicating that the user's id verification status has changed.

If needed, generate a certificate task.
"""
if not auto_certificate_generation_enabled():
return

user_enrollments = CourseEnrollment.enrollments_for_user(user=user)

grade_factory = CourseGradeFactory()
expected_verification_status = IDVerificationService.user_status(user)
expected_verification_status = expected_verification_status['status']

for enrollment in user_enrollments:
if can_generate_certificate_task(user, enrollment.course_id):
log.info(f'{enrollment.course_id} is using V2 certificates. Attempt will be made to generate a V2 '
f'certificate for {user.id}. Id verification status is {expected_verification_status}')
generate_certificate_task(user, enrollment.course_id)
elif grade_factory.read(user=user, course=enrollment.course_overview).passed:
if _fire_ungenerated_certificate_task(user, enrollment.course_id, expected_verification_status):
message = (
'Certificate generation task initiated for {user} : {course} via track change ' +
'with verification status of {status}'
)
log.info(message.format(
user=user.id,
course=enrollment.course_id,
status=expected_verification_status
))
log.info(f'Attempt will be made to generate a course certificate for {user.id} : {enrollment.course_id}. Id '
f'verification status is {expected_verification_status}')
generate_certificate_task(user, enrollment.course_id)


@receiver(ENROLLMENT_TRACK_UPDATED)
Expand All @@ -164,63 +132,6 @@ def _listen_for_enrollment_mode_change(sender, user, course_key, mode, **kwargs)
if the user has moved to the audit track.
"""
if modes_api.is_eligible_for_certificate(mode):
if can_generate_certificate_task(user, course_key):
log.info(f'{course_key} is using V2 certificates. Attempt will be made to generate a V2 certificate for '
f'{user.id} since the enrollment mode is now {mode}.')
generate_certificate_task(user, course_key)


def _fire_ungenerated_certificate_task(user, course_key, expected_verification_status=None):
"""
Helper function to fire certificate generation task.
Auto-generation of certificates is available for following course modes:
1- VERIFIED
2- CREDIT_MODE
3- PROFESSIONAL
4- NO_ID_PROFESSIONAL_MODE

Certificate generation task is fired to either generate a certificate
when there is no generated certificate for user in a particular course or
update a certificate if it has 'unverified' status.

Task is fired to attempt an update to a certificate
with 'unverified' status as this method is called when a user is
successfully verified, any certificate associated
with such user can now be verified.

NOTE: Purpose of restricting other course modes (HONOR and AUDIT) from auto-generation is to reduce
traffic to workers.
"""

message = 'Entered into Ungenerated Certificate task for {user} : {course}'
log.info(message.format(user=user.id, course=course_key))

allowed_enrollment_modes_list = [
CourseMode.VERIFIED,
CourseMode.CREDIT_MODE,
CourseMode.PROFESSIONAL,
CourseMode.NO_ID_PROFESSIONAL_MODE,
CourseMode.MASTERS,
CourseMode.EXECUTIVE_EDUCATION,
]
enrollment_mode, __ = CourseEnrollment.enrollment_mode_for_user(user, course_key)
cert = GeneratedCertificate.certificate_for_student(user, course_key)

generate_learner_certificate = (
enrollment_mode in allowed_enrollment_modes_list and (
cert is None or cert.status == CertificateStatuses.unverified)
)

if generate_learner_certificate:
kwargs = {
'student': str(user.id),
'course_key': str(course_key)
}
if expected_verification_status:
kwargs['expected_verification_status'] = str(expected_verification_status)
generate_certificate.apply_async(countdown=CERTIFICATE_DELAY_SECONDS, kwargs=kwargs)
return True

message = 'Certificate Generation task failed for {user} : {course}'
log.info(message.format(user=user.id, course=course_key))
return False
log.info(f'Attempt will be made to generate a course certificate for {user.id} : {course_key} since the '
f'enrollment mode is now {mode}.')
generate_certificate_task(user, course_key)
4 changes: 0 additions & 4 deletions lms/djangoapps/certificates/tests/test_generation_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,10 @@
from unittest import mock

import ddt
from edx_toggles.toggles.testutils import override_waffle_flag

from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
from lms.djangoapps.certificates.data import CertificateStatuses
from lms.djangoapps.certificates.generation_handler import (
CERTIFICATES_USE_UPDATED,
_can_generate_allowlist_certificate,
_can_generate_certificate_for_status,
_can_generate_v2_certificate,
Expand Down Expand Up @@ -271,7 +269,6 @@ def test_cert_status_downloadable(self):
assert _set_allowlist_cert_status(u, key) is None


@override_waffle_flag(CERTIFICATES_USE_UPDATED, active=True)
@mock.patch(ID_VERIFIED_METHOD, mock.Mock(return_value=True))
@mock.patch(CCX_COURSE_METHOD, mock.Mock(return_value=False))
@mock.patch(PASSING_GRADE_METHOD, mock.Mock(return_value=True))
Expand Down Expand Up @@ -313,7 +310,6 @@ def test_handle_valid_task(self):
"""
assert generate_regular_certificate_task(self.user, self.course_run_key) is True

@override_waffle_flag(CERTIFICATES_USE_UPDATED, active=False)
def test_handle_invalid(self):
"""
Test handling of an invalid user/course run combo
Expand Down
107 changes: 16 additions & 91 deletions lms/djangoapps/certificates/tests/test_signals.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,11 @@

from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
from lms.djangoapps.certificates.api import cert_generation_enabled
from lms.djangoapps.certificates.generation_handler import CERTIFICATES_USE_UPDATED
from lms.djangoapps.certificates.data import CertificateStatuses
from lms.djangoapps.certificates.models import (
CertificateGenerationConfiguration,
GeneratedCertificate
)
from lms.djangoapps.certificates.signals import _fire_ungenerated_certificate_task
from lms.djangoapps.certificates.tests.factories import CertificateAllowlistFactory, GeneratedCertificateFactory
from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory
from lms.djangoapps.grades.tests.utils import mock_passing_grade
Expand Down Expand Up @@ -79,40 +77,30 @@ def test_fire_task_allowlist_auto_enabled(self):
Test that the allowlist generation is invoked if automatic generation is enabled
"""
with mock.patch(
'lms.djangoapps.certificates.signals.generate_certificate.apply_async',
'lms.djangoapps.certificates.signals.generate_allowlist_certificate_task',
return_value=None
) as mock_generate_certificate_apply_async:
with mock.patch(
'lms.djangoapps.certificates.signals.generate_allowlist_certificate_task',
return_value=None
) as mock_generate_allowlist_task:
with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True):
CertificateAllowlistFactory(
user=self.user,
course_id=self.ip_course.id
)
mock_generate_certificate_apply_async.assert_not_called()
mock_generate_allowlist_task.assert_called_with(self.user, self.ip_course.id)
) as mock_generate_allowlist_task:
with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True):
CertificateAllowlistFactory(
user=self.user,
course_id=self.ip_course.id
)
mock_generate_allowlist_task.assert_called_with(self.user, self.ip_course.id)

def test_fire_task_allowlist_auto_disabled(self):
"""
Test that the allowlist generation is not invoked if automatic generation is disabled
"""
with mock.patch(
'lms.djangoapps.certificates.signals.generate_certificate.apply_async',
'lms.djangoapps.certificates.signals.generate_allowlist_certificate_task',
return_value=None
) as mock_generate_certificate_apply_async:
with mock.patch(
'lms.djangoapps.certificates.signals.generate_allowlist_certificate_task',
return_value=None
) as mock_generate_allowlist_task:
with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=False):
CertificateAllowlistFactory(
user=self.user,
course_id=self.ip_course.id
)
mock_generate_certificate_apply_async.assert_not_called()
mock_generate_allowlist_task.assert_not_called()
) as mock_generate_allowlist_task:
with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=False):
CertificateAllowlistFactory(
user=self.user,
course_id=self.ip_course.id
)
mock_generate_allowlist_task.assert_not_called()


class PassingGradeCertsTest(ModuleStoreTestCase):
Expand Down Expand Up @@ -146,23 +134,6 @@ def setUp(self):
)
attempt.approve()

def test_cert_already_generated(self):
with mock.patch(
'lms.djangoapps.certificates.signals.generate_certificate.apply_async',
return_value=None
) as mock_generate_certificate_apply_async:
grade_factory = CourseGradeFactory()
# Create the certificate
GeneratedCertificateFactory(
user=self.user,
course_id=self.course.id,
status=CertificateStatuses.downloadable
)
# Certs are not re-fired after passing
with mock_passing_grade():
grade_factory.update(self.user, self.course)
mock_generate_certificate_apply_async.assert_not_called()

def test_passing_grade_allowlist(self):
with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True):
# User who is not on the allowlist
Expand Down Expand Up @@ -200,7 +171,6 @@ def test_passing_grade_allowlist(self):
CourseGradeFactory().update(u, c)
mock_cert_task.assert_called_with(u, course_key)

@override_waffle_flag(CERTIFICATES_USE_UPDATED, active=True)
def test_cert_already_generated_downloadable(self):
with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True):
GeneratedCertificateFactory(
Expand All @@ -218,7 +188,6 @@ def test_cert_already_generated_downloadable(self):
grade_factory.update(self.user, self.course)
mock_cert_task.assert_not_called()

@override_waffle_flag(CERTIFICATES_USE_UPDATED, active=True)
def test_cert_already_generated_unverified(self):
with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True):
GeneratedCertificateFactory(
Expand All @@ -236,7 +205,6 @@ def test_cert_already_generated_unverified(self):
grade_factory.update(self.user, self.course)
mock_cert_task.assert_called_with(self.user, self.course_key)

@override_waffle_flag(CERTIFICATES_USE_UPDATED, active=True)
def test_without_cert(self):
with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True):
with mock.patch(
Expand Down Expand Up @@ -362,7 +330,6 @@ def setUp(self):
grade_factory.update(self.user_one, self.course_one)
grade_factory.update(self.user_two, self.course_two)

@override_waffle_flag(CERTIFICATES_USE_UPDATED, active=True)
def test_cert_generation_on_photo_verification(self):
with mock.patch(
'lms.djangoapps.certificates.signals.generate_certificate_task',
Expand Down Expand Up @@ -417,48 +384,6 @@ def test_id_verification_allowlist(self):
mock_allowlist_task.assert_called_with(u, course_key)


@ddt.ddt
class CertificateGenerationTaskTest(ModuleStoreTestCase):
"""
Tests for certificate generation task.
"""

def setUp(self):
super().setUp()
self.course = CourseFactory.create()

@ddt.data(
('professional', True),
('verified', True),
('no-id-professional', True),
('credit', True),
('masters', True),
('audit', False),
('honor', False),
)
@ddt.unpack
def test_fire_ungenerated_certificate_task_allowed_modes(self, enrollment_mode, should_create):
"""
Test that certificate generation task is fired for only modes that are
allowed to generate certificates automatically.
"""
self.user = UserFactory.create()
CourseEnrollmentFactory(
user=self.user,
course_id=self.course.id,
is_active=True,
mode=enrollment_mode
)
with mock.patch(
'lms.djangoapps.certificates.signals.generate_certificate.apply_async',
return_value=None
) as mock_generate_certificate_apply_async:
with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True):
_fire_ungenerated_certificate_task(self.user, self.course.id)
task_created = mock_generate_certificate_apply_async.called
assert task_created == should_create


@override_waffle_flag(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True)
class EnrollmentModeChangeCertsTest(ModuleStoreTestCase):
"""
Expand Down
Loading