Skip to content

Commit

Permalink
fix: add should_display_status_to_user method and status_changed fiel…
Browse files Browse the repository at this point in the history
…d 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
  • Loading branch information
ilee2u authored Sep 26, 2024
1 parent 468c2bb commit dac0309
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 12 deletions.
11 changes: 9 additions & 2 deletions lms/djangoapps/verify_student/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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(
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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'),
),
]
24 changes: 16 additions & 8 deletions lms/djangoapps/verify_student/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@
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
Expand Down Expand Up @@ -1214,7 +1216,7 @@ def __str__(self):
return str(self.arguments)


class VerificationAttempt(TimeStampedModel):
class VerificationAttempt(StatusModel):
"""
The model represents impelementation-agnostic information about identity verification (IDV) attempts.
Expand All @@ -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):
Expand Down
4 changes: 2 additions & 2 deletions lms/djangoapps/verify_student/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def verifications_for_user(cls, user):
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')):
Expand All @@ -97,7 +97,7 @@ def get_verified_user_ids(cls, users):
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),
Expand Down

0 comments on commit dac0309

Please sign in to comment.