From 292f9ca83117ab4f26f16b8687e4f7c8dde489e0 Mon Sep 17 00:00:00 2001 From: Brittney Exline Date: Wed, 25 Apr 2018 15:03:26 -0400 Subject: [PATCH] Partial revert of "ENT-945 Update IDVerification interfaces to accountfor SSOVerification" This partially reverts commit ee1c3a454851df8a06f751501d1f32c0cd9a6432. The migration files introduced by the commit have been kept since they have been run already on several enviornments. --- common/djangoapps/student/helpers.py | 6 +- common/djangoapps/student/views/dashboard.py | 11 +- common/lib/xmodule/xmodule/seq_module.py | 4 +- lms/djangoapps/certificates/signals.py | 4 +- lms/djangoapps/certificates/tasks.py | 2 +- .../certificates/tests/test_signals.py | 14 +-- .../certificates/tests/test_tasks.py | 13 +-- lms/djangoapps/commerce/views.py | 2 +- lms/djangoapps/courseware/date_summary.py | 4 +- .../0008_populate_idverificationaggregate.py | 25 +--- lms/djangoapps/verify_student/models.py | 69 +++-------- lms/djangoapps/verify_student/services.py | 109 ++++++++++-------- .../verify_student/tests/test_models.py | 60 +++------- .../verify_student/tests/test_services.py | 75 +++++++++--- lms/djangoapps/verify_student/views.py | 6 +- .../dashboard/_dashboard_course_listing.html | 2 +- .../_dashboard_status_verification.html | 70 ++++++----- .../core/djangoapps/user_api/serializers.py | 8 +- openedx/core/djangoapps/user_api/urls.py | 4 +- .../user_api/verification_api/views.py | 12 +- 20 files changed, 228 insertions(+), 272 deletions(-) diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index fe61d7d24a..9fba0e8010 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 78e2b4bd2c..805ba14b74 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 5203d74927..b657f48160 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 dc68c1ee21..589e0894cc 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 537108cc88..69fa1083c4 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 5c2deb051a..8212b04fbf 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 7565807953..61d1db1712 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 0d059cf5dc..c5dea7da03 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 85dda38074..5fb7b51d1c 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 ba4e5500d3..30825518cd 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 5282f23025..b2d51606b0 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 8fb50c67a9..0cfc20fa0a 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 + This checks initial verifications """ - # 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, - } + status = 'none' + error_msg = '' - # 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 + if cls.user_is_verified(user): + status = 'approved' - 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() - - elif attempt.status == 'approved': - user_status['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' - return user_status + 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 (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 c7a52dfca7..dde18aa15c 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 3aaaa2774f..7c27953347 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}) + self.assertEquals(status, ('approved', '')) - # test for when sso verification has been created - SSOVerification.objects.create(user=user, status='approved') + # 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': 'approved', 'error': '', 'should_display': False}) - - # create another sso verification for the same user, make sure the denial - # is handled properly - SSOVerification.objects.create(user=user, status='denied') - 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 f101293b67..fb7886fa12 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 ff48859350..ca55250577 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]:
% if verification_status['status'] == VERIFY_STATUS_NEED_TO_VERIFY:
diff --git a/lms/templates/dashboard/_dashboard_status_verification.html b/lms/templates/dashboard/_dashboard_status_verification.html index 8ed8899a8f..2dce1a12af 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': -
  • - ${_("Current Verification Status: Approved")} -

    ${_("Your edX verification has been approved. Your verification is effective for one year after submission.")}

    -
    • - %elif verification_status == 'pending': -
  • - ${_("Current Verification Status: Pending")} -

    ${_("Your edX ID verification is pending. Your verification information has been submitted and will be reviewed shortly.")}

    -
    • - %elif verification_status in ['denied','must_reverify', 'must_retry']: -
  • - ${_("Current Verification Status: Denied")} -

    - ${_("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': +

  • + ${_("Current Verification Status: Approved")} +

    ${_("Your edX verification has been approved. Your verification is effective for one year after submission.")}

    +
    • +%elif verification_status == 'pending': +
  • + ${_("Current Verification Status: Pending")} +

    ${_("Your edX ID verification is pending. Your verification information has been submitted and will be reviewed shortly.")}

    +
    • +%elif verification_status in ['denied','must_reverify', 'must_retry']: +
  • + ${_("Current Verification Status: Denied")} +

    + ${_("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: -

    - ${_("Your verification was denied for the following reasons:")}
    -

      - %for error in verification_errors: -
    • ${error}
      • - %endfor -
    - %endif -

    + %if verification_errors: +

    + ${_("Your verification was denied for the following reasons:")}
    +
      + %for error in verification_errors: +
    • ${error}
      • + %endfor +
    + %endif +

    +
    + ${_("Resubmit Verification")} +
    +
    • +%elif verification_status == 'expired': +
  • + ${_("Current Verification Status: Expired")} +

    ${_("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.")}

    ${_("Resubmit Verification")}
    -
    • - %elif verification_status == 'expired': -
  • - ${_("Current Verification Status: Expired")} -

    ${_("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.")}

    -
    - ${_("Resubmit Verification")} -
    -
    • - %endif + %endif diff --git a/openedx/core/djangoapps/user_api/serializers.py b/openedx/core/djangoapps/user_api/serializers.py index ce954629d4..c4dcf4fa3b 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 2ac78c660d..9ea27ec077 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 6ec0c60995..5244915f8e 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]