From 886834adb55be719e1a8bb23b65633cc26c65492 Mon Sep 17 00:00:00 2001 From: Zainab Amir Date: Tue, 28 May 2019 12:57:55 +0500 Subject: [PATCH] Move command arguments to common settings Move command parameter arguments, resend-days and days-range, to common settings. This will help in creating a consistency when the default values are changed in the future. LEARNER-7313 --- .../send_verification_expiry_email.py | 34 +++++++------------ .../test_send_verification_expiry_email.py | 29 +++++++++------- lms/envs/common.py | 8 +++++ 3 files changed, 37 insertions(+), 34 deletions(-) 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 0b08cdc89a..152ac497db 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 @@ -16,14 +16,14 @@ from django.urls import reverse 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 @@ class Command(BaseCommand): 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 @@ 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 """ - 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 @@ class Command(BaseCommand): 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 @@ class Command(BaseCommand): 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: 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 5f3a0beafd..23223cfc2b 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 @@ -14,12 +14,12 @@ from django.core.management import call_command 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 @@ class TestSendVerificationExpiryEmail(MockS3Mixin, TestCase): 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 @@ class TestSendVerificationExpiryEmail(MockS3Mixin, TestCase): 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 @@ class TestSendVerificationExpiryEmail(MockS3Mixin, TestCase): 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 @@ class TestSendVerificationExpiryEmail(MockS3Mixin, TestCase): 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 @@ class TestSendVerificationExpiryEmail(MockS3Mixin, TestCase): 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 @@ class TestSendVerificationExpiryEmail(MockS3Mixin, TestCase): """ 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 @@ class TestSendVerificationExpiryEmail(MockS3Mixin, TestCase): 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: diff --git a/lms/envs/common.py b/lms/envs/common.py index 782908fdd8..c3f7d64a6a 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2480,6 +2480,14 @@ VERIFY_STUDENT = { # 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 ###########