diff --git a/lms/djangoapps/certificates/generation_handler.py b/lms/djangoapps/certificates/generation_handler.py index b37041a06e82..a1a3992eb3ec 100644 --- a/lms/djangoapps/certificates/generation_handler.py +++ b/lms/djangoapps/certificates/generation_handler.py @@ -8,6 +8,7 @@ import logging from django.conf import settings +from openedx_filters.learning.filters import CertificateCreationRequested from common.djangoapps.course_modes import api as modes_api from common.djangoapps.course_modes.models import CourseMode @@ -28,6 +29,14 @@ log = logging.getLogger(__name__) +class GeneratedCertificateException(Exception): + pass + + +class CertificateGenerationNotAllowed(GeneratedCertificateException): + pass + + def generate_certificate_task(user, course_key, generation_mode=None, delay_seconds=CERTIFICATE_DELAY_SECONDS): """ Create a task to generate a certificate for this user in this course run, if the user is eligible and a certificate @@ -55,9 +64,19 @@ def generate_allowlist_certificate_task(user, course_key, generation_mode=None, enrollment_mode = _get_enrollment_mode(user, course_key) course_grade = _get_course_grade(user, course_key, send_course_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, - delay_seconds=delay_seconds) + try: + return _generate_certificate_task( + user=user, course_key=course_key, enrollment_mode=enrollment_mode, course_grade=course_grade, + generation_mode=generation_mode, delay_seconds=delay_seconds, + ) + except CertificateGenerationNotAllowed: + # Catch exception to contain error message in console. + log.error( + "Certificate generation not allowed for user %s in course %s", + user.id, + course_key, + ) + return False status = _set_allowlist_cert_status(user, course_key, enrollment_mode, course_grade) if status is not None: @@ -94,6 +113,20 @@ def _generate_certificate_task(user, course_key, enrollment_mode, course_grade, course_grade_val = _get_grade_value(course_grade) + try: + # .. filter_implemented_name: CertificateCreationRequested + # .. filter_type: org.openedx.learning.certificate.creation.requested.v1 + user, course_key, enrollment_mode, status, course_grade, generation_mode = CertificateCreationRequested.run_filter( # pylint: disable=line-too-long + user=user, + course_key=course_key, + mode=enrollment_mode, + status=status, + grade=course_grade, + generation_mode=generation_mode, + ) + except CertificateCreationRequested.PreventCertificateCreation as exc: + raise CertificateGenerationNotAllowed(str(exc)) from exc + kwargs = { 'student': str(user.id), 'course_key': str(course_key), diff --git a/lms/djangoapps/certificates/management/commands/cert_generation.py b/lms/djangoapps/certificates/management/commands/cert_generation.py index 122e16a6754d..e80d2c413d46 100644 --- a/lms/djangoapps/certificates/management/commands/cert_generation.py +++ b/lms/djangoapps/certificates/management/commands/cert_generation.py @@ -9,6 +9,7 @@ from django.core.management.base import BaseCommand, CommandError from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey +from lms.djangoapps.certificates.generation_handler import CertificateGenerationNotAllowed from lms.djangoapps.certificates.generation_handler import generate_certificate_task from lms.djangoapps.certificates.models import CertificateGenerationCommandConfiguration @@ -96,4 +97,11 @@ def handle(self, *args, **options): user=user.id, course=course_key )) - generate_certificate_task(user, course_key) + try: + generate_certificate_task(user, course_key) + except CertificateGenerationNotAllowed as e: + log.exception( + "Certificate generation not allowed for user %s in course %s", + user.id, + course_key, + ) diff --git a/lms/djangoapps/certificates/signals.py b/lms/djangoapps/certificates/signals.py index 7fdb77b98261..28e68e340253 100644 --- a/lms/djangoapps/certificates/signals.py +++ b/lms/djangoapps/certificates/signals.py @@ -11,6 +11,7 @@ from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.signals import ENROLLMENT_TRACK_UPDATED from lms.djangoapps.certificates.generation_handler import ( + CertificateGenerationNotAllowed, generate_allowlist_certificate_task, generate_certificate_task, is_on_certificate_allowlist @@ -79,7 +80,15 @@ def listen_for_passing_grade(sender, user, course_id, **kwargs): # pylint: disa 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) + try: + return generate_certificate_task(user, course_id) + except CertificateGenerationNotAllowed as e: + log.exception( + "Certificate generation not allowed for user %s in course %s", + str(user), + course_id, + ) + return False @receiver(COURSE_GRADE_NOW_FAILED, dispatch_uid="new_failing_learner") @@ -117,7 +126,14 @@ def _listen_for_id_verification_status_changed(sender, user, **kwargs): # pylin for enrollment in user_enrollments: 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) + try: + generate_certificate_task(user, enrollment.course_id) + except CertificateGenerationNotAllowed as e: + log.exception( + "Certificate generation not allowed for user %s in course %s", + str(user), + enrollment.course_id, + ) @receiver(ENROLLMENT_TRACK_UPDATED) @@ -131,4 +147,12 @@ def _listen_for_enrollment_mode_change(sender, user, course_key, mode, **kwargs) if modes_api.is_eligible_for_certificate(mode): 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) + try: + return generate_certificate_task(user, course_key) + except CertificateGenerationNotAllowed as e: + log.exception( + "Certificate generation not allowed for user %s in course %s", + str(user), + course_key, + ) + return False diff --git a/lms/djangoapps/certificates/tests/test_filters.py b/lms/djangoapps/certificates/tests/test_filters.py new file mode 100644 index 000000000000..781b77461f41 --- /dev/null +++ b/lms/djangoapps/certificates/tests/test_filters.py @@ -0,0 +1,379 @@ +""" +Test that various filters are fired for models in the certificates app. +""" +from unittest import mock + +from django.core.management import call_command +from django.test import override_settings +from django.urls import reverse +from openedx_filters import PipelineStep +from openedx_filters.learning.filters import CertificateCreationRequested +from rest_framework import status as status_code +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + +from common.djangoapps.course_modes.models import CourseMode +from common.djangoapps.student.roles import SupportStaffRole +from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory +from lms.djangoapps.certificates.generation_handler import ( + CertificateGenerationNotAllowed, + generate_allowlist_certificate_task, + generate_certificate_task +) +from lms.djangoapps.certificates.models import GeneratedCertificate +from lms.djangoapps.certificates.signals import ( + _listen_for_enrollment_mode_change, + _listen_for_id_verification_status_changed, + listen_for_passing_grade +) +from lms.djangoapps.certificates.tests.factories import CertificateAllowlistFactory +from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory +from openedx.core.djangolib.testing.utils import skip_unless_lms + + +class TestCertificatePipelineStep(PipelineStep): + """ + Utility function used when getting steps for pipeline. + """ + + def run_filter(self, user, course_key, mode, status, grade, generation_mode): # pylint: disable=arguments-differ + """Pipeline steps that changes certificate mode from honor to no-id-professional.""" + if mode == 'honor': + return { + 'mode': 'no-id-professional', + } + return {} + + +class TestStopCertificateGenerationStep(PipelineStep): + """ + Utility function used when getting steps for pipeline. + """ + + def run_filter(self, user, course_key, mode, status, grade, generation_mode): # pylint: disable=arguments-differ + """Pipeline step that stops the certificate generation process.""" + raise CertificateCreationRequested.PreventCertificateCreation( + "You can't generate a certificate from this site." + ) + + +@mock.patch( + 'lms.djangoapps.certificates.generation_handler.has_html_certificates_enabled', mock.Mock(return_value=True), +) +@mock.patch('lms.djangoapps.certificates.generation_handler._is_passing_grade', mock.Mock(return_value=True)) +@skip_unless_lms +class CertificateFiltersTest(SharedModuleStoreTestCase): + """ + Tests for the Open edX Filters associated with the certificate generation process. + + This class guarantees that the following filters are triggered during the user's certificate generation: + + - CertificateCreationRequested + """ + + def setUp(self): # pylint: disable=arguments-differ + super().setUp() + self.course_run = CourseFactory() + self.user = UserFactory.create( + username="somestudent", + first_name="Student", + last_name="Person", + email="robot@robot.org", + is_active=True, + password="password", + ) + self.grade = CourseGradeFactory().read(self.user, self.course_run) + self.enrollment = CourseEnrollmentFactory( + user=self.user, + course_id=self.course_run.id, + is_active=True, + mode=CourseMode.HONOR, + ) + self.client.login(username=self.user.username, password="password") + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.creation.requested.v1": { + "pipeline": [ + "lms.djangoapps.certificates.tests.test_filters.TestCertificatePipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_certificate_creation_filter_executed(self): + """ + Test whether the student certificate filter is triggered before the user's + certificate creation process. + + Expected result: + - CertificateCreationRequested is triggered and executes TestCertificatePipelineStep. + - The certificate generates with no-id-professional mode instead of honor mode. + """ + cert_gen_task_created = generate_certificate_task( + self.user, self.course_run.id, generation_mode=CourseMode.HONOR, + ) + + certificate = GeneratedCertificate.objects.get( + user=self.user, + course_id=self.course_run.id, + ) + + self.assertTrue(cert_gen_task_created) + self.assertEqual(CourseMode.NO_ID_PROFESSIONAL_MODE, certificate.mode) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.creation.requested.v1": { + "pipeline": [ + "lms.djangoapps.certificates.tests.test_filters.TestStopCertificateGenerationStep", + ], + "fail_silently": False, + }, + }, + ) + def test_certificate_creation_filter_prevent_generation(self): + """ + Test prevent the user's certificate generation through a pipeline step. + + Expected result: + - CertificateCreationRequested is triggered and executes TestStopCertificateGenerationStep. + - The certificate is not generated. + """ + with self.assertRaises(CertificateGenerationNotAllowed): + generate_certificate_task( + self.user, self.course_run.id, generation_mode=CourseMode.HONOR, + ) + + self.assertFalse( + GeneratedCertificate.objects.filter( + user=self.user, course_id=self.course_run.id, mode=CourseMode.HONOR, + ) + ) + + @override_settings(OPEN_EDX_FILTERS_CONFIG={}) + def test_certificate_generation_without_filter_configuration(self): + """ + Test usual certificate process, without filter's intervention. + + Expected result: + - CertificateCreationRequested does not have any effect on the certificate generation process. + - The certificate generation process ends successfully. + """ + cert_gen_task_created = generate_certificate_task( + self.user, self.course_run.id, generation_mode=CourseMode.HONOR, + ) + + certificate = GeneratedCertificate.objects.get( + user=self.user, + course_id=self.course_run.id, + ) + + self.assertTrue(cert_gen_task_created) + self.assertEqual(CourseMode.HONOR, certificate.mode) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.creation.requested.v1": { + "pipeline": [ + "lms.djangoapps.certificates.tests.test_filters.TestStopCertificateGenerationStep", + ], + "fail_silently": False, + }, + }, + ) + def test_generate_allowlist_certificate_fail(self): + """ + Test stop certificate process by raising a filter exception when the user is in the + allow list. + + Expected result: + - CertificateCreationRequested is triggered and executes TestStopCertificateGenerationStep. + - The certificate is not generated. + """ + CertificateAllowlistFactory.create(course_id=self.course_run.id, user=self.user) + + certificate_generated = generate_allowlist_certificate_task(self.user, self.course_run.id) + + self.assertFalse(certificate_generated) + self.assertFalse( + GeneratedCertificate.objects.filter( + user=self.user, course_id=self.course_run.id, mode=CourseMode.HONOR, + ) + ) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.creation.requested.v1": { + "pipeline": [ + "lms.djangoapps.certificates.tests.test_filters.TestStopCertificateGenerationStep", + ], + "fail_silently": False, + }, + }, + ) + def test_generate_certificate_command(self): + """ + Test stop certificate process through the Django command by raising a filter exception. + + Expected result: + - CertificateCreationRequested is triggered and executes TestStopCertificateGenerationStep. + - The certificate is not generated. + """ + with self.assertLogs(level="ERROR"): + call_command("cert_generation", "--u", self.user.id, "--c", self.course_run.id) + + self.assertFalse( + GeneratedCertificate.objects.filter( + user=self.user, course_id=self.course_run.id, mode=CourseMode.HONOR, + ) + ) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.creation.requested.v1": { + "pipeline": [ + "lms.djangoapps.certificates.tests.test_filters.TestStopCertificateGenerationStep", + ], + "fail_silently": False, + }, + }, + ) + @mock.patch("lms.djangoapps.certificates.api.auto_certificate_generation_enabled", mock.Mock(return_value=True)) + def test_listen_for_passing_grade(self): + """ + Test stop automatic certificate generation process by raising a filters exception. + + Expected result: + - CertificateCreationRequested is triggered and executes TestStopCertificateGenerationStep. + - The certificate is not generated. + """ + signal_result = listen_for_passing_grade(None, self.user, self.course_run.id) + + self.assertFalse(signal_result) + self.assertFalse( + GeneratedCertificate.objects.filter( + user=self.user, course_id=self.course_run.id, mode=CourseMode.HONOR, + ) + ) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.creation.requested.v1": { + "pipeline": [ + "lms.djangoapps.certificates.tests.test_filters.TestStopCertificateGenerationStep", + ], + "fail_silently": False, + }, + }, + ) + @mock.patch( + 'lms.djangoapps.verify_student.services.IDVerificationService.user_status', + mock.Mock(return_value={"status": "approved"}) + ) + @mock.patch("lms.djangoapps.certificates.api.auto_certificate_generation_enabled", mock.Mock(return_value=True)) + def test_listen_for_id_verification_status_changed(self): + """ + Test stop certificate generation process after the verification status changed by raising a filters exception. + + Expected result: + - CertificateCreationRequested is triggered and executes TestStopCertificateGenerationStep. + - The certificate is not generated. + """ + _listen_for_id_verification_status_changed(None, self.user) + + self.assertFalse( + GeneratedCertificate.objects.filter( + user=self.user, course_id=self.course_run.id, mode=CourseMode.HONOR, + ) + ) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.creation.requested.v1": { + "pipeline": [ + "lms.djangoapps.certificates.tests.test_filters.TestStopCertificateGenerationStep", + ], + "fail_silently": False, + }, + }, + ) + def test_listen_for_enrollment_mode_change(self): + """ + Test stop automatic certificate generation process by raising a filters exception. + + Expected result: + - CertificateCreationRequested is triggered and executes TestStopCertificateGenerationStep. + - The certificate is not generated. + """ + signal_result = _listen_for_enrollment_mode_change(None, self.user, self.course_run.id, CourseMode.HONOR) + + self.assertFalse(signal_result) + self.assertFalse( + GeneratedCertificate.objects.filter( + user=self.user, course_id=self.course_run.id, mode=CourseMode.HONOR, + ) + ) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.creation.requested.v1": { + "pipeline": [ + "lms.djangoapps.certificates.tests.test_filters.TestStopCertificateGenerationStep", + ], + "fail_silently": False, + }, + }, + ) + @mock.patch( + "lms.djangoapps.certificates.generation_handler._can_generate_regular_certificate", + mock.Mock(return_value=True), + ) + def test_generate_cert_support_view(self): + """ + Test stop automatic certificate generation process by raising a filters exception. + + Expected result: + - CertificateCreationRequested is triggered and executes TestStopCertificateGenerationStep. + - The view returns HTTP_400_BAD_REQUEST. + """ + SupportStaffRole().add_users(self.user) + url = reverse( + "certificates:regenerate_certificate_for_user", + ) + body = { + "course_key": str(self.course_run.id), + "username": self.user.username, + } + + response = self.client.post(url, body) + + self.assertEqual(status_code.HTTP_400_BAD_REQUEST, response.status_code) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.creation.requested.v1": { + "pipeline": [ + "lms.djangoapps.certificates.tests.test_filters.TestStopCertificateGenerationStep", + ], + "fail_silently": False, + }, + }, + ) + def test_generate_cert_progress_view(self): + """ + Test stop certificate generation from the progress view by raising a filters exception. + + Expected result: + - CertificateCreationRequested is triggered and executes TestStopCertificateGenerationStep. + - The view returns HTTP_400_BAD_REQUEST. + """ + url = reverse("generate_user_cert", kwargs={"course_id": str(self.course_run.id)}) + + response = self.client.post(url) + + self.assertContains( + response, + "You can't generate a certificate from this site.", + status_code=status_code.HTTP_400_BAD_REQUEST, + ) diff --git a/lms/djangoapps/certificates/views/support.py b/lms/djangoapps/certificates/views/support.py index 67a75f3fa5cb..7aadf3091952 100644 --- a/lms/djangoapps/certificates/views/support.py +++ b/lms/djangoapps/certificates/views/support.py @@ -22,6 +22,7 @@ from common.djangoapps.student.models import CourseEnrollment, User from common.djangoapps.util.json_request import JsonResponse from lms.djangoapps.certificates.api import generate_certificate_task, get_certificates_for_user +from lms.djangoapps.certificates.generation_handler import CertificateGenerationNotAllowed from lms.djangoapps.certificates.permissions import GENERATE_ALL_CERTIFICATES, VIEW_ALL_CERTIFICATES from lms.djangoapps.instructor_task.api import generate_certificates_for_students from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none @@ -202,6 +203,13 @@ def regenerate_certificate_for_user(request): # Attempt to regenerate certificates try: generate_certificate_task(user, course_key) + except CertificateGenerationNotAllowed as e: + log.exception( + "Certificate generation not allowed for user %s in course %s", + str(user), + course_key, + ) + return HttpResponseBadRequest(str(e)) except: # pylint: disable=bare-except # We are pessimistic about the kinds of errors that might get thrown by the # certificates API. This may be overkill, but we're logging everything so we can diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 27dadbf05f89..6ab37358b6a5 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -61,6 +61,7 @@ from lms.djangoapps.ccx.custom_exception import CCXLocatorValidationException from lms.djangoapps.certificates import api as certs_api from lms.djangoapps.certificates.data import CertificateStatuses +from lms.djangoapps.certificates.generation_handler import CertificateGenerationNotAllowed from lms.djangoapps.commerce.utils import EcommerceService from lms.djangoapps.course_goals.models import UserActivity from lms.djangoapps.course_home_api.toggles import course_home_mfe_progress_tab_is_active @@ -1531,7 +1532,16 @@ def generate_user_cert(request, course_id): return HttpResponseBadRequest(_("Course is not valid")) log.info(f'Attempt will be made to generate a course certificate for {student.id} : {course_key}.') - certs_api.generate_certificate_task(student, course_key, 'self') + + try: + certs_api.generate_certificate_task(student, course_key, 'self') + except CertificateGenerationNotAllowed as e: + log.exception( + "Certificate generation not allowed for user %s in course %s", + str(student), + course_key, + ) + return HttpResponseBadRequest(str(e)) if not is_course_passed(student, course): log.info("User %s has not passed the course: %s", student.username, course_id) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index c66594583531..c902dffa2330 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -733,7 +733,7 @@ openedx-calc==3.0.1 # via -r requirements/edx/base.in openedx-events==0.8.2 # via -r requirements/edx/base.in -openedx-filters==0.6.1 +openedx-filters==0.6.2 # via -r requirements/edx/base.in ora2==4.2.0 # via -r requirements/edx/base.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index eb3e3f4a0e58..cd31a04488b2 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -962,7 +962,7 @@ openedx-calc==3.0.1 # via -r requirements/edx/testing.txt openedx-events==0.8.2 # via -r requirements/edx/testing.txt -openedx-filters==0.6.1 +openedx-filters==0.6.2 # via -r requirements/edx/testing.txt ora2==4.2.0 # via -r requirements/edx/testing.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index e3ecb0df1f67..98fba24f36e7 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -912,7 +912,7 @@ openedx-calc==3.0.1 # via -r requirements/edx/base.txt openedx-events==0.8.2 # via -r requirements/edx/base.txt -openedx-filters==0.6.1 +openedx-filters==0.6.2 # via -r requirements/edx/base.txt ora2==4.2.0 # via -r requirements/edx/base.txt