diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index fe61d7d24a9a7d3e9482962ba917800c930ac012..9fba0e8010cb0b0f1220b7936cebce0cbc695e1c 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -154,12 +154,9 @@ def check_verify_status_by_course(user, course_enrollments): # By default, don't show any status related to verification status = None - should_display = True # Check whether the user was approved or is awaiting approval if relevant_verification is not None: - should_display = relevant_verification.should_display_status_to_user() - if relevant_verification.status == "approved": if verification_expiring_soon: status = VERIFY_STATUS_NEED_TO_REVERIFY @@ -217,8 +214,7 @@ def check_verify_status_by_course(user, course_enrollments): status_by_course[enrollment.course_id] = { 'status': status, - 'days_until_deadline': days_until_deadline, - 'should_display': should_display, + 'days_until_deadline': days_until_deadline } if recent_verification_datetime: diff --git a/common/djangoapps/student/views/dashboard.py b/common/djangoapps/student/views/dashboard.py index 78e2b4bd2c609df6d33a6ff6ae3a69417753522d..805ba14b741fa65a02cc22613acf4952f0edf21e 100644 --- a/common/djangoapps/student/views/dashboard.py +++ b/common/djangoapps/student/views/dashboard.py @@ -716,8 +716,8 @@ def student_dashboard(request): # Verification Attempts # Used to generate the "you must reverify for course x" banner - verification_status = IDVerificationService.user_status(user) - verification_errors = get_verification_error_reasons_for_display(verification_status['error']) + verification_status, verification_error_codes = IDVerificationService.user_status(user) + verification_errors = get_verification_error_reasons_for_display(verification_error_codes) # Gets data for midcourse reverifications, if any are necessary or have failed statuses = ["approved", "denied", "pending", "must_reverify"] @@ -770,9 +770,7 @@ def student_dashboard(request): redirect_message = '' valid_verification_statuses = ['approved', 'must_reverify', 'pending', 'expired'] - display_sidebar_on_dashboard = (len(order_history_list) or - (verification_status['status'] in valid_verification_statuses and - verification_status['should_display'])) + display_sidebar_on_dashboard = len(order_history_list) or verification_status in valid_verification_statuses # Filter out any course enrollment course cards that are associated with fulfilled entitlements for entitlement in [e for e in course_entitlements if e.enrollment_course_run is not None]: @@ -804,8 +802,7 @@ def student_dashboard(request): 'credit_statuses': _credit_statuses(user, course_enrollments), 'show_email_settings_for': show_email_settings_for, 'reverifications': reverifications, - 'verification_display': verification_status['should_display'], - 'verification_status': verification_status['status'], + 'verification_status': verification_status, 'verification_status_by_course': verify_status_by_course, 'verification_errors': verification_errors, 'block_courses': block_courses, diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 5203d749278ec5bef599f9bf425ed7627fae006b..b657f481600475ae5a7dbb1472f78a7435ef836d 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -598,9 +598,9 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): # inject verification status if verification_service: - verification_status = verification_service.get_status(user_id) + verification_status, __ = verification_service.get_status(user_id) context.update({ - 'verification_status': verification_status['status'], + 'verification_status': verification_status, 'reverify_url': verification_service.reverify_url(), }) diff --git a/lms/djangoapps/certificates/signals.py b/lms/djangoapps/certificates/signals.py index dc68c1ee21946c8aa582f21f0f2ef43b0fb4843f..589e0894ccce840eb12b26f0eb6ed4c60604dc6d 100644 --- a/lms/djangoapps/certificates/signals.py +++ b/lms/djangoapps/certificates/signals.py @@ -82,7 +82,7 @@ def _listen_for_id_verification_status_changed(sender, user, **kwargs): # pylin user_enrollments = CourseEnrollment.enrollments_for_user(user=user) grade_factory = CourseGradeFactory() - expected_verification_status = IDVerificationService.user_status(user) + expected_verification_status, _ = IDVerificationService.user_status(user) for enrollment in user_enrollments: if grade_factory.read(user=user, course=enrollment.course_overview).passed: if fire_ungenerated_certificate_task(user, enrollment.course_id, expected_verification_status): @@ -93,7 +93,7 @@ def _listen_for_id_verification_status_changed(sender, user, **kwargs): # pylin log.info(message.format( user=user.id, course=enrollment.course_id, - status=expected_verification_status['status'] + status=expected_verification_status )) diff --git a/lms/djangoapps/certificates/tasks.py b/lms/djangoapps/certificates/tasks.py index 537108cc880bcd9dc8d465a54fcc7c409504d40c..69fa1083c4c804b51416b56a871023e767469d5d 100644 --- a/lms/djangoapps/certificates/tasks.py +++ b/lms/djangoapps/certificates/tasks.py @@ -31,7 +31,7 @@ def generate_certificate(self, **kwargs): course_key = CourseKey.from_string(kwargs.pop('course_key')) expected_verification_status = kwargs.pop('expected_verification_status', None) if expected_verification_status: - actual_verification_status = IDVerificationService.user_status(student) + actual_verification_status, _ = IDVerificationService.user_status(student) if expected_verification_status != actual_verification_status: raise self.retry(kwargs=original_kwargs) generate_user_certificates(student=student, course_key=course_key, **kwargs) diff --git a/lms/djangoapps/certificates/tests/test_signals.py b/lms/djangoapps/certificates/tests/test_signals.py index 5c2deb051acbb0825b367e90057cb597d54bca72..8212b04fbf4f477569cad18b63fb25282189e447 100644 --- a/lms/djangoapps/certificates/tests/test_signals.py +++ b/lms/djangoapps/certificates/tests/test_signals.py @@ -256,17 +256,12 @@ class LearnerTrackChangeCertsTest(ModuleStoreTestCase): status='submitted' ) attempt.approve() - expected_verification_status = { - 'status': 'approved', - 'error': '', - 'should_display': True, - } mock_generate_certificate_apply_async.assert_called_with( countdown=CERTIFICATE_DELAY_SECONDS, kwargs={ 'student': unicode(self.user_one.id), 'course_key': unicode(self.course_one.id), - 'expected_verification_status': unicode(expected_verification_status), + 'expected_verification_status': SoftwareSecurePhotoVerification.STATUS.approved } ) @@ -282,17 +277,12 @@ class LearnerTrackChangeCertsTest(ModuleStoreTestCase): status='submitted' ) attempt.approve() - expected_verification_status = { - 'status': 'approved', - 'error': '', - 'should_display': True, - } mock_generate_certificate_apply_async.assert_called_with( countdown=CERTIFICATE_DELAY_SECONDS, kwargs={ 'student': unicode(self.user_two.id), 'course_key': unicode(self.course_two.id), - 'expected_verification_status': unicode(expected_verification_status), + 'expected_verification_status': SoftwareSecurePhotoVerification.STATUS.approved } ) diff --git a/lms/djangoapps/certificates/tests/test_tasks.py b/lms/djangoapps/certificates/tests/test_tasks.py index 75658079534fad4b24d2facd793d13ea82f68962..61d1db1712be9ac149b1b780cd24432f1d2475fd 100644 --- a/lms/djangoapps/certificates/tests/test_tasks.py +++ b/lms/djangoapps/certificates/tests/test_tasks.py @@ -45,22 +45,13 @@ class GenerateUserCertificateTest(TestCase): course_key = 'course-v1:edX+CS101+2017_T2' student = UserFactory() - expected_verification_status = { - 'status': 'approved', - 'error': '', - 'should_display': True, - } - kwargs = { 'student': student.id, 'course_key': course_key, - 'expected_verification_status': expected_verification_status, + 'expected_verification_status': 'approved' } - user_status_mock.side_effect = [ - {'status': 'pending', 'error': '', 'should_display': True}, - {'status': 'approved', 'error': '', 'should_display': True} - ] + user_status_mock.side_effect = [('pending', ''), ('approved', '')] generate_certificate.apply_async(kwargs=kwargs).get() diff --git a/lms/djangoapps/commerce/views.py b/lms/djangoapps/commerce/views.py index 0d059cf5dcc157aca8da181a26c503bf23e7e6e2..c5dea7da032ac37dada79bd84c47c54bd40d41f0 100644 --- a/lms/djangoapps/commerce/views.py +++ b/lms/djangoapps/commerce/views.py @@ -92,7 +92,7 @@ def checkout_receipt(request): 'page_title': page_title, 'is_payment_complete': is_payment_complete, 'platform_name': configuration_helpers.get_value('platform_name', settings.PLATFORM_NAME), - 'verified': IDVerificationService.user_has_valid_or_pending(request.user), + 'verified': IDVerificationService.verification_valid_or_pending(request.user).exists(), 'error_summary': error_summary, 'error_text': error_text, 'for_help_text': for_help_text, diff --git a/lms/djangoapps/courseware/date_summary.py b/lms/djangoapps/courseware/date_summary.py index 85dda380742f4147c28a2877637780403fc43c97..5fb7b51d1cb466b173c30ac3100a7e48f42c70ac 100644 --- a/lms/djangoapps/courseware/date_summary.py +++ b/lms/djangoapps/courseware/date_summary.py @@ -627,8 +627,8 @@ class VerificationDeadlineDate(DateSummary): @lazy def verification_status(self): """Return the verification status for this user.""" - verification_status = IDVerificationService.user_status(self.user) - return verification_status['status'] + status, _ = IDVerificationService.user_status(self.user) + return status def must_retry(self): """Return True if the user must re-submit verification, False otherwise.""" diff --git a/lms/djangoapps/verify_student/migrations/0008_populate_idverificationaggregate.py b/lms/djangoapps/verify_student/migrations/0008_populate_idverificationaggregate.py index ba4e5500d34d66a1b8a9fe5d77d42ad1d5071428..30825518cd363d25af69c762491c757b024874ba 100644 --- a/lms/djangoapps/verify_student/migrations/0008_populate_idverificationaggregate.py +++ b/lms/djangoapps/verify_student/migrations/0008_populate_idverificationaggregate.py @@ -7,25 +7,12 @@ from django.db import migrations def populate_id_verification_aggregate(apps, schema_editor): - ContentType = apps.get_model('contenttypes', 'ContentType') - IDVerificationAggregate = apps.get_model('verify_student', 'IDVerificationAggregate') - SoftwareSecurePhotoVerification = apps.get_model('verify_student', 'SoftwareSecurePhotoVerification') - SSOVerification = apps.get_model('verify_student', 'SSOVerification') - - software_secure_verifications = SoftwareSecurePhotoVerification.objects.all() - sso_verifications = SSOVerification.objects.all() - for verification in chain(software_secure_verifications, sso_verifications): - content_type = ContentType.objects.get_for_model(verification) - IDVerificationAggregate.objects.create( - status=verification.status, - user=verification.user, - name=verification.name, - created_at=verification.created_at, - updated_at=verification.updated_at, - content_type=content_type, - object_id=verification.id, - ) - + """ + The code from this migration was removed because it caused a spike in database errors + when it was run in the edX production environment. More details can be found here: + https://openedx.atlassian.net/browse/ENT-969 + """ + pass class Migration(migrations.Migration): diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index 5282f23025d5bfbc2c8f6fec47fcb9bb30d2a8e5..b2d51606b0181eec127e844e8b2993e7a8b40a8f 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -120,26 +120,6 @@ class IDVerificationAttempt(StatusModel): days_good_for = settings.VERIFY_STUDENT["DAYS_GOOD_FOR"] return self.created_at + timedelta(days=days_good_for) - def should_display_status_to_user(self): - """Whether or not the status from this attempt should be displayed to the user.""" - raise NotImplementedError - - def active_at_datetime(self, deadline): - """Check whether the verification was active at a particular datetime. - - Arguments: - deadline (datetime): The date at which the verification was active - (created before and expiration datetime is after today). - - Returns: - bool - - """ - return ( - self.created_at < deadline and - self.expiration_datetime > datetime.now(pytz.UTC) - ) - class IDVerificationAggregate(IDVerificationAttempt): """ @@ -166,27 +146,6 @@ class IDVerificationAggregate(IDVerificationAttempt): status=self.status, ) - def should_display_status_to_user(self): - """Whether or not the status from this attempt should be displayed to the user.""" - return self.content_object.should_display_status_to_user() - - -def post_save_id_verification(sender, instance, created, **kwargs): # pylint: disable=unused-argument - """ - Post save handler to create/update IDVerificationAttempt instances. - """ - content_type = ContentType.objects.get_for_model(instance) - try: - id_verification = IDVerificationAggregate.objects.get(content_type=content_type, object_id=instance.id) - except IDVerificationAggregate.DoesNotExist: - id_verification = IDVerificationAggregate(content_type=content_type, object_id=instance.id) - id_verification.status = instance.status - id_verification.user = instance.user - id_verification.name = instance.name - id_verification.created_at = instance.created_at - id_verification.updated_at = instance.updated_at - id_verification.save() - class SSOVerification(IDVerificationAttempt): """ @@ -229,12 +188,6 @@ class SSOVerification(IDVerificationAttempt): status=self.status, ) - def should_display_status_to_user(self): - """Whether or not the status from this attempt should be displayed to the user.""" - return False - -models.signals.post_save.connect(post_save_id_verification, sender=SSOVerification) - class PhotoVerification(IDVerificationAttempt, DeletableByUserValue): """ @@ -328,6 +281,22 @@ class PhotoVerification(IDVerificationAttempt, DeletableByUserValue): abstract = True ordering = ['-created_at'] + def active_at_datetime(self, deadline): + """Check whether the verification was active at a particular datetime. + + Arguments: + deadline (datetime): The date at which the verification was active + (created before and expiration datetime is after today). + + Returns: + bool + + """ + return ( + self.created_at < deadline and + self.expiration_datetime > datetime.now(pytz.UTC) + ) + def parsed_error_msg(self): """ Sometimes, the error message we've received needs to be parsed into @@ -880,12 +849,6 @@ class SoftwareSecurePhotoVerification(PhotoVerification): return response - def should_display_status_to_user(self): - """Whether or not the status from this attempt should be displayed to the user.""" - return True - -models.signals.post_save.connect(post_save_id_verification, sender=SoftwareSecurePhotoVerification) - class VerificationDeadline(TimeStampedModel): """ diff --git a/lms/djangoapps/verify_student/services.py b/lms/djangoapps/verify_student/services.py index 8fb50c67a9535a7a1ae73198986c7c69ec5bc3ab..0cfc20fa0a04360af11a30cbf16f78190fe9a958 100644 --- a/lms/djangoapps/verify_student/services.py +++ b/lms/djangoapps/verify_student/services.py @@ -12,7 +12,7 @@ from course_modes.models import CourseMode from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from student.models import User -from .models import IDVerificationAggregate +from .models import SoftwareSecurePhotoVerification from .utils import earliest_allowed_verification_date log = logging.getLogger(__name__) @@ -70,7 +70,7 @@ class IDVerificationService(object): that are still valid according to the earliest_allowed_date value or policy settings. """ - return IDVerificationAggregate.objects.filter( + return SoftwareSecurePhotoVerification.objects.filter( status="approved", created_at__gte=(earliest_allowed_date or earliest_allowed_verification_date()), ) @@ -80,7 +80,7 @@ class IDVerificationService(object): """ Return a query set for all records associated with the given user. """ - return IDVerificationAggregate.objects.filter(user=user) + return SoftwareSecurePhotoVerification.objects.filter(user=user) @classmethod def get_verified_users(cls, users): @@ -115,7 +115,7 @@ class IDVerificationService(object): valid_statuses = ['submitted', 'approved', 'must_retry'] if queryset is None: - queryset = IDVerificationAggregate.objects.filter(user=user) + queryset = SoftwareSecurePhotoVerification.objects.filter(user=user) return queryset.filter( status__in=valid_statuses, @@ -141,11 +141,11 @@ class IDVerificationService(object): verification. """ if queryset is None: - queryset = IDVerificationAggregate.objects.filter(user=user) + queryset = SoftwareSecurePhotoVerification.objects.filter(user=user) - id_verification = queryset.filter(status='approved').first() - if id_verification: - return id_verification.expiration_datetime + photo_verification = queryset.filter(status='approved').first() + if photo_verification: + return photo_verification.expiration_datetime @classmethod def user_has_valid_or_pending(cls, user, earliest_allowed_date=None, queryset=None): @@ -157,6 +157,26 @@ class IDVerificationService(object): """ return cls.verification_valid_or_pending(user, earliest_allowed_date, queryset).exists() + @classmethod + def active_for_user(cls, user): + """ + Return the most recent PhotoVerification that is marked ready (i.e. the + user has said they're set, but we haven't submitted anything yet). + + This checks for the original verification. + """ + # This should only be one at the most, but just in case we create more + # by mistake, we'll grab the most recently created one. + active_attempts = SoftwareSecurePhotoVerification.objects.filter( + user=user, + status='ready' + ).order_by('-created_at') + + if active_attempts: + return active_attempts[0] + else: + return None + @classmethod def user_status(cls, user): """ @@ -168,51 +188,46 @@ class IDVerificationService(object): If the verification process is still ongoing, returns 'pending' If the verification has been denied and the user must resubmit photos, returns 'must_reverify' - This checks most recent verification - """ - # should_display only refers to displaying the verification attempt status to a user - # once a verification attempt has been made, otherwise we will display a prompt to complete ID verification. - user_status = { - 'status': 'none', - 'error': '', - 'should_display': True, - } - - # We need to check the user's most recent attempt. - try: - attempts = IDVerificationAggregate.objects.filter(user=user).order_by('-updated_at') - attempt = attempts[0].content_object - except IndexError: - # The user has no verification attempts, return the default set of data. - return user_status - - user_status['should_display'] = attempt.should_display_status_to_user() - if attempt.created_at < earliest_allowed_verification_date(): - if user_status['should_display']: - user_status['status'] = 'expired' - user_status['error'] = _("Your {platform_name} verification has expired.").format( - platform_name=configuration_helpers.get_value('platform_name', settings.PLATFORM_NAME), - ) - else: - # If we have a verification attempt that never would have displayed to the user, - # and that attempt is expired, then we should treat it as if the user had never verified. - return user_status - - # If someone is denied their original verification attempt, they can try to reverify. - elif attempt.status == 'denied': - user_status['status'] = 'must_reverify' - if hasattr(attempt, 'error_msg') and attempt.error_msg: - user_status['error'] = attempt.parsed_error_msg() + This checks initial verifications + """ + status = 'none' + error_msg = '' - elif attempt.status == 'approved': - user_status['status'] = 'approved' + if cls.user_is_verified(user): + status = 'approved' - elif attempt.status in ['submitted', 'approved', 'must_retry']: + elif cls.user_has_valid_or_pending(user): # user_has_valid_or_pending does include 'approved', but if we are # here, we know that the attempt is still pending - user_status['status'] = 'pending' + status = 'pending' + + else: + # we need to check the most recent attempt to see if we need to ask them to do + # a retry + try: + attempts = SoftwareSecurePhotoVerification.objects.filter(user=user).order_by('-updated_at') + attempt = attempts[0] + except IndexError: + # we return 'none' + + return ('none', error_msg) + + if attempt.created_at < earliest_allowed_verification_date(): + return ( + 'expired', + _("Your {platform_name} verification has expired.").format( + platform_name=configuration_helpers.get_value('platform_name', settings.PLATFORM_NAME), + ) + ) + + # If someone is denied their original verification attempt, they can try to reverify. + if attempt.status == 'denied': + status = 'must_reverify' + + if attempt.error_msg: + error_msg = attempt.parsed_error_msg() - return user_status + return (status, error_msg) @classmethod def verification_status_for_user(cls, user, user_enrollment_mode, user_is_verified=None): diff --git a/lms/djangoapps/verify_student/tests/test_models.py b/lms/djangoapps/verify_student/tests/test_models.py index c7a52dfca7a70153f96a313babb3c3e843770af9..dde18aa15c95f7294822b93a9cd28fffa676175f 100644 --- a/lms/djangoapps/verify_student/tests/test_models.py +++ b/lms/djangoapps/verify_student/tests/test_models.py @@ -8,7 +8,6 @@ import mock import pytz import requests.exceptions from django.conf import settings -from django.test import TestCase from freezegun import freeze_time from mock import patch from nose.tools import ( # pylint: disable=no-name-in-module @@ -22,7 +21,6 @@ from testfixtures import LogCapture from common.test.utils import MockS3Mixin from lms.djangoapps.verify_student.models import ( SoftwareSecurePhotoVerification, - SSOVerification, VerificationDeadline, VerificationException ) @@ -97,38 +95,11 @@ def mock_software_secure_post_unavailable(url, headers=None, data=None, **kwargs raise requests.exceptions.ConnectionError -class TestVerification(TestCase): - """ - Common tests across all types of Verications (e.g., SoftwareSecurePhotoVerication, SSOVerification) - """ - def verification_active_at_datetime(self, attempt): - """ - Tests to ensure the Verification is active or inactive at the appropriate datetimes. - """ - # Not active before the created date - before = attempt.created_at - timedelta(seconds=1) - self.assertFalse(attempt.active_at_datetime(before)) - - # Active immediately after created date - after_created = attempt.created_at + timedelta(seconds=1) - self.assertTrue(attempt.active_at_datetime(after_created)) - - # Active immediately before expiration date - expiration = attempt.created_at + timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"]) - before_expiration = expiration - timedelta(seconds=1) - self.assertTrue(attempt.active_at_datetime(before_expiration)) - - # Not active after the expiration date - attempt.created_at = attempt.created_at - timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"]) - attempt.save() - self.assertFalse(attempt.active_at_datetime(datetime.now(pytz.UTC) + timedelta(days=1))) - - # Lots of patching to stub in our own settings, and HTTP posting @patch.dict(settings.VERIFY_STUDENT, FAKE_SETTINGS) @patch('lms.djangoapps.verify_student.models.requests.post', new=mock_software_secure_post) @ddt.ddt -class TestPhotoVerification(TestVerification, MockS3Mixin, ModuleStoreTestCase): +class TestPhotoVerification(MockS3Mixin, ModuleStoreTestCase): def setUp(self): super(TestPhotoVerification, self).setUp() @@ -281,7 +252,24 @@ class TestPhotoVerification(TestVerification, MockS3Mixin, ModuleStoreTestCase): def test_active_at_datetime(self): user = UserFactory.create() attempt = SoftwareSecurePhotoVerification.objects.create(user=user) - self.verification_active_at_datetime(attempt) + + # Not active before the created date + before = attempt.created_at - timedelta(seconds=1) + self.assertFalse(attempt.active_at_datetime(before)) + + # Active immediately after created date + after_created = attempt.created_at + timedelta(seconds=1) + self.assertTrue(attempt.active_at_datetime(after_created)) + + # Active immediately before expiration date + expiration = attempt.created_at + timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"]) + before_expiration = expiration - timedelta(seconds=1) + self.assertTrue(attempt.active_at_datetime(before_expiration)) + + # Not active after the expiration date + attempt.created_at = attempt.created_at - timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"]) + attempt.save() + self.assertFalse(attempt.active_at_datetime(datetime.now(pytz.UTC) + timedelta(days=1))) def test_initial_verification_for_user(self): """Test that method 'get_initial_verification' of model @@ -352,16 +340,6 @@ class TestPhotoVerification(TestVerification, MockS3Mixin, ModuleStoreTestCase): self.assertFalse(SoftwareSecurePhotoVerification.delete_by_user_value(user, "user")) -class SSOVerificationTest(TestVerification): - """ - Tests for the SSOVerification model - """ - def test_active_at_datetime(self): - user = UserFactory.create() - attempt = SSOVerification.objects.create(user=user) - self.verification_active_at_datetime(attempt) - - class VerificationDeadlineTest(CacheIsolationTestCase): """ Tests for the VerificationDeadline model. diff --git a/lms/djangoapps/verify_student/tests/test_services.py b/lms/djangoapps/verify_student/tests/test_services.py index 3aaaa2774f53008fb8bc15078e63455144c78728..7c2795334722e66eb8e4957eeb1310f386540de1 100644 --- a/lms/djangoapps/verify_student/tests/test_services.py +++ b/lms/djangoapps/verify_student/tests/test_services.py @@ -16,7 +16,7 @@ from nose.tools import ( ) from common.test.utils import MockS3Mixin -from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification, SSOVerification +from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification from lms.djangoapps.verify_student.services import IDVerificationService from student.tests.factories import UserFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -34,6 +34,52 @@ class TestIDVerificationService(MockS3Mixin, ModuleStoreTestCase): Tests for IDVerificationService. """ + def test_active_for_user(self): + """ + Make sure we can retrive a user's active (in progress) verification + attempt. + """ + user = UserFactory.create() + + # This user has no active at the moment... + assert_is_none(IDVerificationService.active_for_user(user)) + + # Create an attempt and mark it ready... + attempt = SoftwareSecurePhotoVerification(user=user) + attempt.mark_ready() + assert_equals(attempt, IDVerificationService.active_for_user(user)) + + # A new user won't see this... + user2 = UserFactory.create() + user2.save() + assert_is_none(IDVerificationService.active_for_user(user2)) + + # If it's got a different status, it doesn't count + for status in ["submitted", "must_retry", "approved", "denied"]: + attempt.status = status + attempt.save() + assert_is_none(IDVerificationService.active_for_user(user)) + + # But if we create yet another one and mark it ready, it passes again. + attempt_2 = SoftwareSecurePhotoVerification(user=user) + attempt_2.mark_ready() + assert_equals(attempt_2, IDVerificationService.active_for_user(user)) + + # And if we add yet another one with a later created time, we get that + # one instead. We always want the most recent attempt marked ready() + attempt_3 = SoftwareSecurePhotoVerification( + user=user, + created_at=attempt_2.created_at + timedelta(days=1) + ) + attempt_3.save() + + # We haven't marked attempt_3 ready yet, so attempt_2 still wins + assert_equals(attempt_2, IDVerificationService.active_for_user(user)) + + # Now we mark attempt_3 ready and expect it to come back + attempt_3.mark_ready() + assert_equals(attempt_3, IDVerificationService.active_for_user(user)) + def test_user_is_verified(self): """ Test to make sure we correctly answer whether a user has been verified. @@ -77,31 +123,26 @@ class TestIDVerificationService(MockS3Mixin, ModuleStoreTestCase): # test for correct status when no error returned user = UserFactory.create() status = IDVerificationService.user_status(user) - self.assertEquals(status, {'status': 'none', 'error': '', 'should_display': True}) + self.assertEquals(status, ('none', '')) - # test for when photo verification has been created - SoftwareSecurePhotoVerification.objects.create(user=user, status='approved') + # test for when one has been created + attempt = SoftwareSecurePhotoVerification.objects.create(user=user, status='approved') status = IDVerificationService.user_status(user) - self.assertEquals(status, {'status': 'approved', 'error': '', 'should_display': True}) + self.assertEquals(status, ('approved', '')) - # create another photo verification for the same user, make sure the denial - # is handled properly + # create another one for the same user, make sure the right one is + # returned SoftwareSecurePhotoVerification.objects.create( user=user, status='denied', error_msg='[{"photoIdReasons": ["Not provided"]}]' ) status = IDVerificationService.user_status(user) - self.assertEquals(status, {'status': 'must_reverify', 'error': ['id_image_missing'], 'should_display': True}) - - # test for when sso verification has been created - SSOVerification.objects.create(user=user, status='approved') - status = IDVerificationService.user_status(user) - self.assertEquals(status, {'status': 'approved', 'error': '', 'should_display': False}) + self.assertEquals(status, ('approved', '')) - # create another sso verification for the same user, make sure the denial - # is handled properly - SSOVerification.objects.create(user=user, status='denied') + # now delete the first one and verify that the denial is being handled + # properly + attempt.delete() status = IDVerificationService.user_status(user) - self.assertEquals(status, {'status': 'must_reverify', 'error': '', 'should_display': False}) + self.assertEquals(status, ('must_reverify', ['id_image_missing'])) @ddt.unpack @ddt.data( diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index f101293b67dbc9d540162756ed9e27f96062da7b..fb7886fa128ea94c7db0423cecd3439278bed780 100644 --- a/lms/djangoapps/verify_student/views.py +++ b/lms/djangoapps/verify_student/views.py @@ -1226,7 +1226,7 @@ class ReverifyView(View): Most of the work is done client-side by composing the same Backbone views used in the initial verification flow. """ - verification_status = IDVerificationService.user_status(request.user) + status, __ = IDVerificationService.user_status(request.user) expiration_datetime = IDVerificationService.get_expiration_datetime(request.user) can_reverify = False @@ -1243,7 +1243,7 @@ class ReverifyView(View): # A photo verification is marked as 'pending' if its status is either # 'submitted' or 'must_retry'. - if verification_status['status'] in ["none", "must_reverify", "expired", "pending"] or can_reverify: + if status in ["none", "must_reverify", "expired", "pending"] or can_reverify: context = { "user_full_name": request.user.profile.name, "platform_name": configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME), @@ -1252,6 +1252,6 @@ class ReverifyView(View): return render_to_response("verify_student/reverify.html", context) else: context = { - "status": verification_status['status'] + "status": status } return render_to_response("verify_student/reverify_not_allowed.html", context) diff --git a/lms/templates/dashboard/_dashboard_course_listing.html b/lms/templates/dashboard/_dashboard_course_listing.html index ff488593506204e2e3c86af5b961f3b63681183b..ca5525057793bd3f7adfca500db11aec0012e1b7 100644 --- a/lms/templates/dashboard/_dashboard_course_listing.html +++ b/lms/templates/dashboard/_dashboard_course_listing.html @@ -399,7 +399,7 @@ from util.course import get_link_for_about_page, get_encoded_course_sharing_utm_ <%include file="_dashboard_show_consent.html" args="course_overview=course_overview, course_target=course_target, enrollment=enrollment, enterprise_customer_name=enterprise_customer_name"/> %endif - % if verification_status.get('should_display') and verification_status.get('status') in [VERIFY_STATUS_NEED_TO_VERIFY, VERIFY_STATUS_SUBMITTED, VERIFY_STATUS_RESUBMITTED, VERIFY_STATUS_APPROVED, VERIFY_STATUS_NEED_TO_REVERIFY]: + % if verification_status.get('status') in [VERIFY_STATUS_NEED_TO_VERIFY, VERIFY_STATUS_SUBMITTED, VERIFY_STATUS_RESUBMITTED, VERIFY_STATUS_APPROVED, VERIFY_STATUS_NEED_TO_REVERIFY]: <div class="message message-status wrapper-message-primary is-shown"> % if verification_status['status'] == VERIFY_STATUS_NEED_TO_VERIFY: <div class="verification-reminder"> diff --git a/lms/templates/dashboard/_dashboard_status_verification.html b/lms/templates/dashboard/_dashboard_status_verification.html index 8ed8899a8f6cee55b6b1b8c97014e88c7ebde97c..2dce1a12aff2499e4d00044a53eebea31f7a5902 100644 --- a/lms/templates/dashboard/_dashboard_status_verification.html +++ b/lms/templates/dashboard/_dashboard_status_verification.html @@ -5,44 +5,42 @@ from django.core.urlresolvers import reverse from django.utils.translation import ugettext as _ %> -%if verification_display: - %if verification_status == 'approved': - <li class="status status-verification is-accepted"> - <span class="title status-title">${_("Current Verification Status: Approved")}</span> - <p class="status-note">${_("Your edX verification has been approved. Your verification is effective for one year after submission.")}</p> - </li> - %elif verification_status == 'pending': - <li class="status status-verification is-pending"> - <span class="title status-title">${_("Current Verification Status: Pending")}</span> - <p class="status-note">${_("Your edX ID verification is pending. Your verification information has been submitted and will be reviewed shortly.")}</p> - </li> - %elif verification_status in ['denied','must_reverify', 'must_retry']: - <li class="status status-verification is-denied"> - <span class="title status-title">${_("Current Verification Status: Denied")}</span> - <p class="status-note"> - ${_("Your verification submission was not accepted. To receive a verified certificate, you must submit a new photo of yourself and your government-issued photo ID before the verification deadline for your course.")} +%if verification_status == 'approved': + <li class="status status-verification is-accepted"> + <span class="title status-title">${_("Current Verification Status: Approved")}</span> + <p class="status-note">${_("Your edX verification has been approved. Your verification is effective for one year after submission.")}</p> + </li> +%elif verification_status == 'pending': + <li class="status status-verification is-pending"> + <span class="title status-title">${_("Current Verification Status: Pending")}</span> + <p class="status-note">${_("Your edX ID verification is pending. Your verification information has been submitted and will be reviewed shortly.")}</p> + </li> +%elif verification_status in ['denied','must_reverify', 'must_retry']: + <li class="status status-verification is-denied"> + <span class="title status-title">${_("Current Verification Status: Denied")}</span> + <p class="status-note"> + ${_("Your verification submission was not accepted. To receive a verified certificate, you must submit a new photo of yourself and your government-issued photo ID before the verification deadline for your course.")} - %if verification_errors: - <br><br> - ${_("Your verification was denied for the following reasons:")}<br> - <ul> - %for error in verification_errors: - <li>${error}</li> - %endfor - </ul> - %endif - </p> + %if verification_errors: + <br><br> + ${_("Your verification was denied for the following reasons:")}<br> + <ul> + %for error in verification_errors: + <li>${error}</li> + %endfor + </ul> + %endif + </p> + <div class="btn-reverify"> + <a href="${reverse('verify_student_reverify')}" class="action action-reverify">${_("Resubmit Verification")}</a> + </div> + </li> +%elif verification_status == 'expired': + <li class="status status-verification is-denied"> + <span class="title status-title">${_("Current Verification Status: Expired")}</span> + <p class="status-note">${_("Your verification has expired. To receive a verified certificate, you must submit a new photo of yourself and your government-issued photo ID before the verification deadline for your course.")}</p> <div class="btn-reverify"> <a href="${reverse('verify_student_reverify')}" class="action action-reverify">${_("Resubmit Verification")}</a> </div> - </li> - %elif verification_status == 'expired': - <li class="status status-verification is-denied"> - <span class="title status-title">${_("Current Verification Status: Expired")}</span> - <p class="status-note">${_("Your verification has expired. To receive a verified certificate, you must submit a new photo of yourself and your government-issued photo ID before the verification deadline for your course.")}</p> - <div class="btn-reverify"> - <a href="${reverse('verify_student_reverify')}" class="action action-reverify">${_("Resubmit Verification")}</a> - </div> - </li> - %endif + </li> %endif diff --git a/openedx/core/djangoapps/user_api/serializers.py b/openedx/core/djangoapps/user_api/serializers.py index ce954629d4841d1b1ad8ca465f0f5f4e14201bf6..c4dcf4fa3bdaaa58d64eb22246a34faa8430a5bc 100644 --- a/openedx/core/djangoapps/user_api/serializers.py +++ b/openedx/core/djangoapps/user_api/serializers.py @@ -5,7 +5,7 @@ from django.contrib.auth.models import User from django.utils.timezone import now from rest_framework import serializers -from lms.djangoapps.verify_student.models import IDVerificationAggregate +from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification from .models import UserPreference @@ -94,9 +94,9 @@ class CountryTimeZoneSerializer(serializers.Serializer): # pylint: disable=abst description = serializers.CharField() -class IDVerificationAggregateSerializer(serializers.ModelSerializer): +class SoftwareSecurePhotoVerificationSerializer(serializers.ModelSerializer): """ - Serializer that generates a representation of a user's ID verification status. + Serializer that generates a representation of a user's photo verification status. """ is_verified = serializers.SerializerMethodField() @@ -108,4 +108,4 @@ class IDVerificationAggregateSerializer(serializers.ModelSerializer): class Meta(object): fields = ('status', 'expiration_datetime', 'is_verified') - model = IDVerificationAggregate + model = SoftwareSecurePhotoVerification diff --git a/openedx/core/djangoapps/user_api/urls.py b/openedx/core/djangoapps/user_api/urls.py index 2ac78c660dcfe554c82ad9044a533e90502ae0e5..9ea27ec0776ade7caf38abf899290fbff8bc9cfb 100644 --- a/openedx/core/djangoapps/user_api/urls.py +++ b/openedx/core/djangoapps/user_api/urls.py @@ -14,7 +14,7 @@ from .accounts.views import ( DeactivateLogoutView ) from .preferences.views import PreferencesDetailView, PreferencesView -from .verification_api.views import IDVerificationStatusView +from .verification_api.views import PhotoVerificationStatusView from .validation.views import RegistrationValidationView ME = AccountViewSet.as_view({ @@ -81,7 +81,7 @@ urlpatterns = [ ), url( r'^v1/accounts/{}/verification_status/$'.format(settings.USERNAME_PATTERN), - IDVerificationStatusView.as_view(), + PhotoVerificationStatusView.as_view(), name='verification_status' ), url( diff --git a/openedx/core/djangoapps/user_api/verification_api/views.py b/openedx/core/djangoapps/user_api/verification_api/views.py index 6ec0c60995f68df3be7faf8e51cca15a2cba8f1b..5244915f8e30c260a7934013074986a0d8ca5403 100644 --- a/openedx/core/djangoapps/user_api/verification_api/views.py +++ b/openedx/core/djangoapps/user_api/verification_api/views.py @@ -5,20 +5,20 @@ from rest_framework.authentication import SessionAuthentication from rest_framework.generics import RetrieveAPIView from rest_framework_oauth.authentication import OAuth2Authentication -from lms.djangoapps.verify_student.models import IDVerificationAggregate -from openedx.core.djangoapps.user_api.serializers import IDVerificationAggregateSerializer +from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification +from openedx.core.djangoapps.user_api.serializers import SoftwareSecurePhotoVerificationSerializer from openedx.core.lib.api.permissions import IsStaffOrOwner -class IDVerificationStatusView(RetrieveAPIView): - """ IDVerificationStatus detail endpoint. """ +class PhotoVerificationStatusView(RetrieveAPIView): + """ PhotoVerificationStatus detail endpoint. """ authentication_classes = (JwtAuthentication, OAuth2Authentication, SessionAuthentication,) permission_classes = (IsStaffOrOwner,) - serializer_class = IDVerificationAggregateSerializer + serializer_class = SoftwareSecurePhotoVerificationSerializer def get_object(self): username = self.kwargs['username'] - verifications = IDVerificationAggregate.objects.filter(user__username=username).order_by('-updated_at') + verifications = SoftwareSecurePhotoVerification.objects.filter(user__username=username).order_by('-updated_at') if len(verifications) > 0: verification = verifications[0]