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.

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

feat: add LMS retirement listener for VerificationAttempts #35436

Merged
merged 1 commit into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions lms/djangoapps/verify_student/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1214,3 +1214,13 @@ class VerificationAttempt(TimeStampedModel):
null=True,
blank=True,
)

@classmethod
def retire_user(cls, user_id):
"""
Retire user as part of GDPR pipeline

:param user_id: int
"""
verification_attempts = cls.objects.filter(user_id=user_id)
verification_attempts.delete()
10 changes: 8 additions & 2 deletions lms/djangoapps/verify_student/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
from xmodule.modulestore.django import SignalHandler, modulestore

from common.djangoapps.student.models_api import get_name, get_pending_name_change
from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_LMS_CRITICAL
from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_LMS_CRITICAL, USER_RETIRE_LMS_MISC

from .models import SoftwareSecurePhotoVerification, VerificationDeadline
from .models import SoftwareSecurePhotoVerification, VerificationDeadline, VerificationAttempt

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -75,3 +75,9 @@ def send_idv_update(sender, instance, **kwargs): # pylint: disable=unused-argum
photo_id_name=instance.name,
full_name=full_name
)


@receiver(USER_RETIRE_LMS_MISC)
def _listen_for_lms_retire_verification_attempts(sender, **kwargs): # pylint: disable=unused-argument
user = kwargs.get('user')
VerificationAttempt.retire_user(user.id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we handle exception during deletion? I feel like we should at least log info here to let the logger know we are deleting users.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had this same thought as you on Alie's persona-integration pull request: https://github.com/edx/persona-integration/pull/17, but my final opinion was that because this code runs as part of a separate retirement job that runs the retirement scripts, it would probably be better for it to fail loudly than to fail silently so that we know something went wrong. My thinking was that no one is going to be reviewing the output of that job to look for silent failures, but loud failures may get someone's attention.

7 changes: 6 additions & 1 deletion lms/djangoapps/verify_student/tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"""
from factory.django import DjangoModelFactory

from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification, SSOVerification
from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification, SSOVerification, VerificationAttempt


class SoftwareSecurePhotoVerificationFactory(DjangoModelFactory):
Expand All @@ -19,3 +19,8 @@ class Meta:
class SSOVerificationFactory(DjangoModelFactory):
class Meta():
model = SSOVerification


class VerificationAttemptFactory(DjangoModelFactory):
class Meta:
model = VerificationAttempt
40 changes: 37 additions & 3 deletions lms/djangoapps/verify_student/tests/test_signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,20 @@

from common.djangoapps.student.models_api import do_name_change_request
from common.djangoapps.student.tests.factories import UserFactory
from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification, VerificationDeadline
from lms.djangoapps.verify_student.signals import _listen_for_course_publish, _listen_for_lms_retire
from lms.djangoapps.verify_student.tests.factories import SoftwareSecurePhotoVerificationFactory
from lms.djangoapps.verify_student.models import (
SoftwareSecurePhotoVerification,
VerificationDeadline,
VerificationAttempt
)
from lms.djangoapps.verify_student.signals import (
_listen_for_course_publish,
_listen_for_lms_retire,
_listen_for_lms_retire_verification_attempts
)
from lms.djangoapps.verify_student.tests.factories import (
SoftwareSecurePhotoVerificationFactory,
VerificationAttemptFactory
)
from openedx.core.djangoapps.user_api.accounts.tests.retirement_helpers import fake_completed_retirement
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order
Expand Down Expand Up @@ -174,3 +185,26 @@ def test_post_save_signal_pending_name(self, mock_signal):
photo_id_name=attempt.name,
full_name=pending_name_change.new_name
)


class RetirementSignalVerificationAttemptsTest(ModuleStoreTestCase):
"""
Tests for the LMS User Retirement signal for Verification Attempts
"""

def setUp(self):
super().setUp()
self.user = UserFactory.create()
self.other_user = UserFactory.create()
VerificationAttemptFactory.create(user=self.user)
VerificationAttemptFactory.create(user=self.other_user)

def test_retirement_signal(self):
_listen_for_lms_retire_verification_attempts(sender=self.__class__, user=self.user)
self.assertEqual(len(VerificationAttempt.objects.filter(user=self.user)), 0)
self.assertEqual(len(VerificationAttempt.objects.filter(user=self.other_user)), 1)

def test_retirement_signal_no_attempts(self):
no_attempt_user = UserFactory.create()
_listen_for_lms_retire_verification_attempts(sender=self.__class__, user=no_attempt_user)
self.assertEqual(len(VerificationAttempt.objects.all()), 2)
Loading