Merge pull request from zamir/LEARNER_7313_move_command_arguments_to_…

…common_settings

Move command arguments to common settings

lms/djangoapps/verify_student/management/commands/send_verification_expiry_email.py

@@ -16,14 +16,14 @@
 from django.utils.timezone import now
 from edx_ace import ace
 from edx_ace.recipient import Recipient
+from util.query import use_read_replica_if_available
 
+from lms.djangoapps.verify_student.message_types import VerificationExpiry
 from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification
 from openedx.core.djangoapps.ace_common.template_context import get_base_template_context
 from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY
 from openedx.core.djangoapps.user_api.preferences.api import get_user_preference
 from openedx.core.lib.celery.task_utils import emulate_http_request
-from util.query import use_read_replica_if_available
-from verify_student.message_types import VerificationExpiry
 
 logger = logging.getLogger(__name__)
 
@@ -34,11 +34,14 @@ class Command(BaseCommand):
 
     The expiry email is sent when the date represented by SoftwareSecurePhotoVerification's field `expiry_date`
     lies within the date range provided by command arguments. If the email is already sent indicated by field
-    `expiry_email_date` then filter if the specified number of days given as command argument `--resend_days` have
-    passed
+    `expiry_email_date` then filter if the specified number of days given in settings as
+    VERIFICATION_EXPIRY_EMAIL['RESEND_DAYS'] have passed since the last email.
 
-    The range to filter expired verification is selected based on --days-range. This represents the number of days
-    before now and gives us start_date of the range
+    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
+
+    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
          Range:       start_date to today
 
     The task is performed in batches with maximum number of users to send email given in `batch_size` and the
@@ -55,12 +58,6 @@ class Command(BaseCommand):
     help = 'Send email to users for which Software Secure Photo Verification has expired'
 
     def add_arguments(self, parser):
-        parser.add_argument(
-            '-d', '--resend-days',
-            type=int,
-            default=15,
-            help='Desired days after which the email will be resent to learners with expired verification'
-        )
         parser.add_argument(
             '--batch-size',
             type=int,
@@ -71,11 +68,6 @@ def add_arguments(self, parser):
             type=int,
             default=10,
             help='Sleep time in seconds between update of batches')
