diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index 1aea3a6a4c..912fa5453a 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -163,14 +163,20 @@ def check_verify_status_by_course(user, course_enrollment_pairs, all_course_mode * status (str): One of the enumerated status codes. * days_until_deadline (int): Number of days until the verification deadline. * verification_good_until (str): Date string for the verification expiration date. - """ + """ status_by_course = {} # Retrieve all verifications for the user, sorted in descending # order by submission datetime verifications = SoftwareSecurePhotoVerification.objects.filter(user=user) + # Check whether the user has an active or pending verification attempt + # To avoid another database hit, we re-use the queryset we have already retrieved. + has_active_or_pending = SoftwareSecurePhotoVerification.user_has_valid_or_pending( + user, queryset=verifications + ) + for course, enrollment in course_enrollment_pairs: # Get the verified mode (if any) for this course @@ -210,7 +216,13 @@ def check_verify_status_by_course(user, course_enrollment_pairs, all_course_mode ) if status is None and not submitted: if deadline is None or deadline > datetime.now(UTC): - status = VERIFY_STATUS_NEED_TO_VERIFY + # If a user already has an active or pending verification, + # but it will expire by the deadline, the we do NOT show the + # verification message. This is because we don't currently + # allow users to resubmit an attempt before their current + # attempt expires. + if not has_active_or_pending: + status = VERIFY_STATUS_NEED_TO_VERIFY else: status = VERIFY_STATUS_MISSED_DEADLINE diff --git a/common/djangoapps/student/tests/test_verification_status.py b/common/djangoapps/student/tests/test_verification_status.py index 32b3317bbd..633762a3c2 100644 --- a/common/djangoapps/student/tests/test_verification_status.py +++ b/common/djangoapps/student/tests/test_verification_status.py @@ -199,6 +199,26 @@ class TestCourseVerificationStatus(UrlResetMixin, ModuleStoreTestCase): # messaging relating to verification self._assert_course_verification_status(None) + def test_verification_will_expire_by_deadline(self): + # Expiration date in the future + self._setup_mode_and_enrollment(self.FUTURE, "verified") + + # Create a verification attempt that: + # 1) Is current (submitted in the last year) + # 2) Will expire by the deadline for the course + attempt = SoftwareSecurePhotoVerification.objects.create(user=self.user) + attempt.mark_ready() + attempt.submit() + + # This attempt will expire tomorrow, before the course deadline + attempt.created_at = attempt.created_at - timedelta(days=364) + attempt.save() + + # Expect that the "verify now" message is hidden + # (since the user isn't allowed to submit another attempt while + # a verification is active). + self._assert_course_verification_status(None) + def _setup_mode_and_enrollment(self, deadline, enrollment_mode): """Create a course mode and enrollment. @@ -227,9 +247,12 @@ class TestCourseVerificationStatus(UrlResetMixin, ModuleStoreTestCase): } NOTIFICATION_MESSAGES = { - VERIFY_STATUS_NEED_TO_VERIFY: "You still need to verify for this course.", - VERIFY_STATUS_SUBMITTED: "Thanks for your patience as we process your request.", - VERIFY_STATUS_APPROVED: "You have already verified your ID!", + VERIFY_STATUS_NEED_TO_VERIFY: [ + "You still need to verify for this course.", + "Verification not yet complete" + ], + VERIFY_STATUS_SUBMITTED: ["Thanks for your patience as we process your request."], + VERIFY_STATUS_APPROVED: ["You have already verified your ID!"], } def _assert_course_verification_status(self, status): @@ -254,7 +277,25 @@ class TestCourseVerificationStatus(UrlResetMixin, ModuleStoreTestCase): # Verify that the correct copy is rendered on the dashboard if status is not None: if status in self.NOTIFICATION_MESSAGES: - self.assertContains(response, self.NOTIFICATION_MESSAGES[status]) + # Different states might have different messaging + # so in some cases we check several possibilities + # and fail if none of these are found. + found_msg = False + for message in self.NOTIFICATION_MESSAGES[status]: + if message in response.content: + found_msg = True + break + + fail_msg = "Could not find any of these messages: {expected}".format( + expected=self.NOTIFICATION_MESSAGES[status] + ) + self.assertTrue(found_msg, msg=fail_msg) else: - for msg in self.NOTIFICATION_MESSAGES.values(): + # Combine all possible messages into a single list + all_messages = [] + for msg_group in self.NOTIFICATION_MESSAGES.values(): + all_messages.extend(msg_group) + + # Verify that none of the messages are displayed + for msg in all_messages: self.assertNotContains(response, msg) diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index 4a73d8fa94..18ac4313d9 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -211,7 +211,7 @@ class PhotoVerification(StatusModel): ).exists() @classmethod - def user_has_valid_or_pending(cls, user, earliest_allowed_date=None, window=None): + def user_has_valid_or_pending(cls, user, earliest_allowed_date=None, window=None, queryset=None): """ Return whether the user has a complete verification attempt that is or *might* be good. This means that it's approved, been submitted, or would @@ -222,15 +222,21 @@ class PhotoVerification(StatusModel): If window=None, this will check for the user's *initial* verification. If window is anything else, this will check for the reverification associated with that window. + + If a queryset is provided, that will be used instead of hitting the database. """ valid_statuses = ['submitted', 'approved'] if not window: valid_statuses.append('must_retry') - return cls.objects.filter( - user=user, + if queryset is None: + queryset = cls.objects.filter(user=user) + + return queryset.filter( status__in=valid_statuses, - created_at__gte=(earliest_allowed_date - or cls._earliest_allowed_date()), + created_at__gte=( + earliest_allowed_date + or cls._earliest_allowed_date() + ), window=window, ).exists()