Update verification expiry email criteria (#20725)

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

LEARNER-7313

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.

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)

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')

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):
     """

lms/djangoapps/verify_student/tests/factories.py

 """
 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"])

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):
     """