-        parser.add_argument(
-            '--days-range',
-            type=int,
-            default=1,
-            help="The number of days before now to check expired verification")
         parser.add_argument(
             '--dry-run',
             action='store_true',
@@ -88,10 +80,10 @@ def handle(self, *args, **options):
         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
         """
-        resend_days = options['resend_days']
+        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']
-        days = options['days_range']
         dry_run = options['dry_run']
 
         end_date = now().replace(hour=0, minute=0, second=0, microsecond=0)
@@ -104,7 +96,7 @@ def handle(self, *args, **options):
         query = SoftwareSecurePhotoVerification.objects.filter(Q(status='approved') &
                                                                (Q(expiry_date__gte=start_date,
                                                                   expiry_date__lt=end_date) |
-                                                                Q(expiry_email_date__lt=date_resend_days_ago)
+                                                                Q(expiry_email_date__lte=date_resend_days_ago)
                                                                 )).order_by()
 
         sspv = use_read_replica_if_available(query)
@@ -121,7 +113,7 @@ def handle(self, *args, **options):
         batch_verifications = []
 
         for verification in sspv:
-            if not verification.expiry_email_date or verification.expiry_email_date < date_resend_days_ago:
+            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:

lms/djangoapps/verify_student/management/commands/tests/test_send_verification_expiry_email.py

@@ -14,12 +14,12 @@
 from django.test import TestCase
 from django.utils.timezone import now
 from mock import patch
+from student.tests.factories import UserFactory
 from testfixtures import LogCapture
 
 from common.test.utils import MockS3Mixin
 from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification
 from lms.djangoapps.verify_student.tests.test_models import FAKE_SETTINGS, mock_software_secure_post
-from student.tests.factories import UserFactory
 
 LOGGER_NAME = 'lms.djangoapps.verify_student.management.commands.send_verification_expiry_email'
 
@@ -35,6 +35,9 @@ def setUp(self):
         connection = boto.connect_s3()
         connection.create_bucket(FAKE_SETTINGS['SOFTWARE_SECURE']['S3_BUCKET'])
         Site.objects.create(domain='edx.org', name='edx.org')
+        self.resend_days = settings.VERIFICATION_EXPIRY_EMAIL['RESEND_DAYS']
+        self.days = settings.VERIFICATION_EXPIRY_EMAIL['DAYS_RANGE']
+        self.default_no_of_emails = settings.VERIFICATION_EXPIRY_EMAIL['DEFAULT_EMAILS']
 
     def create_and_submit(self, user):
         """ Helper method that lets us create new SoftwareSecurePhotoVerifications """
@@ -53,16 +56,16 @@ def test_expiry_date_range(self):
         user = UserFactory.create()
         verification_in_range = self.create_and_submit(user)
         verification_in_range.status = 'approved'
-        verification_in_range.expiry_date = now() - timedelta(days=1)
+        verification_in_range.expiry_date = now() - timedelta(days=self.days)
         verification_in_range.save()
 
         user = UserFactory.create()
         verification = self.create_and_submit(user)
         verification.status = 'approved'
-        verification.expiry_date = now() - timedelta(days=5)
+        verification.expiry_date = now() - timedelta(days=self.days + 1)
         verification.save()
 
-        call_command('send_verification_expiry_email', '--days-range=2')
+        call_command('send_verification_expiry_email')
 
         # Check that only one email is sent
         self.assertEqual(len(mail.outbox), 1)
@@ -77,14 +80,14 @@ def test_expiry_email_date_range(self):
         resending email
         """
         user = UserFactory.create()
+        today = now().replace(hour=0, minute=0, second=0, microsecond=0)
         verification_in_range = self.create_and_submit(user)
         verification_in_range.status = 'approved'
-        verification_in_range.expiry_date = now() - timedelta(days=30)
-        verification_in_range.expiry_email_date = now() - timedelta(days=3)
+        verification_in_range.expiry_date = today - timedelta(days=self.days + 1)
+        verification_in_range.expiry_email_date = today - timedelta(days=self.resend_days)
         verification_in_range.save()
 
-        command_args = '--days-range={} --resend-days={}'  # pylint: disable=unicode-format-string
-        call_command('send_verification_expiry_email', *command_args.format(2, 2).split(' '))
+        call_command('send_verification_expiry_email')
 
         # Check that email is sent even if the verification is not in expiry_date range but matches the criteria
         # to resend email
@@ -112,7 +115,7 @@ def test_send_verification_expiry_email(self):
         user = UserFactory.create()
         verification = self.create_and_submit(user)
         verification.status = 'approved'
-        verification.expiry_date = now() - timedelta(days=1)
+        verification.expiry_date = now() - timedelta(days=self.days)
         verification.save()
 
         call_command('send_verification_expiry_email')
@@ -130,7 +133,7 @@ def test_email_already_sent(self):
         user = UserFactory.create()
         verification = self.create_and_submit(user)
         verification.status = 'approved'
-        verification.expiry_date = now() - timedelta(days=1)
+        verification.expiry_date = now() - timedelta(days=self.days)
         verification.expiry_email_date = now()
         verification.save()
 
@@ -142,7 +145,7 @@ def test_no_verification_found(self):
         """
         Test that if no approved and expired verifications are found the management command terminates gracefully
         """
-        start_date = now() - timedelta(days=1)  # using default days
+        start_date = now() - timedelta(days=self.days)  # using default days
         with LogCapture(LOGGER_NAME) as logger:
             call_command('send_verification_expiry_email')
             logger.check(
@@ -158,10 +161,10 @@ def test_dry_run_flag(self):
         user = UserFactory.create()
         verification = self.create_and_submit(user)
         verification.status = 'approved'
-        verification.expiry_date = now() - timedelta(days=1)
+        verification.expiry_date = now() - timedelta(days=self.days)
         verification.save()
 
-        start_date = now() - timedelta(days=1)  # using default days
+        start_date = now() - timedelta(days=self.days)  # using default days
         count = 1
 
         with LogCapture(LOGGER_NAME) as logger:

lms/envs/common.py

@@ -2480,6 +2480,14 @@ def _make_locale_paths(settings):
     # The variable represents the window within which a verification is considered to be "expiring soon."
     "EXPIRING_SOON_WINDOW": 28,
 }
+
+################# Student Verification Expiry Email #################
+VERIFICATION_EXPIRY_EMAIL = {
+    "RESEND_DAYS": 15,
+    "DAYS_RANGE": 1,
+    "DEFAULT_EMAILS": 2,
+}
+
 DISABLE_ACCOUNT_ACTIVATION_REQUIREMENT_SWITCH = "verify_student_disable_account_activation_requirement"
 
 ### This enables the Metrics tab for the Instructor dashboard ###########