From f6603cb8cfd1f46236db6049516f7f8c6e1c38ab Mon Sep 17 00:00:00 2001
From: Zainab Amir <40633976+zainab-amir@users.noreply.github.com>
Date: Wed, 3 Jul 2019 14:19:22 +0500
Subject: [PATCH] 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 | 14 ++
.../send_verification_expiry_email.py | 123 ++++++++++++------
.../test_send_verification_expiry_email.py | 89 ++++++++++++-
lms/djangoapps/verify_student/models.py | 35 +++++
.../verify_student/tests/factories.py | 5 +
.../verify_student/tests/test_models.py | 51 ++++++++
6 files changed, 272 insertions(+), 45 deletions(-)
diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py
index 0fa401d5a2c..7cd56b94c7b 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 152ac497db7..fce36f60416 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 23223cfc2ba..e1c39038c49 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 9388fd1fba5..50f505297fe 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 295e632b461..581768f6073 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 f03c9825f77..42df6b4d597 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):
"""
--
GitLab