From dac0309b0ff590ebaba5cf52e8fe518bd0c38270 Mon Sep 17 00:00:00 2001 From: Isaac Lee <124631592+ilee2u@users.noreply.github.com> Date: Thu, 26 Sep 2024 16:58:01 -0400 Subject: [PATCH] fix: add should_display_status_to_user method and status_changed field to VerificationAttempt model (#35514) * fix: add placeholder should_display_status_to_user * fix: have VerificationAttempt inherit StatusModel - should_display_status_to_user now returns False * chore: makemigrations * feat: status_changed field added * temp: idea to add should_display_status_to_user * feat: add should_display_status_to_user * fix: correct call in helpers+services * chore: lint+test fix * fix: default hide_status_user as False * chore: rename field call to STATUS * chore: remove extra status field - comment cleanup * temp: lint + comment out created status for now * fix: revamp status_changed for back-compat * fix: override save for status_changed * fix: replace created/updated instead of status - also made migrations * fix: squash commits - also remove extra updated_at property * chore: nits --- lms/djangoapps/verify_student/api.py | 11 +++- ...ve_verificationattempt_created_and_more.py | 50 +++++++++++++++++++ lms/djangoapps/verify_student/models.py | 24 ++++++--- lms/djangoapps/verify_student/services.py | 4 +- 4 files changed, 77 insertions(+), 12 deletions(-) create mode 100644 lms/djangoapps/verify_student/migrations/0016_remove_verificationattempt_created_and_more.py diff --git a/lms/djangoapps/verify_student/api.py b/lms/djangoapps/verify_student/api.py index 941dd60453..7b8310fde0 100644 --- a/lms/djangoapps/verify_student/api.py +++ b/lms/djangoapps/verify_student/api.py @@ -54,7 +54,13 @@ def send_approval_email(attempt): send_verification_approved_email(context=email_context) -def create_verification_attempt(user: User, name: str, status: str, expiration_datetime: Optional[datetime] = None): +def create_verification_attempt( + user: User, + name: str, + status: str, + expiration_datetime: Optional[datetime] = None, + hide_status_from_user: Optional[bool] = False, +): """ Create a verification attempt. @@ -74,6 +80,7 @@ def create_verification_attempt(user: User, name: str, status: str, expiration_d name=name, status=status, expiration_datetime=expiration_datetime, + hide_status_from_user=hide_status_from_user, ) emit_idv_attempt_created_event( @@ -129,7 +136,7 @@ def update_verification_attempt( 'Status must be one of: %(status_list)s', { 'status': status, - 'status_list': VerificationAttempt.STATUS_CHOICES, + 'status_list': VerificationAttempt.STATUS, }, ) raise VerificationAttemptInvalidStatus diff --git a/lms/djangoapps/verify_student/migrations/0016_remove_verificationattempt_created_and_more.py b/lms/djangoapps/verify_student/migrations/0016_remove_verificationattempt_created_and_more.py new file mode 100644 index 0000000000..d972dba3db --- /dev/null +++ b/lms/djangoapps/verify_student/migrations/0016_remove_verificationattempt_created_and_more.py @@ -0,0 +1,50 @@ +# Generated by Django 4.2.15 on 2024-09-26 20:08 + +from django.db import migrations, models +import django.utils.timezone +import lms.djangoapps.verify_student.statuses +import model_utils.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ('verify_student', '0015_verificationattempt'), + ] + + operations = [ + migrations.RemoveField( + model_name='verificationattempt', + name='created', + ), + migrations.RemoveField( + model_name='verificationattempt', + name='modified', + ), + migrations.AddField( + model_name='verificationattempt', + name='created_at', + field=models.DateTimeField(auto_now_add=True, db_index=True, default=django.utils.timezone.now), + preserve_default=False, + ), + migrations.AddField( + model_name='verificationattempt', + name='hide_status_from_user', + field=models.BooleanField(default=False, null=True), + ), + migrations.AddField( + model_name='verificationattempt', + name='status_changed', + field=model_utils.fields.MonitorField(default=django.utils.timezone.now, monitor='status', verbose_name='status changed'), + ), + migrations.AddField( + model_name='verificationattempt', + name='updated_at', + field=models.DateTimeField(auto_now=True, db_index=True), + ), + migrations.AlterField( + model_name='verificationattempt', + name='status', + field=model_utils.fields.StatusField(choices=[(lms.djangoapps.verify_student.statuses.VerificationAttemptStatus['CREATED'], lms.djangoapps.verify_student.statuses.VerificationAttemptStatus['CREATED']), (lms.djangoapps.verify_student.statuses.VerificationAttemptStatus['PENDING'], lms.djangoapps.verify_student.statuses.VerificationAttemptStatus['PENDING']), (lms.djangoapps.verify_student.statuses.VerificationAttemptStatus['APPROVED'], lms.djangoapps.verify_student.statuses.VerificationAttemptStatus['APPROVED']), (lms.djangoapps.verify_student.statuses.VerificationAttemptStatus['DENIED'], lms.djangoapps.verify_student.statuses.VerificationAttemptStatus['DENIED'])], default=lms.djangoapps.verify_student.statuses.VerificationAttemptStatus['CREATED'], max_length=100, no_check_for_status=True, verbose_name='status'), + ), + ] diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index 23729c99a0..9a0ac36964 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -15,9 +15,11 @@ import json import logging import os.path import uuid + from datetime import timedelta from email.utils import formatdate + import requests from config_models.models import ConfigurationModel from django.conf import settings @@ -1214,7 +1216,7 @@ class SSPVerificationRetryConfig(ConfigurationModel): # pylint: disable=model-m return str(self.arguments) -class VerificationAttempt(TimeStampedModel): +class VerificationAttempt(StatusModel): """ The model represents impelementation-agnostic information about identity verification (IDV) attempts. @@ -1224,23 +1226,29 @@ class VerificationAttempt(TimeStampedModel): user = models.ForeignKey(User, db_index=True, on_delete=models.CASCADE) name = models.CharField(blank=True, max_length=255) - STATUS_CHOICES = [ + STATUS = Choices( VerificationAttemptStatus.CREATED, VerificationAttemptStatus.PENDING, VerificationAttemptStatus.APPROVED, VerificationAttemptStatus.DENIED, - ] - status = models.CharField(max_length=64, choices=[(status, status) for status in STATUS_CHOICES]) + ) expiration_datetime = models.DateTimeField( null=True, blank=True, ) - @property - def updated_at(self): - """Backwards compatibility with existing IDVerification models""" - return self.modified + hide_status_from_user = models.BooleanField( + default=False, + null=True, + ) + + created_at = models.DateTimeField(auto_now_add=True, db_index=True) + updated_at = models.DateTimeField(auto_now=True, db_index=True) + + def should_display_status_to_user(self): + """When called, returns true or false based on the type of VerificationAttempt""" + return not self.hide_status_from_user @classmethod def retire_user(cls, user_id): diff --git a/lms/djangoapps/verify_student/services.py b/lms/djangoapps/verify_student/services.py index 1a2d145e89..f0d8a86314 100644 --- a/lms/djangoapps/verify_student/services.py +++ b/lms/djangoapps/verify_student/services.py @@ -76,7 +76,7 @@ class IDVerificationService: Return a list of all verifications associated with the given user. """ verifications = [] - for verification in chain(VerificationAttempt.objects.filter(user=user).order_by('-created'), + for verification in chain(VerificationAttempt.objects.filter(user=user).order_by('-created_at'), SoftwareSecurePhotoVerification.objects.filter(user=user).order_by('-created_at'), SSOVerification.objects.filter(user=user).order_by('-created_at'), ManualVerification.objects.filter(user=user).order_by('-created_at')): @@ -97,7 +97,7 @@ class IDVerificationService: VerificationAttempt.objects.filter(**{ 'user__in': users, 'status': 'approved', - 'created__gt': now() - timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"]) + 'created_at__gt': now() - timedelta(days=settings.VERIFY_STUDENT["DAYS_GOOD_FOR"]) }).values_list('user_id', flat=True), SoftwareSecurePhotoVerification.objects.filter(**filter_kwargs).values_list('user_id', flat=True), SSOVerification.objects.filter(**filter_kwargs).values_list('user_id', flat=True),