From 206977d571863945b902a379eb33a8c4a393d9c3 Mon Sep 17 00:00:00 2001 From: Asad Date: Tue, 7 Jul 2020 16:42:20 +0500 Subject: [PATCH] Fixed email sent on valid sso verification --- .../send_verification_expiry_email.py | 29 ++++++++++++++- .../test_send_verification_expiry_email.py | 37 ++++++++++++++++++- 2 files changed, 64 insertions(+), 2 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 e4a59e33d9..3b0ca6f33f 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 @@ -21,7 +21,7 @@ from student.models import CourseEnrollment 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 lms.djangoapps.verify_student.models import ManualVerification, SoftwareSecurePhotoVerification, SSOVerification 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 @@ -127,6 +127,9 @@ class Command(BaseCommand): success = True for verification in sspv: + user = verification.user + if self.user_has_valid_verification(user): + continue if not verification.expiry_email_date or verification.expiry_email_date <= date_resend_days_ago: batch_verifications.append(verification) @@ -142,6 +145,30 @@ class Command(BaseCommand): if not success: raise CommandError('One or more email attempts failed. Search for "Could not send" above.') + def user_has_valid_verification(self, user): + """ + Check if the user has a valid sso or manual verification + """ + return self.has_valid_sso_verification(user) or self.has_valid_manual_verification(user) + + def has_valid_sso_verification(self, user): + """ + Checks if the user has a valid sso verification + """ + sso_verifications = SSOVerification.objects.filter(user=user, status='approved') + for sso_verification in sso_verifications: + if sso_verification.expiration_datetime > now(): + return True + + def has_valid_manual_verification(self, user): + """ + Checks if the user has a valid manual verification + """ + manual_verifications = ManualVerification.objects.filter(user=user, status='approved') + for manual_verification in manual_verifications: + if manual_verification.expiration_datetime > now(): + return True + def send_verification_expiry_email(self, batch_verifications, email_config): """ Sends verification expiry email to the learners in the batch using edx_ace 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 e7215c06de..7b2e950eed 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 @@ -17,7 +17,7 @@ from student.tests.factories import UserFactory from testfixtures import LogCapture from common.test.utils import MockS3BotoMixin -from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification +from lms.djangoapps.verify_student.models import ManualVerification, SoftwareSecurePhotoVerification, SSOVerification from lms.djangoapps.verify_student.tests.test_models import FAKE_SETTINGS, mock_software_secure_post LOGGER_NAME = 'lms.djangoapps.verify_student.management.commands.send_verification_expiry_email' @@ -45,6 +45,17 @@ class TestSendVerificationExpiryEmail(MockS3BotoMixin, TestCase): attempt.submit() return attempt + def create_expired_software_secure_photo_verification(self): + """ + Helper method that creates an expired ssp verification + """ + user = UserFactory.create() + verification = self.create_and_submit(user) + verification.status = 'approved' + verification.expiry_date = now() - timedelta(days=self.days) + verification.save() + return verification + def test_expiry_date_range(self): """ Test that the verifications are filtered on the given range. Email is not sent for any verification with @@ -124,6 +135,30 @@ class TestSendVerificationExpiryEmail(MockS3BotoMixin, TestCase): self.assertEqual(attempt.expiry_email_date.date(), expected_date.date()) self.assertEqual(len(mail.outbox), 1) + def test_verification_expiry_email_not_sent_valid_ssov(self): + """ + Test that user has an expired software secure verification but a valid sso verification + so an email is not sent to the user + """ + expired_ssp_verification = self.create_expired_software_secure_photo_verification() + + SSOVerification.objects.create(user=expired_ssp_verification.user, status='approved') + + call_command('send_verification_expiry_email') + self.assertEqual(len(mail.outbox), 0) + + def test_verification_expiry_email_not_sent_valid_manual_verification(self): + """ + Test that user has an expired software secure verification but a valid manual verification + so an email is not sent to the user + """ + expired_ssp_verification = self.create_expired_software_secure_photo_verification() + + ManualVerification.objects.create(user=expired_ssp_verification.user, status='approved') + + call_command('send_verification_expiry_email') + self.assertEqual(len(mail.outbox), 0) + def test_email_already_sent(self): """ Test that if email is already sent as indicated by expiry_email_date then don't send again if it has been less