diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 0fa401d5a2cde26ed829b7d90a418d163e353d17..7cd56b94c7bdb233b5e8abd6c14d7aee31946801 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -61,6 +61,7 @@ from courseware.models import ( OrgDynamicUpgradeDeadlineConfiguration ) from lms.djangoapps.certificates.models import GeneratedCertificate +from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification from openedx.core.djangoapps.content.course_overviews.models import CourseOverview import openedx.core.djangoapps.django_comment_common.comment_client as cc from openedx.core.djangoapps.enrollments.api import _default_course_mode @@ -2059,6 +2060,19 @@ def invalidate_enrollment_mode_cache(sender, instance, **kwargs): # pylint: dis cache.delete(cache_key) +@receiver(models.signals.post_save, sender=CourseEnrollment) +def update_expiry_email_date(sender, instance, **kwargs): # pylint: disable=unused-argument + """ + If the user has enrolled in verified track of a course and has expired ID + verification then send email to get the ID verified by setting the + expiry_email_date field. + """ + email_config = getattr(settings, 'VERIFICATION_EXPIRY_EMAIL', {'DAYS_RANGE': 1, 'RESEND_DAYS': 15}) + + if instance.mode == CourseMode.VERIFIED: + SoftwareSecurePhotoVerification.update_expiry_email_date_for_user(instance.user, email_config) + + class ManualEnrollmentAudit(models.Model): """ Table for tracking which enrollments were performed through manual enrollment. diff --git a/lms/djangoapps/verify_student/management/commands/send_verification_expiry_email.py b/lms/djangoapps/verify_student/management/commands/send_verification_expiry_email.py index 152ac497db7cc11e3cffede70e65c2db7926bdbb..fce36f604168bdc09a8f4a6ddda6bda58f34892c 100644 --- a/lms/djangoapps/verify_student/management/commands/send_verification_expiry_email.py +++ b/lms/djangoapps/verify_student/management/commands/send_verification_expiry_email.py @@ -7,15 +7,17 @@ import logging import time from datetime import timedelta +from course_modes.models import CourseMode from django.conf import settings from django.contrib.auth.models import User from django.contrib.sites.models import Site -from django.core.management.base import BaseCommand +from django.core.management.base import BaseCommand, CommandError from django.db.models import Q from django.urls import reverse from django.utils.timezone import now from edx_ace import ace from edx_ace.recipient import Recipient +from student.models import CourseEnrollment from util.query import use_read_replica_if_available from lms.djangoapps.verify_student.message_types import VerificationExpiry @@ -38,7 +40,7 @@ class Command(BaseCommand): VERIFICATION_EXPIRY_EMAIL['RESEND_DAYS'] have passed since the last email. Since a user can have multiple verification all the previous verifications have expiry_date and expiry_email_date - set to None so that they are not filtered. See lms.djangoapps.verify_student.models.SoftwareSecurePhotoVerification + set to None so that they are not filtered. See lms/djangoapps/verify_student/views.py:1174 The range to filter expired verification is selected based on VERIFICATION_EXPIRY_EMAIL['DAYS_RANGE']. This represents the number of days before now and gives us start_date of the range @@ -48,7 +50,7 @@ class Command(BaseCommand): delay between batches is indicated by `sleep_time`.For each batch a celery task is initiated that sends the email Example usage: - $ ./manage.py lms send_verification_expiry_email --resend-days=30 --batch-size=2000 --sleep-time=5 + $ ./manage.py lms send_verification_expiry_email --batch-size=2000 --sleep-time=5 OR $ ./manage.py lms send_verification_expiry_email @@ -80,12 +82,17 @@ class Command(BaseCommand): It creates batches of expired Software Secure Photo Verification and sends it to send_verification_expiry_email that used edx_ace to send email to these learners """ + default_emails = settings.VERIFICATION_EXPIRY_EMAIL['DEFAULT_EMAILS'] resend_days = settings.VERIFICATION_EXPIRY_EMAIL['RESEND_DAYS'] days = settings.VERIFICATION_EXPIRY_EMAIL['DAYS_RANGE'] batch_size = options['batch_size'] sleep_time = options['sleep_time'] dry_run = options['dry_run'] + if default_emails <= 0: + raise CommandError(u'DEFAULT_EMAILS must be a positive integer. If you do not wish to send emails ' + u'use --dry-run flag instead.') + end_date = now().replace(hour=0, minute=0, second=0, microsecond=0) # If email was sent and user did not re-verify then this date will be used as the criteria for resending email date_resend_days_ago = end_date - timedelta(days=resend_days) @@ -111,55 +118,89 @@ class Command(BaseCommand): .format(start_date.date(), now().date(), total_verification)) batch_verifications = [] + email_config = { + 'resend_days': resend_days, + 'dry_run': dry_run, + 'default_emails': default_emails + } for verification in sspv: if not verification.expiry_email_date or verification.expiry_email_date <= date_resend_days_ago: batch_verifications.append(verification) if len(batch_verifications) == batch_size: - send_verification_expiry_email(batch_verifications, dry_run) + self.send_verification_expiry_email(batch_verifications, email_config) time.sleep(sleep_time) batch_verifications = [] # If selected verification in batch are less than batch_size if batch_verifications: - send_verification_expiry_email(batch_verifications, dry_run) + self.send_verification_expiry_email(batch_verifications, email_config) + def send_verification_expiry_email(self, batch_verifications, email_config): + """ + Sends verification expiry email to the learners in the batch using edx_ace + If the email is successfully sent change the expiry_email_date to reflect when the + email was sent -def send_verification_expiry_email(batch_verifications, dry_run=False): - """ - Spins a task to send verification expiry email to the learners in the batch using edx_ace - If the email is successfully sent change the expiry_email_date to reflect when the - email was sent - """ - if dry_run: - logger.info( - u"This was a dry run, no email was sent. For the actual run email would have been sent " - u"to {} learner(s)".format(len(batch_verifications)) - ) - return - - site = Site.objects.get_current() - message_context = get_base_template_context(site) - message_context.update({ - 'platform_name': settings.PLATFORM_NAME, - 'lms_verification_link': '{}{}'.format(settings.LMS_ROOT_URL, reverse("verify_student_reverify")), - 'help_center_link': settings.ID_VERIFICATION_SUPPORT_LINK - }) - - expiry_email = VerificationExpiry(context=message_context) - users = User.objects.filter(pk__in=[verification.user_id for verification in batch_verifications]) - - for verification in batch_verifications: - user = users.get(pk=verification.user_id) - with emulate_http_request(site=site, user=user): - msg = expiry_email.personalize( - recipient=Recipient(user.username, user.email), - language=get_user_preference(user, LANGUAGE_KEY), - user_context={ - 'full_name': user.profile.name, - } + :param batch_verifications: verification objects for which email will be sent + :param email_config: Contains configuration like dry-run flag value, which determines whether actual email will + be sent or not + """ + if email_config['dry_run']: + logger.info( + u"This was a dry run, no email was sent. For the actual run email would have been sent " + u"to {} learner(s)".format(len(batch_verifications)) ) - ace.send(msg) - verification_qs = SoftwareSecurePhotoVerification.objects.filter(pk=verification.pk) - verification_qs.update(expiry_email_date=now()) + return + + site = Site.objects.get_current() + message_context = get_base_template_context(site) + message_context.update({ + 'platform_name': settings.PLATFORM_NAME, + 'lms_verification_link': '{}{}'.format(settings.LMS_ROOT_URL, reverse("verify_student_reverify")), + 'help_center_link': settings.ID_VERIFICATION_SUPPORT_LINK + }) + + expiry_email = VerificationExpiry(context=message_context) + users = User.objects.filter(pk__in=[verification.user_id for verification in batch_verifications]) + + for verification in batch_verifications: + user = users.get(pk=verification.user_id) + with emulate_http_request(site=site, user=user): + msg = expiry_email.personalize( + recipient=Recipient(user.username, user.email), + language=get_user_preference(user, LANGUAGE_KEY), + user_context={ + 'full_name': user.profile.name, + } + ) + ace.send(msg) + self._set_email_expiry_date(verification, user, email_config) + + def _set_email_expiry_date(self, verification, user, email_config): + """ + If already DEFAULT Number of emails are sent, then verify that user is enrolled in at least + one verified course run for which the course has not ended else stop sending emails + + Setting email_expiry_date to None will prevent from sending any emails in future to the learner + + :param user: User for which course enrollments will be fetched + :param email_config: Contains configurations like resend_days and default_emails value + """ + send_expiry_email_again = True + email_duration = email_config['resend_days'] * (email_config['default_emails'] - 1) + days_since_expiry = (now() - verification.expiry_date).days + + if days_since_expiry >= email_duration: + send_expiry_email_again = False + + enrollments = CourseEnrollment.enrollments_for_user(user=user) + for enrollment in enrollments: + if CourseMode.VERIFIED == enrollment.mode and not enrollment.course.has_ended(): + send_expiry_email_again = True + break + + verification_qs = SoftwareSecurePhotoVerification.objects.filter(pk=verification.pk) + email_date = now().replace(hour=0, minute=0, second=0, microsecond=0) if send_expiry_email_again else None + verification_qs.update(expiry_email_date=email_date) diff --git a/lms/djangoapps/verify_student/management/commands/tests/test_send_verification_expiry_email.py b/lms/djangoapps/verify_student/management/commands/tests/test_send_verification_expiry_email.py index 23223cfc2ba057b32693e1e4bd87d940704170d5..e1c39038c49774e96c5af9c2a3ef21c98e9c9b99 100644 --- a/lms/djangoapps/verify_student/management/commands/tests/test_send_verification_expiry_email.py +++ b/lms/djangoapps/verify_student/management/commands/tests/test_send_verification_expiry_email.py @@ -10,12 +10,14 @@ import boto from django.conf import settings from django.contrib.sites.models import Site from django.core import mail -from django.core.management import call_command -from django.test import TestCase +from django.core.management import call_command, CommandError +from django.test.utils import override_settings from django.utils.timezone import now from mock import patch -from student.tests.factories import UserFactory +from student.tests.factories import CourseEnrollmentFactory, UserFactory from testfixtures import LogCapture +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory from common.test.utils import MockS3Mixin from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification @@ -26,7 +28,7 @@ LOGGER_NAME = 'lms.djangoapps.verify_student.management.commands.send_verificati @patch.dict(settings.VERIFY_STUDENT, FAKE_SETTINGS) @patch('lms.djangoapps.verify_student.models.requests.post', new=mock_software_secure_post) -class TestSendVerificationExpiryEmail(MockS3Mixin, TestCase): +class TestSendVerificationExpiryEmail(MockS3Mixin, ModuleStoreTestCase): """ Tests for django admin command `send_verification_expiry_email` in the verify_student module """ def setUp(self): @@ -181,3 +183,82 @@ class TestSendVerificationExpiryEmail(MockS3Mixin, TestCase): u"to {} learner(s)".format(count) )) self.assertEqual(len(mail.outbox), 0) + + def test_not_enrolled_in_verified_course(self): + """ + Test that if the user is not enrolled in verified track, then after sending the default no of + emails, `expiry_email_date` is updated to None so that it's not filtered in the future for + sending emails + """ + user = UserFactory.create() + today = now().replace(hour=0, minute=0, second=0, microsecond=0) + verification = self.create_and_submit(user) + verification.status = 'approved' + verification.expiry_date = now() - timedelta(days=self.resend_days * (self.default_no_of_emails - 1)) + verification.expiry_email_date = today - timedelta(days=self.resend_days) + verification.save() + + call_command('send_verification_expiry_email') + + # check that after sending the default number of emails, the expiry_email_date is set to none for a + # user who is not enrolled in verified track + attempt = SoftwareSecurePhotoVerification.objects.get(pk=verification.id) + self.assertEqual(len(mail.outbox), 1) + self.assertIsNone(attempt.expiry_email_date) + + def test_user_enrolled_in_verified_course(self): + """ + Test that if the user is enrolled in verified track, then after sending the default no of + emails, `expiry_email_date` is updated to now() so that it's filtered in the future to send + email again + """ + user = UserFactory.create() + course = CourseFactory() + CourseEnrollmentFactory.create(user=user, course_id=course.id, mode='verified') + today = now().replace(hour=0, minute=0, second=0, microsecond=0) + verification = self.create_and_submit(user) + verification.status = 'approved' + verification.expiry_date = now() - timedelta(days=self.resend_days * (self.default_no_of_emails - 1)) + verification.expiry_email_date = today - timedelta(days=self.resend_days) + verification.save() + + call_command('send_verification_expiry_email') + + attempt = SoftwareSecurePhotoVerification.objects.get(pk=verification.id) + self.assertEqual(attempt.expiry_email_date, today) + + def test_number_of_emails_sent(self): + """ + Tests that the number of emails sent in case the user is only enrolled in audit track are same + as DEFAULT_EMAILS set in the settings + """ + user = UserFactory.create() + verification = self.create_and_submit(user) + verification.status = 'approved' + + verification.expiry_date = now() - timedelta(days=1) + verification.save() + call_command('send_verification_expiry_email') + + # running the loop one extra time to verify that after sending DEFAULT_EMAILS no extra emails are sent and + # for this reason expiry_email_date is set to None + for i in range(1, self.default_no_of_emails + 1): + if SoftwareSecurePhotoVerification.objects.get(pk=verification.id).expiry_email_date: + today = now().replace(hour=0, minute=0, second=0, microsecond=0) + verification.expiry_date = today - timedelta(days=self.resend_days * i + 1) + verification.expiry_email_date = today - timedelta(days=self.resend_days) + verification.save() + call_command('send_verification_expiry_email') + else: + break + + # expiry_email_date set to None means it no longer will be filtered hence no emails will be sent in future + self.assertIsNone(SoftwareSecurePhotoVerification.objects.get(pk=verification.id).expiry_email_date) + self.assertEqual(len(mail.outbox), self.default_no_of_emails) + + @override_settings(VERIFICATION_EXPIRY_EMAIL={'RESEND_DAYS': 15, 'DAYS_RANGE': 1, 'DEFAULT_EMAILS': 0}) + def test_command_error(self): + err_string = u"DEFAULT_EMAILS must be a positive integer. If you do not wish to send " \ + u"emails use --dry-run flag instead." + with self.assertRaisesRegexp(CommandError, err_string): + call_command('send_verification_expiry_email') diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index 9388fd1fba5afd54c1f80742d921c014814f040d..50f505297fe3a809caf5e8dacb24b12f176e3b98 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -929,6 +929,41 @@ class SoftwareSecurePhotoVerification(PhotoVerification): """Whether or not the status from this attempt should be displayed to the user.""" return True + @classmethod + def get_recent_verification(cls, user): + """ + Return the most recent approved verification of user + + Keyword Arguments: + user (User): The user for which the most recent approved verification is fetched + + Returns: + SoftwareSecurePhotoVerification (object) or None + """ + recent_verification = SoftwareSecurePhotoVerification.objects.filter(status='approved', user_id=user.id) + return recent_verification.latest('updated_at') if recent_verification.exists() else None + + @classmethod + def update_expiry_email_date_for_user(cls, user, email_config): + """ + Updates the expiry_email_date to send expiry email if the most recent approved + verification for the user is expired. + + Keyword Arguments: + user (User): user object + email_config (dict): Contains configuration related to verification expiry email + + """ + today = now().replace(hour=0, minute=0, second=0, microsecond=0) + recently_expired_date = today - timedelta(days=email_config['DAYS_RANGE']) + + verification = SoftwareSecurePhotoVerification.get_recent_verification(user) + + if verification and verification.expiry_date < recently_expired_date and not verification.expiry_email_date: + expiry_email_date = today - timedelta(days=email_config['RESEND_DAYS']) + SoftwareSecurePhotoVerification.objects.filter(pk=verification.pk).update( + expiry_email_date=expiry_email_date) + class VerificationDeadline(TimeStampedModel): """ diff --git a/lms/djangoapps/verify_student/tests/factories.py b/lms/djangoapps/verify_student/tests/factories.py index 295e632b46189324f84dc8b656e878b0656118f6..581768f60733cea8d75275537e32b4fce30c976a 100644 --- a/lms/djangoapps/verify_student/tests/factories.py +++ b/lms/djangoapps/verify_student/tests/factories.py @@ -1,7 +1,10 @@ """ Factories related to student verification. """ +from datetime import timedelta +from django.conf import settings +from django.utils.timezone import now from factory.django import DjangoModelFactory from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification @@ -15,3 +18,5 @@ class SoftwareSecurePhotoVerificationFactory(DjangoModelFactory): model = SoftwareSecurePhotoVerification status = 'approved' + if hasattr(settings, 'VERIFY_STUDENT'): + expiry_date = now() + timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"]) diff --git a/lms/djangoapps/verify_student/tests/test_models.py b/lms/djangoapps/verify_student/tests/test_models.py index f03c9825f77afec8e2e928c019bbe6b44d353980..42df6b4d59705cb307088af5ee555a506a7e39f3 100644 --- a/lms/djangoapps/verify_student/tests/test_models.py +++ b/lms/djangoapps/verify_student/tests/test_models.py @@ -368,6 +368,57 @@ class TestPhotoVerification(TestVerification, MockS3Mixin, ModuleStoreTestCase): # No user self.assertFalse(attempt.retire_user(user_id=47)) + def test_get_recent_verification(self): + """Test that method 'get_recent_verification' of model + 'SoftwareSecurePhotoVerification' always returns the most + recent 'approved' verification based on updated_at set + against a user. + """ + user = UserFactory.create() + attempt = None + + for _ in range(2): + # Make an approved verification + attempt = SoftwareSecurePhotoVerification(user=user) + attempt.status = 'approved' + attempt.save() + + # Test method 'get_recent_verification' returns the most recent + # verification attempt based on updated_at + recent_verification = SoftwareSecurePhotoVerification.get_recent_verification(user=user) + self.assertIsNotNone(recent_verification) + self.assertEqual(recent_verification.id, attempt.id) + + def test_no_approved_verification(self): + """Test that method 'get_recent_verification' of model + 'SoftwareSecurePhotoVerification' returns None if no + 'approved' verification are found + """ + user = UserFactory.create() + SoftwareSecurePhotoVerification(user=user) + + result = SoftwareSecurePhotoVerification.get_recent_verification(user=user) + self.assertIs(result, None) + + def test_update_expiry_email_date_for_user(self): + """Test that method update_expiry_email_date_for_user of + model 'SoftwareSecurePhotoVerification' set expiry_email_date + if the most recent approved verification is expired. + """ + email_config = getattr(settings, 'VERIFICATION_EXPIRY_EMAIL', {'DAYS_RANGE': 1, 'RESEND_DAYS': 15}) + user = UserFactory.create() + verification = SoftwareSecurePhotoVerification(user=user) + verification.expiry_date = now() - timedelta(days=FAKE_SETTINGS['DAYS_GOOD_FOR']) + verification.status = 'approved' + verification.save() + + self.assertIsNone(verification.expiry_email_date) + + SoftwareSecurePhotoVerification.update_expiry_email_date_for_user(user, email_config) + result = SoftwareSecurePhotoVerification.get_recent_verification(user=user) + + self.assertIsNotNone(result.expiry_email_date) + class SSOVerificationTest(TestVerification): """