feat: add idv events to api (#35468)

* feat: add idv events to api - moved what was in signals.py to a handlers.py (which is what their file should have been called) * chore: quality * fix: rename test file + imports * fix: change handler reverse url in other tests * fix: refactor signals and handlers pattern - following OEP-49 pattern for signals directory - user removed as param for update function - event now emitted after save * fix: unpin edx-name-affirmation * chore: add init to signals dir * fix: compile requirements * chore: quality * chore: fix some imports * chore: quality * test: added signal emissions to test_api * chore: lint
2024-09-17 15:59:33 -04:00
parent 5927be7e0e
commit 575e240961
14 changed files with 227 additions and 44 deletions
--- a/lms/djangoapps/verify_student/api.py
+++ b/lms/djangoapps/verify_student/api.py
@@ -13,6 +13,12 @@ from typing import Optional
 from lms.djangoapps.verify_student.emails import send_verification_approved_email
 from lms.djangoapps.verify_student.exceptions import VerificationAttemptInvalidStatus
 from lms.djangoapps.verify_student.models import VerificationAttempt
+from lms.djangoapps.verify_student.signals.signals import (
+    emit_idv_attempt_approved_event,
+    emit_idv_attempt_created_event,
+    emit_idv_attempt_denied_event,
+    emit_idv_attempt_pending_event,
+)
 from lms.djangoapps.verify_student.statuses import VerificationAttemptStatus
 from lms.djangoapps.verify_student.tasks import send_verification_status_email

@@ -70,6 +76,14 @@ def create_verification_attempt(user: User, name: str, status: str, expiration_d
        expiration_datetime=expiration_datetime,
    )

+    emit_idv_attempt_created_event(
+        attempt_id=verification_attempt.id,
+        user=user,
+        status=status,
+        name=name,
+        expiration_date=expiration_datetime,
+    )
+
    return verification_attempt.id


@@ -77,7 +91,7 @@ def update_verification_attempt(
    attempt_id: int,
    name: Optional[str] = None,
    status: Optional[str] = None,
-    expiration_datetime: Optional[datetime] = None
+    expiration_datetime: Optional[datetime] = None,
 ):
    """
    Update a verification attempt.
@@ -125,3 +139,29 @@ def update_verification_attempt(
    attempt.expiration_datetime = expiration_datetime

    attempt.save()
+
+    user = attempt.user
+    if status == VerificationAttemptStatus.PENDING:
+        emit_idv_attempt_pending_event(
+            attempt_id=attempt_id,
+            user=user,
+            status=status,
+            name=name,
+            expiration_date=expiration_datetime,
+        )
+    elif status == VerificationAttemptStatus.APPROVED:
+        emit_idv_attempt_approved_event(
+            attempt_id=attempt_id,
+            user=user,
+            status=status,
+            name=name,
+            expiration_date=expiration_datetime,
+        )
+    elif status == VerificationAttemptStatus.DENIED:
+        emit_idv_attempt_denied_event(
+            attempt_id=attempt_id,
+            user=user,
+            status=status,
+            name=name,
+            expiration_date=expiration_datetime,
+        )
--- a/lms/djangoapps/verify_student/apps.py
+++ b/lms/djangoapps/verify_student/apps.py
@@ -17,5 +17,5 @@ class VerifyStudentConfig(AppConfig):
        """
        Connect signal handlers.
        """
-        from lms.djangoapps.verify_student import signals  # pylint: disable=unused-import
+        from lms.djangoapps.verify_student.signals import signals  # pylint: disable=unused-import
        from lms.djangoapps.verify_student import tasks    # pylint: disable=unused-import
--- a/lms/djangoapps/verify_student/management/commands/tests/test_retry_failed_photo_verifications.py
+++ b/lms/djangoapps/verify_student/management/commands/tests/test_retry_failed_photo_verifications.py
@@ -121,7 +121,7 @@ class TestRetryFailedPhotoVerificationsBetweenDates(MockS3Boto3Mixin, TestVerifi
        for _ in range(num_attempts):
            self.create_upload_and_submit_attempt_for_user()

-    @patch('lms.djangoapps.verify_student.signals.idv_update_signal.send')
+    @patch('lms.djangoapps.verify_student.signals.signals.idv_update_signal.send')
    def test_resubmit_in_date_range(self, send_idv_update_mock):
        call_command('retry_failed_photo_verifications',
                     status="submitted",
--- a/lms/djangoapps/verify_student/management/commands/tests/test_trigger_softwaresecurephotoverifications_post_save_signal.py
+++ b/lms/djangoapps/verify_student/management/commands/tests/test_trigger_softwaresecurephotoverifications_post_save_signal.py
@@ -38,7 +38,7 @@ class TestTriggerSoftwareSecurePhotoVerificationsPostSaveSignal(MockS3Boto3Mixin
        for _ in range(num_attempts):
            self.create_and_submit_attempt_for_user()

-    @patch('lms.djangoapps.verify_student.signals.idv_update_signal.send')
+    @patch('lms.djangoapps.verify_student.signals.signals.idv_update_signal.send')
    def test_command(self, send_idv_update_mock):
        call_command('trigger_softwaresecurephotoverifications_post_save_signal', start_date_time='2021-10-31 06:00:00')

--- a/lms/djangoapps/verify_student/signals/init.py
+++ b/lms/djangoapps/verify_student/signals/init.py
--- a/lms/djangoapps/verify_student/signals/handlers.py
+++ b/lms/djangoapps/verify_student/signals/handlers.py
@@ -5,23 +5,23 @@ import logging

 from django.core.exceptions import ObjectDoesNotExist
 from django.db.models.signals import post_save
-from django.dispatch import Signal
 from django.dispatch.dispatcher import receiver
 from xmodule.modulestore.django import SignalHandler, modulestore

 from common.djangoapps.student.models_api import get_name, get_pending_name_change
+from lms.djangoapps.verify_student.apps import VerifyStudentConfig  # pylint: disable=unused-import
+from lms.djangoapps.verify_student.signals.signals import idv_update_signal
 from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_LMS_CRITICAL, USER_RETIRE_LMS_MISC

-from .models import SoftwareSecurePhotoVerification, VerificationDeadline, VerificationAttempt
+from lms.djangoapps.verify_student.models import (
+    SoftwareSecurePhotoVerification,
+    VerificationDeadline,
+    VerificationAttempt
+)

 log = logging.getLogger(__name__)


-# Signal for emitting IDV submission and review updates
-# providing_args = ["attempt_id", "user_id", "status", "full_name", "profile_name"]
-idv_update_signal = Signal()
-
-
@receiver(SignalHandler.course_published)
 def _listen_for_course_publish(sender, course_key, **kwargs):  # pylint: disable=unused-argument
    """
--- a/lms/djangoapps/verify_student/signals/signals.py
+++ b/lms/djangoapps/verify_student/signals/signals.py
@@ -0,0 +1,109 @@
+"""
+Signal definitions and functions to send those signals for the verify_student application.
+"""
+
+from django.dispatch import Signal
+
+from openedx_events.learning.data import UserData, UserPersonalData, VerificationAttemptData
+from openedx_events.learning.signals import (
+    IDV_ATTEMPT_CREATED,
+    IDV_ATTEMPT_PENDING,
+    IDV_ATTEMPT_APPROVED,
+    IDV_ATTEMPT_DENIED,
+)
+
+# Signal for emitting IDV submission and review updates
+# providing_args = ["attempt_id", "user_id", "status", "full_name", "profile_name"]
+idv_update_signal = Signal()
+
+
+def _create_user_data(user):
+    """
+    Helper function to create a UserData object.
+    """
+    user_data = UserData(
+        id=user.id,
+        is_active=user.is_active,
+        pii=UserPersonalData(
+            username=user.username,
+            email=user.email,
+            name=user.get_full_name()
+        )
+    )
+
+    return user_data
+
+
+def emit_idv_attempt_created_event(attempt_id, user, status, name, expiration_date):
+    """
+    Emit the IDV_ATTEMPT_CREATED Open edX event.
+    """
+    user_data = _create_user_data(user)
+
+    # .. event_implemented_name: IDV_ATTEMPT_CREATED
+    IDV_ATTEMPT_CREATED.send_event(
+        idv_attempt=VerificationAttemptData(
+            attempt_id=attempt_id,
+            user=user_data,
+            status=status,
+            name=name,
+            expiration_date=expiration_date,
+        )
+    )
+    return user_data
+
+
+def emit_idv_attempt_pending_event(attempt_id, user, status, name, expiration_date):
+    """
+    Emit the IDV_ATTEMPT_PENDING Open edX event.
+    """
+    user_data = _create_user_data(user)
+
+    # .. event_implemented_name: IDV_ATTEMPT_PENDING
+    IDV_ATTEMPT_PENDING.send_event(
+        idv_attempt=VerificationAttemptData(
+            attempt_id=attempt_id,
+            user=user_data,
+            status=status,
+            name=name,
+            expiration_date=expiration_date,
+        )
+    )
+    return user_data
+
+
+def emit_idv_attempt_approved_event(attempt_id, user, status, name, expiration_date):
+    """
+    Emit the IDV_ATTEMPT_APPROVED Open edX event.
+    """
+    user_data = _create_user_data(user)
+
+    # .. event_implemented_name: IDV_ATTEMPT_APPROVED
+    IDV_ATTEMPT_APPROVED.send_event(
+        idv_attempt=VerificationAttemptData(
+            attempt_id=attempt_id,
+            user=user_data,
+            status=status,
+            name=name,
+            expiration_date=expiration_date,
+        )
+    )
+    return user_data
+
+
+def emit_idv_attempt_denied_event(attempt_id, user, status, name, expiration_date):
+    """
+    Emit the IDV_ATTEMPT_DENIED Open edX event.
+    """
+    user_data = _create_user_data(user)
+
+    # .. event_implemented_name: IDV_ATTEMPT_DENIED
+    IDV_ATTEMPT_DENIED.send_event(
+        idv_attempt=VerificationAttemptData(
+            attempt_id=attempt_id,
+            user=user_data,
+            status=status,
+            name=name,
+            expiration_date=expiration_date,
+        )
+    )
--- a/lms/djangoapps/verify_student/tests/test_api.py
+++ b/lms/djangoapps/verify_student/tests/test_api.py
@@ -69,7 +69,8 @@ class CreateVerificationAttempt(TestCase):
        )
        self.attempt.save()

