From c34ccffc7fd98e83a87300189683e6b23b56a1b0 Mon Sep 17 00:00:00 2001 From: Zachary Hancock Date: Fri, 4 Oct 2024 12:58:15 -0400 Subject: [PATCH] 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 4bfc710fe901..c19b0b8d96aa 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 @@ def test_verification_signal(self): """ 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 d7ca8fd9a400..e5b66bf3b2ab 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 53055bf9c86e..39042ff34164 100644 --- a/lms/djangoapps/certificates/signals.py +++ b/lms/djangoapps/certificates/signals.py @@ -32,6 +32,8 @@ 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 781b77461f41..a131a1d52582 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.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 @@ def test_listen_for_passing_grade(self): 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 @@ def test_listen_for_id_verification_status_changed(self): - 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 891ff9fda5d8..d9f356758dd8 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 @@ def test_performance(self): #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 9a0ac369640a..ae00ca8a1c57 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -44,9 +44,8 @@ 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 @@ def send_approval_signal(self, approved_by='None'): 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 @@ def approve(self, user_id=None, service=""): ) 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 495389152f7a..24624464ab91 100644 --- a/openedx/core/djangoapps/signals/signals.py +++ b/openedx/core/djangoapps/signals/signals.py @@ -36,5 +36,16 @@ # ] 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