diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index 9fba0e8010..fe61d7d24a 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -154,9 +154,12 @@ 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 @@ -214,7 +217,8 @@ def check_verify_status_by_course(user, course_enrollments): status_by_course[enrollment.course_id] = { 'status': status, - 'days_until_deadline': days_until_deadline + 'days_until_deadline': days_until_deadline, + 'should_display': should_display, } if recent_verification_datetime: diff --git a/common/djangoapps/student/views/dashboard.py b/common/djangoapps/student/views/dashboard.py index 805ba14b74..78e2b4bd2c 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, verification_error_codes = IDVerificationService.user_status(user) - verification_errors = get_verification_error_reasons_for_display(verification_error_codes) + verification_status = IDVerificationService.user_status(user) + verification_errors = get_verification_error_reasons_for_display(verification_status['error']) # Gets data for midcourse reverifications, if any are necessary or have failed statuses = ["approved", "denied", "pending", "must_reverify"] @@ -770,7 +770,9 @@ 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 in valid_verification_statuses + display_sidebar_on_dashboard = (len(order_history_list) or + (verification_status['status'] in valid_verification_statuses and + verification_status['should_display'])) # 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]: @@ -802,7 +804,8 @@ def student_dashboard(request): 'credit_statuses': _credit_statuses(user, course_enrollments), 'show_email_settings_for': show_email_settings_for, 'reverifications': reverifications, - 'verification_status': verification_status, + 'verification_display': verification_status['should_display'], + 'verification_status': verification_status['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 b657f48160..5203d74927 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, + 'verification_status': verification_status['status'], 'reverify_url': verification_service.reverify_url(), }) diff --git a/lms/djangoapps/certificates/signals.py b/lms/djangoapps/certificates/signals.py index 589e0894cc..dc68c1ee21 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=expected_verification_status['status'] )) diff --git a/lms/djangoapps/certificates/tasks.py b/lms/djangoapps/certificates/tasks.py index 69fa1083c4..537108cc88 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 8212b04fbf..5c2deb051a 100644 --- a/lms/djangoapps/certificates/tests/test_signals.py +++ b/lms/djangoapps/certificates/tests/test_signals.py @@ -256,12 +256,17 @@ 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': SoftwareSecurePhotoVerification.STATUS.approved + 'expected_verification_status': unicode(expected_verification_status), } ) @@ -277,12 +282,17 @@ 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': SoftwareSecurePhotoVerification.STATUS.approved + 'expected_verification_status': unicode(expected_verification_status), } ) diff --git a/lms/djangoapps/certificates/tests/test_tasks.py b/lms/djangoapps/certificates/tests/test_tasks.py index 61d1db1712..7565807953 100644 --- a/lms/djangoapps/certificates/tests/test_tasks.py +++ b/lms/djangoapps/certificates/tests/test_tasks.py @@ -45,13 +45,22 @@ 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': 'approved' + 'expected_verification_status': expected_verification_status, } - user_status_mock.side_effect = [('pending', ''), ('approved', '')] + user_status_mock.side_effect = [ + {'status': 'pending', 'error': '', 'should_display': True}, + {'status': 'approved', 'error': '', 'should_display': True} + ] generate_certificate.apply_async(kwargs=kwargs).get() diff --git a/lms/djangoapps/commerce/views.py b/lms/djangoapps/commerce/views.py index c5dea7da03..0d059cf5dc 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.verification_valid_or_pending(request.user).exists(), + 'verified': IDVerificationService.user_has_valid_or_pending(request.user), '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 5fb7b51d1c..85dda38074 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.""" - status, _ = IDVerificationService.user_status(self.user) - return status + verification_status = IDVerificationService.user_status(self.user) + return verification_status['status'] def must_retry(self): """Return True if the user must re-submit verification, False otherwise.""" diff --git a/lms/djangoapps/verify_student/migrations/0007_idverificationaggregate.py b/lms/djangoapps/verify_student/migrations/0007_idverificationaggregate.py new file mode 100644 index 0000000000..2722f0e2d9 --- /dev/null +++ b/lms/djangoapps/verify_student/migrations/0007_idverificationaggregate.py @@ -0,0 +1,38 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.12 on 2018-04-12 15:22 +from __future__ import unicode_literals + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import model_utils.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ('contenttypes', '0002_remove_content_type_name'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('verify_student', '0006_ssoverification'), + ] + + operations = [ + migrations.CreateModel( + name='IDVerificationAggregate', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('status', model_utils.fields.StatusField(choices=[(b'created', b'created'), (b'ready', b'ready'), (b'submitted', b'submitted'), (b'must_retry', b'must_retry'), (b'approved', b'approved'), (b'denied', b'denied')], default=b'created', max_length=100, no_check_for_status=True, verbose_name='status')), + ('status_changed', model_utils.fields.MonitorField(default=django.utils.timezone.now, monitor='status', verbose_name='status changed')), + ('name', models.CharField(blank=True, max_length=255)), + ('object_id', models.PositiveIntegerField()), + ('created_at', models.DateTimeField(db_index=True)), + ('updated_at', models.DateTimeField(db_index=True)), + ('content_type', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='contenttypes.ContentType')), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + options={ + 'ordering': ['-created_at'], + }, + ), + ] diff --git a/lms/djangoapps/verify_student/migrations/0008_populate_idverificationaggregate.py b/lms/djangoapps/verify_student/migrations/0008_populate_idverificationaggregate.py new file mode 100644 index 0000000000..ba4e5500d3 --- /dev/null +++ b/lms/djangoapps/verify_student/migrations/0008_populate_idverificationaggregate.py @@ -0,0 +1,38 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.12 on 2018-04-11 19:15 +from __future__ import unicode_literals + +from itertools import chain +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, + ) + + +class Migration(migrations.Migration): + + dependencies = [ + ('verify_student', '0007_idverificationaggregate'), + ] + + operations = [ + migrations.RunPython(populate_id_verification_aggregate, reverse_code=migrations.RunPython.noop), + ] diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index 77a7c880f8..a57221c683 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -21,6 +21,8 @@ import requests import six from django.conf import settings from django.contrib.auth.models import User +from django.contrib.contenttypes.fields import GenericForeignKey +from django.contrib.contenttypes.models import ContentType from django.core.cache import cache from django.core.files.base import ContentFile from django.core.urlresolvers import reverse @@ -117,6 +119,73 @@ 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): + """ + IDVerificationAggregate is the source of truth for all instances of IDVerificationAttempt. This + includes all types of verification, including PhotoVerification and SSOVerification. A generic + relation is used to refer to the appropriate Model object. + """ + content_type = models.ForeignKey(ContentType) + object_id = models.PositiveIntegerField() + content_object = GenericForeignKey('content_type', 'object_id') + + # override these fields so we can set the value + created_at = models.DateTimeField(db_index=True) + updated_at = models.DateTimeField(db_index=True) + + class Meta(object): + app_label = "verify_student" + ordering = ['-created_at'] + + def __unicode__(self): + return 'IDVerificationAggregate for {name} - type: {type}, status: {status}'.format( + name=self.name, + type=self.content_type, + 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): """ @@ -159,6 +228,12 @@ 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): """ @@ -252,22 +327,6 @@ class PhotoVerification(IDVerificationAttempt): 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 @@ -820,6 +879,12 @@ 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 0cfc20fa0a..8fb50c67a9 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 SoftwareSecurePhotoVerification +from .models import IDVerificationAggregate 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 SoftwareSecurePhotoVerification.objects.filter( + return IDVerificationAggregate.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 SoftwareSecurePhotoVerification.objects.filter(user=user) + return IDVerificationAggregate.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 = SoftwareSecurePhotoVerification.objects.filter(user=user) + queryset = IDVerificationAggregate.objects.filter(user=user) return queryset.filter( status__in=valid_statuses, @@ -141,11 +141,11 @@ class IDVerificationService(object): verification. """ if queryset is None: - queryset = SoftwareSecurePhotoVerification.objects.filter(user=user) + queryset = IDVerificationAggregate.objects.filter(user=user) - photo_verification = queryset.filter(status='approved').first() - if photo_verification: - return photo_verification.expiration_datetime + id_verification = queryset.filter(status='approved').first() + if id_verification: + return id_verification.expiration_datetime @classmethod def user_has_valid_or_pending(cls, user, earliest_allowed_date=None, queryset=None): @@ -157,26 +157,6 @@ 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): """ @@ -188,46 +168,51 @@ 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 initial verifications + This checks most recent verification """ - status = 'none' - error_msg = '' + # 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, + } - if cls.user_is_verified(user): - status = 'approved' + # 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 - elif cls.user_has_valid_or_pending(user): + 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']: # user_has_valid_or_pending does include 'approved', but if we are # here, we know that the attempt is still pending - status = 'pending' + user_status['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 (status, error_msg) + return user_status @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 fab3bd0551..26e65f54f9 100644 --- a/lms/djangoapps/verify_student/tests/test_models.py +++ b/lms/djangoapps/verify_student/tests/test_models.py @@ -8,6 +8,7 @@ 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 @@ -21,6 +22,7 @@ from testfixtures import LogCapture from common.test.utils import MockS3Mixin from lms.djangoapps.verify_student.models import ( SoftwareSecurePhotoVerification, + SSOVerification, VerificationDeadline, VerificationException ) @@ -95,11 +97,38 @@ 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(MockS3Mixin, ModuleStoreTestCase): +class TestPhotoVerification(TestVerification, MockS3Mixin, ModuleStoreTestCase): def setUp(self): super(TestPhotoVerification, self).setUp() @@ -252,24 +281,7 @@ class TestPhotoVerification(MockS3Mixin, ModuleStoreTestCase): def test_active_at_datetime(self): user = UserFactory.create() attempt = SoftwareSecurePhotoVerification.objects.create(user=user) - - # 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))) + self.verification_active_at_datetime(attempt) def test_initial_verification_for_user(self): """Test that method 'get_initial_verification' of model @@ -319,6 +331,16 @@ class TestPhotoVerification(MockS3Mixin, ModuleStoreTestCase): self.assertEqual(fourth_result, first_result) +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 7c27953347..3aaaa2774f 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 +from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification, SSOVerification from lms.djangoapps.verify_student.services import IDVerificationService from student.tests.factories import UserFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -34,52 +34,6 @@ 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. @@ -123,26 +77,31 @@ class TestIDVerificationService(MockS3Mixin, ModuleStoreTestCase): # test for correct status when no error returned user = UserFactory.create() status = IDVerificationService.user_status(user) - self.assertEquals(status, ('none', '')) + self.assertEquals(status, {'status': 'none', 'error': '', 'should_display': True}) - # test for when one has been created - attempt = SoftwareSecurePhotoVerification.objects.create(user=user, status='approved') + # test for when photo verification has been created + SoftwareSecurePhotoVerification.objects.create(user=user, status='approved') status = IDVerificationService.user_status(user) - self.assertEquals(status, ('approved', '')) + self.assertEquals(status, {'status': 'approved', 'error': '', 'should_display': True}) - # create another one for the same user, make sure the right one is - # returned + # create another photo verification for the same user, make sure the denial + # is handled properly SoftwareSecurePhotoVerification.objects.create( user=user, status='denied', error_msg='[{"photoIdReasons": ["Not provided"]}]' ) status = IDVerificationService.user_status(user) - self.assertEquals(status, ('approved', '')) + self.assertEquals(status, {'status': 'must_reverify', 'error': ['id_image_missing'], 'should_display': True}) - # now delete the first one and verify that the denial is being handled - # properly - attempt.delete() + # test for when sso verification has been created + SSOVerification.objects.create(user=user, status='approved') status = IDVerificationService.user_status(user) - self.assertEquals(status, ('must_reverify', ['id_image_missing'])) + 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}) @ddt.unpack @ddt.data( diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index fb7886fa12..f101293b67 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. """ - status, __ = IDVerificationService.user_status(request.user) + verification_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 status in ["none", "must_reverify", "expired", "pending"] or can_reverify: + if verification_status['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": status + "status": verification_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 ca55250577..ff48859350 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('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('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['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 2dce1a12af..8ed8899a8f 100644 --- a/lms/templates/dashboard/_dashboard_status_verification.html +++ b/lms/templates/dashboard/_dashboard_status_verification.html @@ -5,42 +5,44 @@ from django.core.urlresolvers import reverse from django.utils.translation import ugettext as _ %> -%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_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_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.")}

    + %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")} +
    +
    • + %endif %endif diff --git a/openedx/core/djangoapps/user_api/serializers.py b/openedx/core/djangoapps/user_api/serializers.py index c4dcf4fa3b..ce954629d4 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 SoftwareSecurePhotoVerification +from lms.djangoapps.verify_student.models import IDVerificationAggregate from .models import UserPreference @@ -94,9 +94,9 @@ class CountryTimeZoneSerializer(serializers.Serializer): # pylint: disable=abst description = serializers.CharField() -class SoftwareSecurePhotoVerificationSerializer(serializers.ModelSerializer): +class IDVerificationAggregateSerializer(serializers.ModelSerializer): """ - Serializer that generates a representation of a user's photo verification status. + Serializer that generates a representation of a user's ID verification status. """ is_verified = serializers.SerializerMethodField() @@ -108,4 +108,4 @@ class SoftwareSecurePhotoVerificationSerializer(serializers.ModelSerializer): class Meta(object): fields = ('status', 'expiration_datetime', 'is_verified') - model = SoftwareSecurePhotoVerification + model = IDVerificationAggregate diff --git a/openedx/core/djangoapps/user_api/urls.py b/openedx/core/djangoapps/user_api/urls.py index 2c9bb31a6f..e6348814a7 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 PhotoVerificationStatusView +from .verification_api.views import IDVerificationStatusView from .validation.views import RegistrationValidationView ME = AccountViewSet.as_view({ @@ -81,7 +81,7 @@ urlpatterns = [ ), url( r'^v1/accounts/{}/verification_status/$'.format(settings.USERNAME_PATTERN), - PhotoVerificationStatusView.as_view(), + IDVerificationStatusView.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 5244915f8e..6ec0c60995 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 SoftwareSecurePhotoVerification -from openedx.core.djangoapps.user_api.serializers import SoftwareSecurePhotoVerificationSerializer +from lms.djangoapps.verify_student.models import IDVerificationAggregate +from openedx.core.djangoapps.user_api.serializers import IDVerificationAggregateSerializer from openedx.core.lib.api.permissions import IsStaffOrOwner -class PhotoVerificationStatusView(RetrieveAPIView): - """ PhotoVerificationStatus detail endpoint. """ +class IDVerificationStatusView(RetrieveAPIView): + """ IDVerificationStatus detail endpoint. """ authentication_classes = (JwtAuthentication, OAuth2Authentication, SessionAuthentication,) permission_classes = (IsStaffOrOwner,) - serializer_class = SoftwareSecurePhotoVerificationSerializer + serializer_class = IDVerificationAggregateSerializer def get_object(self): username = self.kwargs['username'] - verifications = SoftwareSecurePhotoVerification.objects.filter(user__username=username).order_by('-updated_at') + verifications = IDVerificationAggregate.objects.filter(user__username=username).order_by('-updated_at') if len(verifications) > 0: verification = verifications[0]