From 8d386d7b2414f38752481d54598cf97839f4136a Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Fri, 4 Oct 2024 16:45:47 +0500 Subject: [PATCH 1/2] feat!: upgrading api to DRF. (#35536) --- lms/djangoapps/instructor/views/api.py | 37 ++++++++++++--------- lms/djangoapps/instructor/views/api_urls.py | 2 +- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 94184f6d93..98df3e48bc 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -3239,26 +3239,31 @@ class MarkStudentCanSkipEntranceExam(APIView): return JsonResponse(response_payload) -@transaction.non_atomic_requests -@ensure_csrf_cookie -@cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_course_permission(permissions.START_CERTIFICATE_GENERATION) -@require_POST -@common_exceptions_400 -def start_certificate_generation(request, course_id): +@method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch') +@method_decorator(transaction.non_atomic_requests, name='dispatch') +class StartCertificateGeneration(DeveloperErrorViewMixin, APIView): """ Start generating certificates for all students enrolled in given course. """ - course_key = CourseKey.from_string(course_id) - task = task_api.generate_certificates_for_students(request, course_key) - message = _('Certificate generation task for all students of this course has been started. ' - 'You can view the status of the generation task in the "Pending Tasks" section.') - response_payload = { - 'message': message, - 'task_id': task.task_id - } + permission_classes = (IsAuthenticated, permissions.InstructorPermission) + permission_name = permissions.START_CERTIFICATE_GENERATION - return JsonResponse(response_payload) + @method_decorator(ensure_csrf_cookie) + @method_decorator(transaction.non_atomic_requests) + def post(self, request, course_id): + """ + Generating certificates for all students enrolled in given course. + """ + course_key = CourseKey.from_string(course_id) + task = task_api.generate_certificates_for_students(request, course_key) + message = _('Certificate generation task for all students of this course has been started. ' + 'You can view the status of the generation task in the "Pending Tasks" section.') + response_payload = { + 'message': message, + 'task_id': task.task_id + } + + return JsonResponse(response_payload) @transaction.non_atomic_requests diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index 5976411a97..e7309d7f5f 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -82,7 +82,7 @@ urlpatterns = [ # Certificates path('enable_certificate_generation', api.enable_certificate_generation, name='enable_certificate_generation'), - path('start_certificate_generation', api.start_certificate_generation, name='start_certificate_generation'), + path('start_certificate_generation', api.StartCertificateGeneration.as_view(), name='start_certificate_generation'), path('start_certificate_regeneration', api.start_certificate_regeneration, name='start_certificate_regeneration'), path('certificate_exception_view/', api.certificate_exception_view, name='certificate_exception_view'), re_path(r'^generate_certificate_exceptions/(?P[^/]*)', api.generate_certificate_exceptions, From c34ccffc7fd98e83a87300189683e6b23b56a1b0 Mon Sep 17 00:00:00 2001 From: Zachary Hancock Date: Fri, 4 Oct 2024 12:58:15 -0400 Subject: [PATCH 2/2] feat: rework idv cert trigger (#35580) * feat: rework idv cert trigger * feat: separate PhotoVerification events --- .../tests/test_pipeline_integration.py | 2 +- .../docs/diagrams/certificate_generation.dsl | 2 + lms/djangoapps/certificates/signals.py | 29 ++++++++++--- .../certificates/tests/test_filters.py | 6 +-- ...sso_verifications_for_old_account_links.py | 2 +- lms/djangoapps/verify_student/models.py | 43 +++++-------------- openedx/core/djangoapps/signals/signals.py | 11 +++++ 7 files changed, 52 insertions(+), 43 deletions(-) diff --git a/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py b/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py index 4bfc710fe9..c19b0b8d96 100644 --- a/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py +++ b/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py @@ -583,7 +583,7 @@ class SetIDVerificationStatusTestCase(TestCase): """ Verification signal is sent upon approval. """ - with mock.patch('openedx_events.learning.signals.IDV_ATTEMPT_APPROVED.send_event') as mock_signal: + with mock.patch('openedx.core.djangoapps.signals.signals.LEARNER_SSO_VERIFIED.send_robust') as mock_signal: # Begin the pipeline. pipeline.set_id_verification_status( auth_entry=pipeline.AUTH_ENTRY_LOGIN, diff --git a/lms/djangoapps/certificates/docs/diagrams/certificate_generation.dsl b/lms/djangoapps/certificates/docs/diagrams/certificate_generation.dsl index d7ca8fd9a4..e5b66bf3b2 100644 --- a/lms/djangoapps/certificates/docs/diagrams/certificate_generation.dsl +++ b/lms/djangoapps/certificates/docs/diagrams/certificate_generation.dsl @@ -32,6 +32,8 @@ workspace { grades_app -> signal_handlers "Emits COURSE_GRADE_NOW_PASSED signal" verify_student_app -> signal_handlers "Emits IDV_ATTEMPT_APPROVED signal" + verify_student_app -> signal_handlers "Emits LEARNER_SSO_VERIFIED signal" + verify_student_app -> signal_handlers "Emits PHOTO_VERIFICATION_APPROVED signal" student_app -> signal_handlers "Emits ENROLLMENT_TRACK_UPDATED signal" allowlist -> signal_handlers "Emits APPEND_CERTIFICATE_ALLOWLIST signal" signal_handlers -> generation_handler "Invokes generate_allowlist_certificate()" diff --git a/lms/djangoapps/certificates/signals.py b/lms/djangoapps/certificates/signals.py index 53055bf9c8..39042ff341 100644 --- a/lms/djangoapps/certificates/signals.py +++ b/lms/djangoapps/certificates/signals.py @@ -32,6 +32,8 @@ from openedx.core.djangoapps.content.course_overviews.signals import COURSE_PACI from openedx.core.djangoapps.signals.signals import ( COURSE_GRADE_NOW_FAILED, COURSE_GRADE_NOW_PASSED, + LEARNER_SSO_VERIFIED, + PHOTO_VERIFICATION_APPROVED, ) from openedx_events.learning.signals import EXAM_ATTEMPT_REJECTED, IDV_ATTEMPT_APPROVED @@ -117,17 +119,13 @@ def _listen_for_failing_grade(sender, user, course_id, grade, **kwargs): # pyli log.info(f'Certificate marked not passing for {user.id} : {course_id} via failing grade') -@receiver(IDV_ATTEMPT_APPROVED, dispatch_uid="learner_track_changed") -def _listen_for_id_verification_status_changed(sender, signal, **kwargs): # pylint: disable=unused-argument +def _handle_id_verification_approved(user): """ - Listen for a signal indicating that the user's id verification status has changed. + Generate a certificate for the user if they are now verified """ if not auto_certificate_generation_enabled(): return - event_data = kwargs.get('idv_attempt') - user = User.objects.get(id=event_data.user.id) - user_enrollments = CourseEnrollment.enrollments_for_user(user=user) expected_verification_status = IDVerificationService.user_status(user) expected_verification_status = expected_verification_status['status'] @@ -145,6 +143,25 @@ def _listen_for_id_verification_status_changed(sender, signal, **kwargs): # pyl ) +@receiver(LEARNER_SSO_VERIFIED, dispatch_uid="sso_learner_verified") +@receiver(PHOTO_VERIFICATION_APPROVED, dispatch_uid="photo_verification_approved") +def _listen_for_sso_verification_approved(sender, user, **kwargs): # pylint: disable=unused-argument + """ + Listen for a signal on SSOVerification indicating that the user has been verified. + """ + _handle_id_verification_approved(user) + + +@receiver(IDV_ATTEMPT_APPROVED, dispatch_uid="openedx_idv_attempt_approved") +def _listen_for_id_verification_approved_event(sender, signal, **kwargs): # pylint: disable=unused-argument + """ + Listen for an openedx event indicating that the user's id verification status has changed. + """ + event_data = kwargs.get('idv_attempt') + user = User.objects.get(id=event_data.user.id) + _handle_id_verification_approved(user) + + @receiver(ENROLLMENT_TRACK_UPDATED) def _listen_for_enrollment_mode_change(sender, user, course_key, mode, **kwargs): # pylint: disable=unused-argument """ diff --git a/lms/djangoapps/certificates/tests/test_filters.py b/lms/djangoapps/certificates/tests/test_filters.py index 781b77461f..a131a1d525 100644 --- a/lms/djangoapps/certificates/tests/test_filters.py +++ b/lms/djangoapps/certificates/tests/test_filters.py @@ -23,7 +23,7 @@ from lms.djangoapps.certificates.generation_handler import ( from lms.djangoapps.certificates.models import GeneratedCertificate from lms.djangoapps.certificates.signals import ( _listen_for_enrollment_mode_change, - _listen_for_id_verification_status_changed, + _handle_id_verification_approved, listen_for_passing_grade ) from lms.djangoapps.certificates.tests.factories import CertificateAllowlistFactory @@ -272,7 +272,7 @@ class CertificateFiltersTest(SharedModuleStoreTestCase): 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): + def test_handle_id_verification_approved(self): """ Test stop certificate generation process after the verification status changed by raising a filters exception. @@ -280,7 +280,7 @@ class CertificateFiltersTest(SharedModuleStoreTestCase): - CertificateCreationRequested is triggered and executes TestStopCertificateGenerationStep. - The certificate is not generated. """ - _listen_for_id_verification_status_changed(None, self.user) + _handle_id_verification_approved(self.user) self.assertFalse( GeneratedCertificate.objects.filter( diff --git a/lms/djangoapps/verify_student/management/commands/tests/test_backfill_sso_verifications_for_old_account_links.py b/lms/djangoapps/verify_student/management/commands/tests/test_backfill_sso_verifications_for_old_account_links.py index 891ff9fda5..d9f356758d 100644 --- a/lms/djangoapps/verify_student/management/commands/tests/test_backfill_sso_verifications_for_old_account_links.py +++ b/lms/djangoapps/verify_student/management/commands/tests/test_backfill_sso_verifications_for_old_account_links.py @@ -54,7 +54,7 @@ class TestBackfillSSOVerificationsCommand(TestCase): #self.assertNumQueries(100) def test_signal_called(self): - with patch('openedx_events.learning.signals.IDV_ATTEMPT_APPROVED.send_event') as mock_signal: + with patch('openedx.core.djangoapps.signals.signals.LEARNER_SSO_VERIFIED.send_robust') as mock_signal: call_command('backfill_sso_verifications_for_old_account_links', '--provider-slug', self.provider.provider_id) # lint-amnesty, pylint: disable=line-too-long assert mock_signal.call_count == 1 diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index 9a0ac36964..ae00ca8a1c 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -44,9 +44,8 @@ from lms.djangoapps.verify_student.ssencrypt import ( rsa_decrypt, rsa_encrypt ) +from openedx.core.djangoapps.signals.signals import LEARNER_SSO_VERIFIED, PHOTO_VERIFICATION_APPROVED from openedx.core.storage import get_storage -from openedx_events.learning.signals import IDV_ATTEMPT_APPROVED -from openedx_events.learning.data import UserData, VerificationAttemptData from .utils import auto_verify_for_testing_enabled, earliest_allowed_verification_date, submit_request_to_ss @@ -251,23 +250,13 @@ class SSOVerification(IDVerificationAttempt): user_id=self.user, reviewer=approved_by )) - # Emit event to find and generate eligible certificates - verification_data = VerificationAttemptData( - attempt_id=self.id, - user=UserData( - pii=None, - id=self.user.id, - is_active=self.user.is_active, - ), - status=self.status, - name=self.name, - expiration_date=self.expiration_datetime, - ) - IDV_ATTEMPT_APPROVED.send_event( - idv_attempt=verification_data, + # Emit signal to find and generate eligible certificates + LEARNER_SSO_VERIFIED.send_robust( + sender=PhotoVerification, + user=self.user, ) - message = 'IDV_ATTEMPT_APPROVED signal fired for {user} from SSOVerification' + message = 'LEARNER_SSO_VERIFIED signal fired for {user} from SSOVerification' log.info(message.format(user=self.user.username)) @@ -465,23 +454,13 @@ class PhotoVerification(IDVerificationAttempt): ) self.save() - # Emit event to find and generate eligible certificates - verification_data = VerificationAttemptData( - attempt_id=self.id, - user=UserData( - pii=None, - id=self.user.id, - is_active=self.user.is_active, - ), - status=self.status, - name=self.name, - expiration_date=self.expiration_datetime, - ) - IDV_ATTEMPT_APPROVED.send_event( - idv_attempt=verification_data, + # Emit signal to find and generate eligible certificates + PHOTO_VERIFICATION_APPROVED.send_robust( + sender=PhotoVerification, + user=self.user, ) - message = 'IDV_ATTEMPT_APPROVED signal fired for {user} from PhotoVerification' + message = 'PHOTO_VERIFICATION_APPROVED signal fired for {user} from PhotoVerification' log.info(message.format(user=self.user.username)) @status_before_must_be("ready", "must_retry") diff --git a/openedx/core/djangoapps/signals/signals.py b/openedx/core/djangoapps/signals/signals.py index 495389152f..24624464ab 100644 --- a/openedx/core/djangoapps/signals/signals.py +++ b/openedx/core/djangoapps/signals/signals.py @@ -36,5 +36,16 @@ COURSE_GRADE_NOW_PASSED = Signal() # ] COURSE_GRADE_NOW_FAILED = Signal() +# Signal that indicates that a user has become verified via SSO for certificate purposes +# providing_args=['user'] +LEARNER_SSO_VERIFIED = Signal() + +# Signal that indicates a user has been verified via verify_studnet.PhotoVerification for certificate purposes +# Please note that this signal and the corresponding PhotoVerification model are planned for deprecation. +# Future implementations of IDV will use the verify_student.VerificationAttempt model and corresponding +# openedx events. +# DEPR: https://github.com/openedx/edx-platform/issues/35128 +PHOTO_VERIFICATION_APPROVED = Signal() + # providing_args=['user'] USER_ACCOUNT_ACTIVATED = Signal() # Signal indicating email verification