From 2e72791491782242a67d9e990801ca88c68815c1 Mon Sep 17 00:00:00 2001 From: Bianca Severino Date: Wed, 10 Feb 2021 16:55:51 -0500 Subject: [PATCH] Create command to update expiration_date for old SoftwareSecurePhotoVerification entries (#26471) --- .../tests/test_populate_expiration_date.py | 65 ----------- .../tests/test_update_expiration_date.py | 102 ++++++++++++++++++ ...tion_date.py => update_expiration_date.py} | 51 ++++----- 3 files changed, 124 insertions(+), 94 deletions(-) delete mode 100644 lms/djangoapps/verify_student/management/commands/tests/test_populate_expiration_date.py create mode 100644 lms/djangoapps/verify_student/management/commands/tests/test_update_expiration_date.py rename lms/djangoapps/verify_student/management/commands/{populate_expiration_date.py => update_expiration_date.py} (56%) diff --git a/lms/djangoapps/verify_student/management/commands/tests/test_populate_expiration_date.py b/lms/djangoapps/verify_student/management/commands/tests/test_populate_expiration_date.py deleted file mode 100644 index e1f18a4e64..0000000000 --- a/lms/djangoapps/verify_student/management/commands/tests/test_populate_expiration_date.py +++ /dev/null @@ -1,65 +0,0 @@ -""" -Tests for django admin command `populate_expiration_date` in the verify_student module -""" - - -from datetime import timedelta - -from django.conf import settings -from django.core.management import call_command -from django.test import TestCase -from django.utils.timezone import now -from mock import patch -from testfixtures import LogCapture - -from common.test.utils import MockS3BotoMixin -from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification -from lms.djangoapps.verify_student.tests.test_models import FAKE_SETTINGS, mock_software_secure_post -from common.djangoapps.student.tests.factories import UserFactory - -LOGGER_NAME = 'lms.djangoapps.verify_student.management.commands.populate_expiration_date' - - -@patch.dict(settings.VERIFY_STUDENT, FAKE_SETTINGS) -@patch('lms.djangoapps.verify_student.models.requests.post', new=mock_software_secure_post) -class TestPopulateExpiryationDate(MockS3BotoMixin, TestCase): - """ Tests for django admin command `populate_expiration_date` in the verify_student module """ - - def create_and_submit(self, user): - """ Helper method that lets us create new SoftwareSecurePhotoVerifications """ - attempt = SoftwareSecurePhotoVerification(user=user) - attempt.upload_face_image("Fake Data") - attempt.upload_photo_id_image("More Fake Data") - attempt.mark_ready() - attempt.submit() - attempt.expiry_date = now() + timedelta(days=FAKE_SETTINGS["DAYS_GOOD_FOR"]) - return attempt - - def test_no_expiry_date(self): - """ - Test that the `expiration_date` for most recent approved verification is updated only when the - deprecated field `expiry_date` is still present - """ - user = UserFactory.create() - verification = self.create_and_submit(user) - verification.status = 'approved' - verification.expiry_date = None - verification.save() - - expiration_date = verification.expiration_date - call_command('populate_expiration_date') - - # Check that the `expiration_date` for approved verification is not changed - verification_expiration_date = SoftwareSecurePhotoVerification.objects.get(pk=verification.pk).expiration_date - - self.assertEqual(verification_expiration_date, expiration_date) - - def test_no_approved_verification_found(self): - """ - Test that if no approved verifications are found the management command terminates gracefully - """ - with LogCapture(LOGGER_NAME) as logger: - call_command('populate_expiration_date') - logger.check( - (LOGGER_NAME, 'INFO', "No approved entries found in SoftwareSecurePhotoVerification") - ) diff --git a/lms/djangoapps/verify_student/management/commands/tests/test_update_expiration_date.py b/lms/djangoapps/verify_student/management/commands/tests/test_update_expiration_date.py new file mode 100644 index 0000000000..329d894634 --- /dev/null +++ b/lms/djangoapps/verify_student/management/commands/tests/test_update_expiration_date.py @@ -0,0 +1,102 @@ +""" +Tests for django admin command `update_expiration_date` in the verify_student module +""" + + +from datetime import timedelta + +from django.conf import settings +from django.core.management import call_command +from django.test import TestCase +from django.utils.timezone import now +from mock import patch +from testfixtures import LogCapture + +from common.test.utils import MockS3BotoMixin +from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification +from lms.djangoapps.verify_student.tests.test_models import FAKE_SETTINGS, mock_software_secure_post +from common.djangoapps.student.tests.factories import UserFactory + +LOGGER_NAME = 'lms.djangoapps.verify_student.management.commands.update_expiration_date' + + +@patch.dict(settings.VERIFY_STUDENT, FAKE_SETTINGS) +@patch('lms.djangoapps.verify_student.models.requests.post', new=mock_software_secure_post) +class TestPopulateExpiryationDate(MockS3BotoMixin, TestCase): + """ Tests for django admin command `update_expiration_date` in the verify_student module """ + + def create_and_submit(self, user): + """ Helper method that lets us create new SoftwareSecurePhotoVerifications """ + attempt = SoftwareSecurePhotoVerification(user=user) + attempt.upload_face_image("Fake Data") + attempt.upload_photo_id_image("More Fake Data") + attempt.mark_ready() + attempt.submit() + attempt.expiry_date = now() + timedelta(days=FAKE_SETTINGS["DAYS_GOOD_FOR"]) + return attempt + + def test_expiration_date_not_changed(self): + """ + Test that the `expiration_date` is updated only when its value is over 365 days higher + than `updated_at` + """ + user = UserFactory.create() + verification = self.create_and_submit(user) + expected_date = verification.expiration_date + call_command('update_expiration_date') + # Check that the `expiration_date` is not changed + expiration_date = SoftwareSecurePhotoVerification.objects.get(pk=verification.pk).expiration_date + self.assertEqual(expiration_date, expected_date) + + def test_expiration_date_updated(self): + """ + Test that `expiration_date` is properly updated when its value is over 365 days higher + than `updated_at` + """ + user = UserFactory.create() + verification = self.create_and_submit(user) + verification.status = 'approved' + verification.expiration_date = now() + timedelta(days=FAKE_SETTINGS["DAYS_GOOD_FOR"] + 1) + verification.expiry_date = now() + timedelta(days=FAKE_SETTINGS["DAYS_GOOD_FOR"]) + verification.save() + expected_date = verification.created_at + timedelta( + days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"]) + call_command('update_expiration_date') + updated_verification = SoftwareSecurePhotoVerification.objects.get(pk=verification.pk) + # Check that the `expiration_date` is updated + self.assertEqual(updated_verification.expiration_date, expected_date) + # Check that the `expiry_date` is set to NULL + self.assertEqual(updated_verification.expiry_date, None) + + def test_expiration_date_updated_multiple_records(self): + """ + Test that `expiration_date` is properly updated with multiple records fits the selection + criteria + """ + users = UserFactory.create_batch(10) + for user in users: + verification = self.create_and_submit(user) + verification.status = 'approved' + verification.expiry_date = now() + timedelta(days=3) + verification.expiration_date = '2021-11-12T16:33:15.691Z' + verification.save() + + call_command('update_expiration_date', batch_size=3, sleep_time=1) + updated_verifications = SoftwareSecurePhotoVerification.objects.filter(user__in=users) + for updated_verification in updated_verifications: + expected_date = updated_verification.created_at + timedelta( + days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"]) + # Check that the `expiration_date` is updated + self.assertEqual(updated_verification.expiration_date, expected_date) + # Check that the `expiry_date` is set to NULL + self.assertEqual(updated_verification.expiry_date, None) + + def test_no_approved_verification_found(self): + """ + Test that if no approved verifications are found the management command terminates gracefully + """ + with LogCapture(LOGGER_NAME) as logger: + call_command('update_expiration_date') + logger.check( + (LOGGER_NAME, 'INFO', "No approved entries found in SoftwareSecurePhotoVerification") + ) diff --git a/lms/djangoapps/verify_student/management/commands/populate_expiration_date.py b/lms/djangoapps/verify_student/management/commands/update_expiration_date.py similarity index 56% rename from lms/djangoapps/verify_student/management/commands/populate_expiration_date.py rename to lms/djangoapps/verify_student/management/commands/update_expiration_date.py index 6932ef2d3b..c669a3dcb0 100644 --- a/lms/djangoapps/verify_student/management/commands/populate_expiration_date.py +++ b/lms/djangoapps/verify_student/management/commands/update_expiration_date.py @@ -1,5 +1,5 @@ """ -Django admin command to populate expiration_date for approved verifications in SoftwareSecurePhotoVerification +Django admin command to update expiration_date for approved verifications in SoftwareSecurePhotoVerification """ @@ -19,18 +19,18 @@ logger = logging.getLogger(__name__) class Command(BaseCommand): """ - This command sets the `expiration_date` for users for which the deprecated field `expiry_date` is set + This command updates the `expiration_date` for old entries still dependent on `expiry_date` The task is performed in batches with maximum number of rows to process given in argument `batch_size` and a sleep time between each batch given by `sleep_time` Default values: `batch_size` = 1000 rows `sleep_time` = 10 seconds Example usage: - $ ./manage.py lms populate_expiration_date --batch_size=1000 --sleep_time=5 + $ ./manage.py lms update_expiration_date --batch_size=1000 --sleep_time=5 OR - $ ./manage.py lms populate_expiration_date + $ ./manage.py lms update_expiration_date """ - help = 'Populate expiration_date for approved verifications' + help = 'Update expiration_date for approved verifications' def add_arguments(self, parser): parser.add_argument( @@ -52,8 +52,7 @@ class Command(BaseCommand): def handle(self, *args, **options): """ Handler for the command - It filters approved Software Secure Photo Verification and then for each distinct user it finds the most - recent approved verification and set its expiration_date + It filters approved Software Secure Photo Verifications and then sets the correct expiration_date """ batch_size = options['batch_size'] sleep_time = options['sleep_time'] @@ -65,24 +64,23 @@ class Command(BaseCommand): logger.info("No approved entries found in SoftwareSecurePhotoVerification") return - distinct_user_ids = set() update_verification_ids = [] update_verification_count = 0 for verification in sspv: - if verification.user_id not in distinct_user_ids: - distinct_user_ids.add(verification.user_id) + # The expiration date should not be higher than 365 days + # past the `updated_at` field, so only update those entries + if verification.expiration_date > ( + verification.updated_at + timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"]) + ): + update_verification_ids.append(verification.pk) + update_verification_count += 1 - recent_verification = self.find_recent_verification(sspv, verification.user_id) - if recent_verification.expiry_date: - update_verification_ids.append(recent_verification.pk) - update_verification_count += 1 - - if update_verification_count == batch_size: - self.bulk_update(update_verification_ids) - update_verification_count = 0 - update_verification_ids = [] - time.sleep(sleep_time) + if update_verification_count == batch_size: + self.bulk_update(update_verification_ids) + update_verification_count = 0 + update_verification_ids = [] + time.sleep(sleep_time) if update_verification_ids: self.bulk_update(update_verification_ids) @@ -92,12 +90,7 @@ class Command(BaseCommand): It updates the expiration_date and sets the expiry_date to NULL for all the verifications whose ids lie in verification_ids """ - recent_verification_qs = SoftwareSecurePhotoVerification.objects.filter(pk__in=verification_ids) - recent_verification_qs.update(expiration_date=F('expiry_date')) - recent_verification_qs.update(expiry_date=None) - - def find_recent_verification(self, model, user_id): - """ - Returns the most recent approved verification for a user - """ - return model.filter(user_id=user_id).latest('updated_at') + verification_qs = SoftwareSecurePhotoVerification.objects.filter(pk__in=verification_ids) + verification_qs.update(expiration_date=F('created_at') + timedelta( + days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"])) + verification_qs.update(expiry_date=None)