From 465d864c775b6ba7b499d83ab3933bbf83f3d0b3 Mon Sep 17 00:00:00 2001 From: Awais Jibran <awaisdar001@gmail.com> Date: Tue, 14 Apr 2020 19:13:56 +0500 Subject: [PATCH] Adds settings for celery task auto discovery --- lms/djangoapps/verify_student/apps.py | 1 + lms/djangoapps/verify_student/models.py | 9 +++---- lms/djangoapps/verify_student/tasks.py | 11 +++++--- .../verify_student/tests/test_utils.py | 25 +++++++++++++++++- .../verify_student/tests/test_views.py | 4 +-- lms/djangoapps/verify_student/utils.py | 26 ++++++++++++++++--- 6 files changed, 61 insertions(+), 15 deletions(-) diff --git a/lms/djangoapps/verify_student/apps.py b/lms/djangoapps/verify_student/apps.py index 32febff4c0d..edf0b2035a8 100644 --- a/lms/djangoapps/verify_student/apps.py +++ b/lms/djangoapps/verify_student/apps.py @@ -18,3 +18,4 @@ class VerifyStudentConfig(AppConfig): Connect signal handlers. """ from . import signals # pylint: disable=unused-variable + from . import tasks # pylint: disable=unused-variable diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index 3f27affb7f9..6a838e06977 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -44,7 +44,7 @@ from lms.djangoapps.verify_student.ssencrypt import ( from openedx.core.djangoapps.signals.signals import LEARNER_NOW_VERIFIED from openedx.core.storage import get_storage -from .utils import auto_verify_for_testing_enabled, earliest_allowed_verification_date +from .utils import auto_verify_for_testing_enabled, earliest_allowed_verification_date, submit_request_to_ss log = logging.getLogger(__name__) @@ -741,7 +741,7 @@ class SoftwareSecurePhotoVerification(PhotoVerification): def submit(self, copy_id_photo_from=None): """ Submit our verification attempt to Software Secure for validation. This - will set our status to "submitted" if the post is successful, and + will set our status to "submitted", if the post is successful or will set to "must_retry" if the post fails. Keyword Arguments: @@ -749,7 +749,6 @@ class SoftwareSecurePhotoVerification(PhotoVerification): data from this attempt. This is used for re-verification, in which new face photos are sent with previously-submitted ID photos. """ - from .tasks import send_request_to_ss_for_user if auto_verify_for_testing_enabled(): self.mark_submit() fake_response = requests.Response() @@ -764,9 +763,9 @@ class SoftwareSecurePhotoVerification(PhotoVerification): self.receipt_id, copy_id_photo_from.receipt_id, ) + transaction.on_commit( - lambda: - send_request_to_ss_for_user.delay(user_verification_id=self.id, copy_id_photo_from=copy_id_photo_from) + lambda: submit_request_to_ss(user_verification=self, copy_id_photo_from=copy_id_photo_from) ) def parsed_error_msg(self): diff --git a/lms/djangoapps/verify_student/tasks.py b/lms/djangoapps/verify_student/tasks.py index cee2c0838d7..4a9781d464f 100644 --- a/lms/djangoapps/verify_student/tasks.py +++ b/lms/djangoapps/verify_student/tasks.py @@ -11,8 +11,8 @@ from celery import Task, task from celery.states import FAILURE from django.conf import settings from django.core.mail import EmailMessage + from edxmako.shortcuts import render_to_string -from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers ACE_ROUTING_KEY = getattr(settings, 'ACE_ROUTING_KEY', None) @@ -41,8 +41,9 @@ class BaseSoftwareSecureTask(Task): 'response_ok': boolean, indicating if the response was ok 'response_text': string, indicating the response text in case of failure. """ - user_verification_id = kwargs['user_verification_id'] - user_verification = SoftwareSecurePhotoVerification.objects.get(id=user_verification_id) + from .models import SoftwareSecurePhotoVerification + + user_verification = SoftwareSecurePhotoVerification.objects.get(id=kwargs['user_verification_id']) if retval['response_ok']: user_verification.mark_submit() log.info( @@ -60,6 +61,8 @@ class BaseSoftwareSecureTask(Task): with "must_retry" so that it can be retried latter. """ if self.max_retries == self.request.retries and status == FAILURE: + from .models import SoftwareSecurePhotoVerification + user_verification_id = kwargs['user_verification_id'] user_verification = SoftwareSecurePhotoVerification.objects.get(id=user_verification_id) user_verification.mark_must_retry() @@ -110,6 +113,8 @@ def send_request_to_ss_for_user(self, user_verification_id, copy_id_photo_from): Returns: request.Response """ + from .models import SoftwareSecurePhotoVerification + user_verification = SoftwareSecurePhotoVerification.objects.get(id=user_verification_id) log.info('=>New Verification Task Received %r', user_verification.user.username) try: diff --git a/lms/djangoapps/verify_student/tests/test_utils.py b/lms/djangoapps/verify_student/tests/test_utils.py index 5418e068c95..a340968019a 100644 --- a/lms/djangoapps/verify_student/tests/test_utils.py +++ b/lms/djangoapps/verify_student/tests/test_utils.py @@ -8,13 +8,18 @@ import unittest from datetime import timedelta import ddt +import mock from django.conf import settings from django.utils import timezone from mock import patch from pytest import mark from lms.djangoapps.verify_student.models import ManualVerification, SoftwareSecurePhotoVerification, SSOVerification -from lms.djangoapps.verify_student.utils import most_recent_verification, verification_for_datetime +from lms.djangoapps.verify_student.utils import ( + most_recent_verification, + submit_request_to_ss, + verification_for_datetime +) from student.tests.factories import UserFactory FAKE_SETTINGS = { @@ -143,3 +148,21 @@ class TestVerifyStudentUtils(unittest.TestCase): self.assertEqual(most_recent, sso_verification) else: self.assertEqual(most_recent, manual_verification) + + @mock.patch('lms.djangoapps.verify_student.utils.log') + @mock.patch( + 'lms.djangoapps.verify_student.tasks.send_request_to_ss_for_user.delay', mock.Mock(side_effect=Exception('error')) + ) + def test_submit_request_to_ss(self, mock_log): + """Tests that we log appropriate information when celery task creation fails.""" + user = UserFactory.create() + attempt = SoftwareSecurePhotoVerification.objects.create(user=user) + attempt.mark_ready() + submit_request_to_ss(user_verification=attempt, copy_id_photo_from=None) + + mock_log.error.assert_called_with( + "Software Secure submit request %r failed, result: %s", + user.username, + 'error' + ) + self.assertTrue(attempt.status, SoftwareSecurePhotoVerification.STATUS.must_retry) diff --git a/lms/djangoapps/verify_student/tests/test_views.py b/lms/djangoapps/verify_student/tests/test_views.py index c4219bc3cf0..ad7716333e0 100644 --- a/lms/djangoapps/verify_student/tests/test_views.py +++ b/lms/djangoapps/verify_student/tests/test_views.py @@ -1749,7 +1749,7 @@ class TestPhotoVerificationResultsCallback(ModuleStoreTestCase, TestVerification mock.Mock(side_effect=mocked_has_valid_signature) ) @patch('lms.djangoapps.verify_student.views.log.error') - @patch('lms.djangoapps.verify_student.utils.SailthruClient.send') + @patch('sailthru.sailthru_client.SailthruClient.send') def test_passed_status_template(self, mock_sailthru_send, mock_log_error): """ Test for verification passed. @@ -1791,7 +1791,7 @@ class TestPhotoVerificationResultsCallback(ModuleStoreTestCase, TestVerification mock.Mock(side_effect=mocked_has_valid_signature) ) @patch('lms.djangoapps.verify_student.views.log.error') - @patch('lms.djangoapps.verify_student.utils.SailthruClient.send') + @patch('sailthru.sailthru_client.SailthruClient.send') def test_first_time_verification(self, mock_sailthru_send, mock_log_error): # pylint: disable=unused-argument """ Test for verification passed if the learner does not have any previous verification diff --git a/lms/djangoapps/verify_student/utils.py b/lms/djangoapps/verify_student/utils.py index 0e754e8ffa0..768f6ca9b76 100644 --- a/lms/djangoapps/verify_student/utils.py +++ b/lms/djangoapps/verify_student/utils.py @@ -3,24 +3,42 @@ Common Utilities for the verify_student application. """ - import datetime import logging from django.conf import settings from django.utils.timezone import now -from sailthru import SailthruClient +from six import text_type + +from lms.djangoapps.verify_student.tasks import send_request_to_ss_for_user log = logging.getLogger(__name__) +def submit_request_to_ss(user_verification, copy_id_photo_from): + """ + Submit our verification attempt to Software Secure for validation. + + Submits the task to software secure and If the task creation fails, + set the verification status to "must_retry". + """ + try: + send_request_to_ss_for_user.delay( + user_verification_id=user_verification.id, copy_id_photo_from=copy_id_photo_from + ) + except Exception as error: + log.error( + "Software Secure submit request %r failed, result: %s", user_verification.user.username, text_type(error) + ) + user_verification.mark_must_retry() + + def is_verification_expiring_soon(expiration_datetime): """ Returns True if verification is expiring within EXPIRING_SOON_WINDOW. """ if expiration_datetime: - if (expiration_datetime - now()).days <= settings.VERIFY_STUDENT.get( - "EXPIRING_SOON_WINDOW"): + if (expiration_datetime - now()).days <= settings.VERIFY_STUDENT.get("EXPIRING_SOON_WINDOW"): return True return False -- GitLab