-    def test_create_verification_attempt(self):
+    @patch('lms.djangoapps.verify_student.api.emit_idv_attempt_created_event')
+    def test_create_verification_attempt(self, mock_created_event):
        expected_id = 2
        self.assertEqual(
            create_verification_attempt(
@@ -86,6 +87,13 @@ class CreateVerificationAttempt(TestCase):
        self.assertEqual(verification_attempt.name, 'Tester McTest')
        self.assertEqual(verification_attempt.status, VerificationAttemptStatus.CREATED)
        self.assertEqual(verification_attempt.expiration_datetime, datetime(2024, 12, 31, tzinfo=timezone.utc))
+        mock_created_event.assert_called_with(
+            attempt_id=verification_attempt.id,
+            user=self.user,
+            status=VerificationAttemptStatus.CREATED,
+            name='Tester McTest',
+            expiration_date=datetime(2024, 12, 31, tzinfo=timezone.utc),
+        )

    def test_create_verification_attempt_no_expiration_datetime(self):
        expected_id = 2
@@ -129,7 +137,18 @@ class UpdateVerificationAttempt(TestCase):
        ('Tester McTest3', VerificationAttemptStatus.DENIED, datetime(2026, 12, 31, tzinfo=timezone.utc)),
    )
    @ddt.unpack
-    def test_update_verification_attempt(self, name, status, expiration_datetime):
+    @patch('lms.djangoapps.verify_student.api.emit_idv_attempt_pending_event')
+    @patch('lms.djangoapps.verify_student.api.emit_idv_attempt_approved_event')
+    @patch('lms.djangoapps.verify_student.api.emit_idv_attempt_denied_event')
+    def test_update_verification_attempt(
+        self,
+        name,
+        status,
+        expiration_datetime,
+        mock_denied_event,
+        mock_approved_event,
+        mock_pending_event,
+    ):
        update_verification_attempt(
            attempt_id=self.attempt.id,
            name=name,
@@ -145,6 +164,31 @@ class UpdateVerificationAttempt(TestCase):
        self.assertEqual(verification_attempt.status, status)
        self.assertEqual(verification_attempt.expiration_datetime, expiration_datetime)

+        if status == VerificationAttemptStatus.PENDING:
+            mock_pending_event.assert_called_with(
+                attempt_id=verification_attempt.id,
+                user=self.user,
+                status=status,
+                name=name,
+                expiration_date=expiration_datetime,
+            )
+        elif status == VerificationAttemptStatus.APPROVED:
+            mock_approved_event.assert_called_with(
+                attempt_id=verification_attempt.id,
+                user=self.user,
+                status=status,
+                name=name,
+                expiration_date=expiration_datetime,
+            )
+        elif status == VerificationAttemptStatus.DENIED:
+            mock_denied_event.assert_called_with(
+                attempt_id=verification_attempt.id,
+                user=self.user,
+                status=status,
+                name=name,
+                expiration_date=expiration_datetime,
+            )
+
    def test_update_verification_attempt_none_values(self):
        update_verification_attempt(
            attempt_id=self.attempt.id,
@@ -166,6 +210,7 @@ class UpdateVerificationAttempt(TestCase):
            VerificationAttempt.DoesNotExist,
            update_verification_attempt,
            attempt_id=999999,
+            name=None,
            status=VerificationAttemptStatus.APPROVED,
        )

--- a/lms/djangoapps/verify_student/tests/test_handlers.py
+++ b/lms/djangoapps/verify_student/tests/test_handlers.py
@@ -15,7 +15,7 @@ from lms.djangoapps.verify_student.models import (
    VerificationDeadline,
    VerificationAttempt
 )
-from lms.djangoapps.verify_student.signals import (
+from lms.djangoapps.verify_student.signals.handlers import (
    _listen_for_course_publish,
    _listen_for_lms_retire,
    _listen_for_lms_retire_verification_attempts
@@ -29,9 +29,9 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase  # lint-a
 from xmodule.modulestore.tests.factories import CourseFactory  # lint-amnesty, pylint: disable=wrong-import-order


-class VerificationDeadlineSignalTest(ModuleStoreTestCase):
+class VerificationDeadlineHandlerTest(ModuleStoreTestCase):
    """
-    Tests for the VerificationDeadline signal
+    Tests for the VerificationDeadline handler
    """

    def setUp(self):
@@ -41,13 +41,13 @@ class VerificationDeadlineSignalTest(ModuleStoreTestCase):
        VerificationDeadline.objects.all().delete()

    def test_no_deadline(self):
-        """ Verify the signal sets deadline to course end when no deadline exists."""
+        """ Verify the handler sets deadline to course end when no deadline exists."""
        _listen_for_course_publish('store', self.course.id)

        assert VerificationDeadline.deadline_for_course(self.course.id) == self.course.end

    def test_deadline(self):
-        """ Verify deadline is set to course end date by signal when changed. """
+        """ Verify deadline is set to course end date by handler when changed. """
        deadline = now() - timedelta(days=7)
        VerificationDeadline.set_deadline(self.course.id, deadline)

@@ -55,7 +55,7 @@ class VerificationDeadlineSignalTest(ModuleStoreTestCase):
        assert VerificationDeadline.deadline_for_course(self.course.id) == self.course.end

    def test_deadline_explicit(self):
-        """ Verify deadline is unchanged by signal when explicitly set. """
+        """ Verify deadline is unchanged by handler when explicitly set. """
        deadline = now() - timedelta(days=7)
        VerificationDeadline.set_deadline(self.course.id, deadline, is_explicit=True)

@@ -66,9 +66,9 @@ class VerificationDeadlineSignalTest(ModuleStoreTestCase):
        assert actual_deadline == deadline


-class RetirementSignalTest(ModuleStoreTestCase):
+class RetirementHandlerTest(ModuleStoreTestCase):
    """
-    Tests for the VerificationDeadline signal
+    Tests for the VerificationDeadline handler
    """

    def _create_entry(self):
@@ -119,8 +119,8 @@ class RetirementSignalTest(ModuleStoreTestCase):

 class PostSavePhotoVerificationTest(ModuleStoreTestCase):
    """
-    Tests for the post_save signal on the SoftwareSecurePhotoVerification model.
-    This receiver should emit another signal that contains limited data about
+    Tests for the post_save handler on the SoftwareSecurePhotoVerification model.
+    This receiver should emit another handler that contains limited data about
    the verification attempt that was updated.
    """

@@ -132,7 +132,7 @@ class PostSavePhotoVerificationTest(ModuleStoreTestCase):
        self.photo_id_image_url = 'https://test.photo'
        self.photo_id_key = 'test+key'

-    @patch('lms.djangoapps.verify_student.signals.idv_update_signal.send')
+    @patch('lms.djangoapps.verify_student.signals.signals.idv_update_signal.send')
    def test_post_save_signal(self, mock_signal):
        # create new softwaresecureverification
        attempt = SoftwareSecurePhotoVerification.objects.create(
@@ -165,7 +165,7 @@ class PostSavePhotoVerificationTest(ModuleStoreTestCase):
            full_name=attempt.user.profile.name
        )

-    @patch('lms.djangoapps.verify_student.signals.idv_update_signal.send')
+    @patch('lms.djangoapps.verify_student.signals.signals.idv_update_signal.send')
    def test_post_save_signal_pending_name(self, mock_signal):
        pending_name_change = do_name_change_request(self.user, 'Pending Name', 'test')[0]

@@ -187,7 +187,7 @@ class PostSavePhotoVerificationTest(ModuleStoreTestCase):
        )


-class RetirementSignalVerificationAttemptsTest(ModuleStoreTestCase):
+class RetirementHandlerVerificationAttemptsTest(ModuleStoreTestCase):
    """
    Tests for the LMS User Retirement signal for Verification Attempts
    """
--- a/requirements/constraints.txt
+++ b/requirements/constraints.txt
@@ -142,7 +142,3 @@ django-storages<1.14.4
 # We are pinning this until after all the smaller migrations get handled and then we can migrate this all at once.
 # Ticket to unpin: https://github.com/edx/edx-arch-experiments/issues/760
 social-auth-app-django<=5.4.1
-
-# Temporary pin as to prevent a new version of edx-name-affirmation from being merged before we modify it to work
-# properly along with work in this PR: https://github.com/openedx/edx-platform/pull/35468
-edx-name-affirmation==2.4.0
--- a/requirements/edx/base.txt
+++ b/requirements/edx/base.txt
@@ -482,10 +482,8 @@ edx-i18n-tools==1.5.0
    #   ora2
 edx-milestones==0.6.0
    # via -r requirements/edx/kernel.in
-edx-name-affirmation==2.4.0
-    # via
-    #   -c requirements/edx/../constraints.txt
-    #   -r requirements/edx/kernel.in
+edx-name-affirmation==2.4.1
+    # via -r requirements/edx/kernel.in
 edx-opaque-keys[django]==2.11.0
    # via
    #   -r requirements/edx/kernel.in
--- a/requirements/edx/development.txt
+++ b/requirements/edx/development.txt
@@ -766,9 +766,8 @@ edx-milestones==0.6.0
    # via
    #   -r requirements/edx/doc.txt
    #   -r requirements/edx/testing.txt
-edx-name-affirmation==2.4.0
+edx-name-affirmation==2.4.1
    # via
-    #   -c requirements/edx/../constraints.txt
    #   -r requirements/edx/doc.txt
    #   -r requirements/edx/testing.txt
 edx-opaque-keys[django]==2.11.0
--- a/requirements/edx/doc.txt
+++ b/requirements/edx/doc.txt
@@ -562,10 +562,8 @@ edx-i18n-tools==1.5.0
    #   ora2
 edx-milestones==0.6.0
    # via -r requirements/edx/base.txt
-edx-name-affirmation==2.4.0
-    # via
-    #   -c requirements/edx/../constraints.txt
-    #   -r requirements/edx/base.txt
+edx-name-affirmation==2.4.1
+    # via -r requirements/edx/base.txt
 edx-opaque-keys[django]==2.11.0
    # via
    #   -r requirements/edx/base.txt
--- a/requirements/edx/testing.txt
+++ b/requirements/edx/testing.txt
@@ -588,10 +588,8 @@ edx-lint==5.3.7
    # via -r requirements/edx/testing.in
 edx-milestones==0.6.0
    # via -r requirements/edx/base.txt
-edx-name-affirmation==2.4.0
-    # via
-    #   -c requirements/edx/../constraints.txt
-    #   -r requirements/edx/base.txt
+edx-name-affirmation==2.4.1
+    # via -r requirements/edx/base.txt
 edx-opaque-keys[django]==2.11.0
    # via
    #   -r requirements/edx/base.txt