From 803bc5532dfa11a2c3d1c77a846597445427e868 Mon Sep 17 00:00:00 2001 From: zubair-arbi Date: Thu, 28 May 2015 17:03:54 +0500 Subject: [PATCH] use reverification xblock location as an identifier of reverification checkpoint ECOM-1612 --- lms/djangoapps/verify_student/admin.py | 8 +- ...ckpoint_checkpoint_name__add_field_veri.py | 153 ++++++++++++ lms/djangoapps/verify_student/models.py | 151 ++++++------ lms/djangoapps/verify_student/services.py | 59 ++--- .../verify_student/tests/test_models.py | 229 ++++++++++-------- .../verify_student/tests/test_services.py | 84 ++++--- .../verify_student/tests/test_views.py | 219 +++++++++-------- lms/djangoapps/verify_student/urls.py | 3 +- lms/djangoapps/verify_student/views.py | 197 +++++++-------- .../js/verify_student/incourse_reverify.js | 1 - .../models/reverification_model.js | 4 +- .../views/incourse_reverify_view.js | 3 - .../verify_student/incourse_reverify.html | 1 - requirements/edx/github.txt | 2 +- 14 files changed, 668 insertions(+), 446 deletions(-) create mode 100644 lms/djangoapps/verify_student/migrations/0008_auto__del_field_verificationcheckpoint_checkpoint_name__add_field_veri.py diff --git a/lms/djangoapps/verify_student/admin.py b/lms/djangoapps/verify_student/admin.py index 0d93b202a7..73d878a786 100644 --- a/lms/djangoapps/verify_student/admin.py +++ b/lms/djangoapps/verify_student/admin.py @@ -23,9 +23,9 @@ class VerificationStatusAdmin(admin.ModelAdmin): """ Admin for the VerificationStatus table. """ - list_display = ('timestamp', 'user', 'status', 'checkpoint', 'location_id') + list_display = ('timestamp', 'user', 'status', 'checkpoint') readonly_fields = () - search_fields = ('checkpoint', 'user') + search_fields = ('checkpoint__checkpoint_location', 'user__username') def get_readonly_fields(self, request, obj=None): """When editing an existing record, all fields should be read-only. @@ -36,7 +36,7 @@ class VerificationStatusAdmin(admin.ModelAdmin): """ if obj: - return self.readonly_fields + ('status', 'checkpoint', 'user', 'location_id', 'response', 'error') + return self.readonly_fields + ('status', 'checkpoint', 'user', 'response', 'error') return self.readonly_fields def has_delete_permission(self, request, obj=None): @@ -48,7 +48,7 @@ class SkippedReverificationAdmin(admin.ModelAdmin): """Admin for the SkippedReverification table. """ list_display = ('created_at', 'user', 'course_id', 'checkpoint') readonly_fields = ('user', 'course_id') - search_fields = ('user', 'course_id', 'checkpoint') + search_fields = ('user__username', 'course_id', 'checkpoint__checkpoint_location') def has_add_permission(self, request): """Skipped verifications can't be created in Django admin. """ diff --git a/lms/djangoapps/verify_student/migrations/0008_auto__del_field_verificationcheckpoint_checkpoint_name__add_field_veri.py b/lms/djangoapps/verify_student/migrations/0008_auto__del_field_verificationcheckpoint_checkpoint_name__add_field_veri.py new file mode 100644 index 0000000000..3ea1810b66 --- /dev/null +++ b/lms/djangoapps/verify_student/migrations/0008_auto__del_field_verificationcheckpoint_checkpoint_name__add_field_veri.py @@ -0,0 +1,153 @@ +# -*- coding: utf-8 -*- +from south.utils import datetime_utils as datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + + +class Migration(SchemaMigration): + + def forwards(self, orm): + # Removing unique constraint on 'VerificationCheckpoint', fields ['course_id', 'checkpoint_name'] + db.delete_unique('verify_student_verificationcheckpoint', ['course_id', 'checkpoint_name']) + + # Deleting field 'VerificationCheckpoint.checkpoint_name' + db.delete_column('verify_student_verificationcheckpoint', 'checkpoint_name') + + # Adding field 'VerificationCheckpoint.checkpoint_location' + db.add_column('verify_student_verificationcheckpoint', 'checkpoint_location', + self.gf('django.db.models.fields.CharField')(default='', max_length=255), + keep_default=False) + + # Adding unique constraint on 'VerificationCheckpoint', fields ['course_id', 'checkpoint_location'] + db.create_unique('verify_student_verificationcheckpoint', ['course_id', 'checkpoint_location']) + + # Deleting field 'VerificationStatus.location_id' + db.delete_column('verify_student_verificationstatus', 'location_id') + + + def backwards(self, orm): + # Removing unique constraint on 'VerificationCheckpoint', fields ['course_id', 'checkpoint_location'] + db.delete_unique('verify_student_verificationcheckpoint', ['course_id', 'checkpoint_location']) + + + # User chose to not deal with backwards NULL issues for 'VerificationCheckpoint.checkpoint_name' + raise RuntimeError("Cannot reverse this migration. 'VerificationCheckpoint.checkpoint_name' and its values cannot be restored.") + + # The following code is provided here to aid in writing a correct migration # Adding field 'VerificationCheckpoint.checkpoint_name' + db.add_column('verify_student_verificationcheckpoint', 'checkpoint_name', + self.gf('django.db.models.fields.CharField')(max_length=32), + keep_default=False) + + # Deleting field 'VerificationCheckpoint.checkpoint_location' + db.delete_column('verify_student_verificationcheckpoint', 'checkpoint_location') + + # Adding unique constraint on 'VerificationCheckpoint', fields ['course_id', 'checkpoint_name'] + db.create_unique('verify_student_verificationcheckpoint', ['course_id', 'checkpoint_name']) + + # Adding field 'VerificationStatus.location_id' + db.add_column('verify_student_verificationstatus', 'location_id', + self.gf('django.db.models.fields.CharField')(max_length=255, null=True, blank=True), + keep_default=False) + + + models = { + 'auth.group': { + 'Meta': {'object_name': 'Group'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}), + 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}) + }, + 'auth.permission': { + 'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'}, + 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + 'auth.user': { + 'Meta': {'object_name': 'User'}, + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}), + 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'}) + }, + 'contenttypes.contenttype': { + 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"}, + 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + }, + 'reverification.midcoursereverificationwindow': { + 'Meta': {'object_name': 'MidcourseReverificationWindow'}, + 'course_id': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}), + 'end_date': ('django.db.models.fields.DateTimeField', [], {'default': 'None', 'null': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'start_date': ('django.db.models.fields.DateTimeField', [], {'default': 'None', 'null': 'True', 'blank': 'True'}) + }, + 'verify_student.incoursereverificationconfiguration': { + 'Meta': {'object_name': 'InCourseReverificationConfiguration'}, + 'change_date': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}), + 'changed_by': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'null': 'True', 'on_delete': 'models.PROTECT'}), + 'enabled': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}) + }, + 'verify_student.skippedreverification': { + 'Meta': {'unique_together': "(('user', 'course_id'),)", 'object_name': 'SkippedReverification'}, + 'checkpoint': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'skipped_checkpoint'", 'to': "orm['verify_student.VerificationCheckpoint']"}), + 'course_id': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}), + 'created_at': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}) + }, + 'verify_student.softwaresecurephotoverification': { + 'Meta': {'ordering': "['-created_at']", 'object_name': 'SoftwareSecurePhotoVerification'}, + 'created_at': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'db_index': 'True', 'blank': 'True'}), + 'display': ('django.db.models.fields.BooleanField', [], {'default': 'True', 'db_index': 'True'}), + 'error_code': ('django.db.models.fields.CharField', [], {'max_length': '50', 'blank': 'True'}), + 'error_msg': ('django.db.models.fields.TextField', [], {'blank': 'True'}), + 'face_image_url': ('django.db.models.fields.URLField', [], {'max_length': '255', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '255', 'blank': 'True'}), + 'photo_id_image_url': ('django.db.models.fields.URLField', [], {'max_length': '255', 'blank': 'True'}), + 'photo_id_key': ('django.db.models.fields.TextField', [], {'max_length': '1024'}), + 'receipt_id': ('django.db.models.fields.CharField', [], {'default': "'4ae40fdd-c39a-4a23-a593-41beec90013b'", 'max_length': '255', 'db_index': 'True'}), + 'reviewing_service': ('django.db.models.fields.CharField', [], {'max_length': '255', 'blank': 'True'}), + 'reviewing_user': ('django.db.models.fields.related.ForeignKey', [], {'default': 'None', 'related_name': "'photo_verifications_reviewed'", 'null': 'True', 'to': "orm['auth.User']"}), + 'status': ('model_utils.fields.StatusField', [], {'default': "'created'", 'max_length': '100', u'no_check_for_status': 'True'}), + 'status_changed': ('model_utils.fields.MonitorField', [], {'default': 'datetime.datetime.now', u'monitor': "u'status'"}), + 'submitted_at': ('django.db.models.fields.DateTimeField', [], {'null': 'True', 'db_index': 'True'}), + 'updated_at': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'db_index': 'True', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}), + 'window': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['reverification.MidcourseReverificationWindow']", 'null': 'True'}) + }, + 'verify_student.verificationcheckpoint': { + 'Meta': {'unique_together': "(('course_id', 'checkpoint_location'),)", 'object_name': 'VerificationCheckpoint'}, + 'checkpoint_location': ('django.db.models.fields.CharField', [], {'max_length': '255'}), + 'course_id': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'photo_verification': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['verify_student.SoftwareSecurePhotoVerification']", 'symmetrical': 'False'}) + }, + 'verify_student.verificationstatus': { + 'Meta': {'object_name': 'VerificationStatus'}, + 'checkpoint': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'checkpoint_status'", 'to': "orm['verify_student.VerificationCheckpoint']"}), + 'error': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'response': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'status': ('django.db.models.fields.CharField', [], {'max_length': '32', 'db_index': 'True'}), + 'timestamp': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}) + } + } + + complete_apps = ['verify_student'] \ No newline at end of file diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index 2c5e376845..53e394967d 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -8,16 +8,17 @@ of a student over a period of time. Right now, the only models are the abstract `SoftwareSecurePhotoVerification`. The hope is to keep as much of the photo verification process as generic as possible. """ -from datetime import datetime, timedelta -from email.utils import formatdate import functools import json import logging +from datetime import datetime, timedelta +from email.utils import formatdate -from course_modes.models import CourseMode import pytz import requests import uuid +from lazy import lazy +from opaque_keys.edx.keys import UsageKey from django.conf import settings from django.contrib.auth.models import User @@ -29,6 +30,7 @@ from django.utils.translation import ugettext as _, ugettext_lazy from boto.s3.connection import S3Connection from boto.s3.key import Key from config_models.models import ConfigurationModel +from course_modes.models import CourseMode from model_utils.models import StatusModel from model_utils import Choices from reverification.models import MidcourseReverificationWindow @@ -36,6 +38,8 @@ from verify_student.ssencrypt import ( random_aes_key, encrypt_and_encode, generate_signed_message, rsa_encrypt ) +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule_django.models import CourseKeyField @@ -852,12 +856,13 @@ class SoftwareSecurePhotoVerification(PhotoVerification): @classmethod def submit_faceimage(cls, user, face_image, photo_id_key): - """Submit the faceimage to SoftwareSecurePhotoVerification + """Submit the face image to SoftwareSecurePhotoVerification. Arguments: user(User): user object face_image (bytestream): raw bytestream image data photo_id_key (str) : SoftwareSecurePhotoVerification attribute + Returns: SoftwareSecurePhotoVerification Object """ @@ -887,42 +892,60 @@ class SoftwareSecurePhotoVerification(PhotoVerification): class VerificationCheckpoint(models.Model): - """Represents a point at which a user is challenged to reverify his or her identity. - Each checkpoint is uniquely identified by a (course_id, checkpoint_name) tuple. + """Represents a point at which a user is asked to re-verify his/her + identity. + + Each checkpoint is uniquely identified by a + (course_id, checkpoint_location) tuple. """ - - CHECKPOINT_CHOICES = ( - ("midterm", "midterm"), - ("final", "final"), - ) - course_id = CourseKeyField(max_length=255, db_index=True) - checkpoint_name = models.CharField(max_length=32, choices=CHECKPOINT_CHOICES) + checkpoint_location = models.CharField(max_length=255) photo_verification = models.ManyToManyField(SoftwareSecurePhotoVerification) class Meta: # pylint: disable=missing-docstring, old-style-class - unique_together = (('course_id', 'checkpoint_name'),) + unique_together = ('course_id', 'checkpoint_location') def __unicode__(self): - """Unicode representation of the checkpoint. """ + """ + Unicode representation of the checkpoint. + """ return u"{checkpoint} in {course}".format( checkpoint=self.checkpoint_name, course=self.course_id ) + @lazy + def checkpoint_name(self): + """Lazy method for getting checkpoint name of reverification block. + + Return location of the checkpoint if no related assessment found in + database. + """ + checkpoint_key = UsageKey.from_string(self.checkpoint_location) + try: + checkpoint_name = modulestore().get_item(checkpoint_key).related_assessment + except ItemNotFoundError: + log.warning( + u"Verification checkpoint block with location '%s' and course id '%s' " + u"not found in database.", self.checkpoint_location, unicode(self.course_id) + ) + checkpoint_name = self.checkpoint_location + + return checkpoint_name + def add_verification_attempt(self, verification_attempt): - """ Add the verification attempt in M2M relation of photo_verification + """Add the verification attempt in M2M relation of photo_verification. Arguments: - verification_attempt(SoftwareSecurePhotoVerification): SoftwareSecurePhotoVerification object + verification_attempt(object): SoftwareSecurePhotoVerification object Returns: None """ - self.photo_verification.add(verification_attempt) # pylint: disable=no-member + self.photo_verification.add(verification_attempt) # pylint: disable=no-member def get_user_latest_status(self, user_id): - """ Return the latest status of the given checkpoint attempt by user + """Get the status of the latest checkpoint attempt of the given user. Args: user_id(str): Id of user @@ -931,34 +954,35 @@ class VerificationCheckpoint(models.Model): VerificationStatus object if found any else None """ try: - return self.checkpoint_status.filter(user_id=user_id).latest() # pylint: disable=E1101 + return self.checkpoint_status.filter(user_id=user_id).latest() # pylint: disable=no-member except ObjectDoesNotExist: return None @classmethod - def get_verification_checkpoint(cls, course_id, checkpoint_name): - """Get the verification checkpoint for given course_id and checkpoint name + def get_verification_checkpoint(cls, course_id, checkpoint_location): + """Get the verification checkpoint for given 'course_id' and + checkpoint name. Arguments: course_id(CourseKey): CourseKey - checkpoint_name(str): checkpoint name + checkpoint_location(str): Verification checkpoint location Returns: VerificationCheckpoint object if exists otherwise None """ try: - return cls.objects.get(course_id=course_id, checkpoint_name=checkpoint_name) + return cls.objects.get(course_id=course_id, checkpoint_location=checkpoint_location) except cls.DoesNotExist: return None class VerificationStatus(models.Model): - """A verification status represents a user’s progress - through the verification process for a particular checkpoint - Model is an append-only table that represents the user status changes in - verification process - """ + """This model is an append-only table that represents user status changes + during the verification process. + A verification status represents a user’s progress through the verification + process for a particular checkpoint. + """ VERIFICATION_STATUS_CHOICES = ( ("submitted", "submitted"), ("approved", "approved"), @@ -973,56 +997,43 @@ class VerificationStatus(models.Model): response = models.TextField(null=True, blank=True) error = models.TextField(null=True, blank=True) - # This field is used to save location of Reverification module in courseware - location_id = models.CharField( - null=True, - blank=True, - max_length=255, - help_text=ugettext_lazy("Usage id of Reverification XBlock.") - ) - class Meta(object): # pylint: disable=missing-docstring get_latest_by = "timestamp" verbose_name = "Verification Status" verbose_name_plural = "Verification Statuses" @classmethod - def add_verification_status(cls, checkpoint, user, status, location_id=None): - """ Create new verification status object + def add_verification_status(cls, checkpoint, user, status): + """Create new verification status object. Arguments: checkpoint(VerificationCheckpoint): VerificationCheckpoint object user(User): user object - status(str): String representing the status from VERIFICATION_STATUS_CHOICES - location_id(str): Usage key of Reverification XBlock + status(str): Status from VERIFICATION_STATUS_CHOICES + Returns: None """ - cls.objects.create(checkpoint=checkpoint, user=user, status=status, location_id=location_id) + cls.objects.create(checkpoint=checkpoint, user=user, status=status) @classmethod def add_status_from_checkpoints(cls, checkpoints, user, status): - """ Create new verification status objects against the given checkpoints + """Create new verification status objects for a user against the given + checkpoints. Arguments: checkpoints(list): list of VerificationCheckpoint objects user(User): user object - status(str): String representing the status from VERIFICATION_STATUS_CHOICES + status(str): Status from VERIFICATION_STATUS_CHOICES + Returns: None """ for checkpoint in checkpoints: - # get 'location_id' from last entry (if it exists) and add it in - # new entry - try: - location_id = cls.objects.filter(checkpoint=checkpoint).latest().location_id - except cls.DoesNotExist: - location_id = None - - cls.objects.create(checkpoint=checkpoint, user=user, status=status, location_id=location_id) + cls.objects.create(checkpoint=checkpoint, user=user, status=status) @classmethod - def get_user_attempts(cls, user_id, course_key, related_assessment, location_id): + def get_user_attempts(cls, user_id, course_key, related_assessment_location): """ Get re-verification attempts against a user for a given 'checkpoint' and 'course_id'. @@ -1030,34 +1041,32 @@ class VerificationStatus(models.Model): Arguments: user_id(str): User Id string course_key(str): A CourseKey of a course - related_assessment(str): Verification checkpoint name - location_id(str): Location of Reverification XBlock in courseware + related_assessment_location(str): Verification checkpoint location Returns: - count of re-verification attempts + Count of re-verification attempts """ return cls.objects.filter( user_id=user_id, checkpoint__course_id=course_key, - checkpoint__checkpoint_name=related_assessment, - location_id=location_id, + checkpoint__checkpoint_location=related_assessment_location, status="submitted" ).count() @classmethod def get_location_id(cls, photo_verification): - """ Return the location id of xblock + """Get the location ID of reverification XBlock. Args: - photo_verification(SoftwareSecurePhotoVerification): SoftwareSecurePhotoVerification object + photo_verification(object): SoftwareSecurePhotoVerification object Return: - Location Id of xblock if any else empty string + Location Id of XBlock if any else empty string """ try: - ver_status = cls.objects.filter(checkpoint__photo_verification=photo_verification).latest() - return ver_status.location_id + verification_status = cls.objects.filter(checkpoint__photo_verification=photo_verification).latest() + return verification_status.checkpoint.checkpoint_location except cls.DoesNotExist: return "" @@ -1071,17 +1080,16 @@ class InCourseReverificationConfiguration(ConfigurationModel): When the flag is enabled, the "in-course re-verification" feature will be enabled. - """ pass class SkippedReverification(models.Model): - """ - Model for tracking skipped Reverification of a user against a specific + """Model for tracking skipped Reverification of a user against a specific course. - If user skipped the Reverification for a specific course then in future - user cannot see the reverification link. + + If a user skipped a Reverification checkpoint for a specific course then in + future that user cannot see the reverification link. """ user = models.ForeignKey(User) course_id = CourseKeyField(max_length=255, db_index=True) @@ -1093,12 +1101,13 @@ class SkippedReverification(models.Model): @classmethod def add_skipped_reverification_attempt(cls, checkpoint, user_id, course_id): - """ Create skipped reverification object + """Create skipped reverification object. Arguments: checkpoint(VerificationCheckpoint): VerificationCheckpoint object user_id(str): User Id of currently logged in user course_id(CourseKey): CourseKey + Returns: None """ @@ -1106,11 +1115,13 @@ class SkippedReverification(models.Model): @classmethod def check_user_skipped_reverification_exists(cls, user, course_id): - """Check user skipped re-verification attempt exists against specific course + """Check existence of a user's skipped re-verification attempt for a + specific course. Arguments: user(User): user object course_id(CourseKey): CourseKey + Returns: Boolean """ diff --git a/lms/djangoapps/verify_student/services.py b/lms/djangoapps/verify_student/services.py index 5e8e989a7f..bb607f17eb 100644 --- a/lms/djangoapps/verify_student/services.py +++ b/lms/djangoapps/verify_student/services.py @@ -9,6 +9,7 @@ from django.core.urlresolvers import reverse from django.db import IntegrityError from opaque_keys.edx.keys import CourseKey + from verify_student.models import VerificationCheckpoint, VerificationStatus, SkippedReverification @@ -20,19 +21,19 @@ class ReverificationService(object): Reverification XBlock service """ - def get_status(self, user_id, course_id, related_assessment): - """ - Get verification attempt status against a user for a given 'checkpoint' - and 'course_id'. + def get_status(self, user_id, course_id, related_assessment_location): + """Get verification attempt status against a user for a given + 'checkpoint' and 'course_id'. Args: user_id(str): User Id string course_id(str): A string of course id - related_assessment(str): Verification checkpoint name + related_assessment_location(str): Location of Reverification XBlock Returns: - "skipped" if has skip the re-verification or Verification Status string if - any attempt submitted by user else None + "skipped" if the user has skipped the re-verification or + Verification Status string if the user has submitted photo + verification attempt else None """ course_key = CourseKey.from_string(course_id) has_skipped = SkippedReverification.check_user_skipped_reverification_exists(user_id, course_key) @@ -42,69 +43,73 @@ class ReverificationService(object): checkpoint_status = VerificationStatus.objects.filter( user_id=user_id, checkpoint__course_id=course_key, - checkpoint__checkpoint_name=related_assessment + checkpoint__checkpoint_location=related_assessment_location ).latest() return checkpoint_status.status except ObjectDoesNotExist: return None - def start_verification(self, course_id, related_assessment, item_id): - """ - Create re-verification link against a verification checkpoint. + def start_verification(self, course_id, related_assessment_location): + """Create re-verification link against a verification checkpoint. Args: course_id(str): A string of course id - related_assessment(str): Verification checkpoint name + related_assessment_location(str): Location of Reverification XBlock Returns: Re-verification link """ course_key = CourseKey.from_string(course_id) - VerificationCheckpoint.objects.get_or_create(course_id=course_key, checkpoint_name=related_assessment) + VerificationCheckpoint.objects.get_or_create( + course_id=course_key, + checkpoint_location=related_assessment_location + ) + re_verification_link = reverse( 'verify_student_incourse_reverify', args=( unicode(course_key), - unicode(related_assessment), - unicode(item_id) + unicode(related_assessment_location) ) ) return re_verification_link - def skip_verification(self, checkpoint_name, user_id, course_id): - """ - Add skipped verification attempt entry against a given 'checkpoint' + def skip_verification(self, user_id, course_id, related_assessment_location): + """Add skipped verification attempt entry for a user against a given + 'checkpoint'. Args: - checkpoint_name(str): Verification checkpoint name user_id(str): User Id string course_id(str): A string of course_id + related_assessment_location(str): Location of Reverification XBlock Returns: None """ course_key = CourseKey.from_string(course_id) - checkpoint = VerificationCheckpoint.objects.get(course_id=course_key, checkpoint_name=checkpoint_name) + checkpoint = VerificationCheckpoint.objects.get( + course_id=course_key, + checkpoint_location=related_assessment_location + ) - # if user do not already skipped the attempt for this course only then he can skip + # user can skip a reverification attempt only if that user has not already + # skipped an attempt try: SkippedReverification.add_skipped_reverification_attempt(checkpoint, user_id, course_key) except IntegrityError: log.exception("Skipped attempt already exists for user %s: with course %s:", user_id, unicode(course_id)) - def get_attempts(self, user_id, course_id, related_assessment, location_id): - """ - Get re-verification attempts against a user for a given 'checkpoint' + def get_attempts(self, user_id, course_id, related_assessment_location): + """Get re-verification attempts against a user for a given 'checkpoint' and 'course_id'. Args: user_id(str): User Id string course_id(str): A string of course id - related_assessment(str): Verification checkpoint name - location_id(str): Location of Reverification XBlock in courseware + related_assessment_location(str): Location of Reverification XBlock Returns: Number of re-verification attempts of a user """ course_key = CourseKey.from_string(course_id) - return VerificationStatus.get_user_attempts(user_id, course_key, related_assessment, location_id) + return VerificationStatus.get_user_attempts(user_id, course_key, related_assessment_location) diff --git a/lms/djangoapps/verify_student/tests/test_models.py b/lms/djangoapps/verify_student/tests/test_models.py index 987a0edadb..b63f96becd 100644 --- a/lms/djangoapps/verify_student/tests/test_models.py +++ b/lms/djangoapps/verify_student/tests/test_models.py @@ -6,7 +6,6 @@ import requests.exceptions import pytz from django.conf import settings -from django.test import TestCase from django.db.utils import IntegrityError from mock import patch from nose.tools import assert_is_none, assert_equals, assert_raises, assert_true, assert_false # pylint: disable=E0611 @@ -486,69 +485,92 @@ class TestPhotoVerification(ModuleStoreTestCase): class VerificationCheckpointTest(ModuleStoreTestCase): """Tests for the VerificationCheckpoint model. """ - MIDTERM = "midterm" - FINAL = "final" - def setUp(self): super(VerificationCheckpointTest, self).setUp() self.user = UserFactory.create() self.course = CourseFactory.create() + self.checkpoint_midterm = u'i4x://{org}/{course}/edx-reverification-block/midterm_uuid'.format( + org=self.course.id.org, course=self.course.id.course + ) + self.checkpoint_final = u'i4x://{org}/{course}/edx-reverification-block/final_uuid'.format( + org=self.course.id.org, course=self.course.id.course + ) - @ddt.data(MIDTERM, FINAL) - def test_get_verification_checkpoint(self, check_point): - """testing class method of VerificationCheckpoint. create the object and then uses the class method to get the - verification check point. + @ddt.data('midterm', 'final') + def test_get_verification_checkpoint(self, checkpoint): """ - # create the VerificationCheckpoint checkpoint - ver_check_point = VerificationCheckpoint.objects.create(course_id=self.course.id, checkpoint_name=check_point) + Test that a reverification checkpoint is created properly. + """ + checkpoint_location = u'i4x://{org}/{course}/edx-reverification-block/{checkpoint}'.format( + org=self.course.id.org, course=self.course.id.course, checkpoint=checkpoint + ) + # create the 'VerificationCheckpoint' checkpoint + verification_checkpoint = VerificationCheckpoint.objects.create( + course_id=self.course.id, + checkpoint_location=checkpoint_location + ) self.assertEqual( - VerificationCheckpoint.get_verification_checkpoint(self.course.id, check_point), - ver_check_point + VerificationCheckpoint.get_verification_checkpoint(self.course.id, checkpoint_location), + verification_checkpoint ) def test_get_verification_checkpoint_for_not_existing_values(self): - """testing class method of VerificationCheckpoint. create the object and then uses the class method to get the - verification check point. + """Test that 'get_verification_checkpoint' method returns None if user + tries to access a checkpoint with an invalid location. """ # create the VerificationCheckpoint checkpoint - VerificationCheckpoint.objects.create(course_id=self.course.id, checkpoint_name=self.MIDTERM) + VerificationCheckpoint.objects.create(course_id=self.course.id, checkpoint_location=self.checkpoint_midterm) - # get verification for not existing checkpoint - self.assertEqual(VerificationCheckpoint.get_verification_checkpoint(self.course.id, 'abc'), None) + # get verification for a non existing checkpoint + self.assertEqual( + VerificationCheckpoint.get_verification_checkpoint( + self.course.id, + u'i4x://edX/DemoX/edx-reverification-block/invalid_location' + ), + None + ) def test_unique_together_constraint(self): - """testing the unique together contraint. + """ + Test the unique together constraint. """ # create the VerificationCheckpoint checkpoint - VerificationCheckpoint.objects.create(course_id=self.course.id, checkpoint_name=self.MIDTERM) + VerificationCheckpoint.objects.create(course_id=self.course.id, checkpoint_location=self.checkpoint_midterm) - # create the VerificationCheckpoint checkpoint with same course id and checkpoint name + # test creating the VerificationCheckpoint checkpoint with same course + # id and checkpoint name with self.assertRaises(IntegrityError): - VerificationCheckpoint.objects.create(course_id=self.course.id, checkpoint_name=self.MIDTERM) + VerificationCheckpoint.objects.create(course_id=self.course.id, checkpoint_location=self.checkpoint_midterm) def test_add_verification_attempt_software_secure(self): - """testing manytomany relationship. adding softwaresecure attempt to the verification checkpoints. + """ + Test adding Software Secure photo verification attempts for the + reverification checkpoints. """ # adding two check points. - check_point1 = VerificationCheckpoint.objects.create(course_id=self.course.id, checkpoint_name=self.MIDTERM) - check_point2 = VerificationCheckpoint.objects.create(course_id=self.course.id, checkpoint_name=self.FINAL) + first_checkpoint = VerificationCheckpoint.objects.create( + course_id=self.course.id, checkpoint_location=self.checkpoint_midterm + ) + second_checkpoint = VerificationCheckpoint.objects.create( + course_id=self.course.id, checkpoint_location=self.checkpoint_final + ) - # Make an attempt and added to the checkpoint1. - check_point1.add_verification_attempt(SoftwareSecurePhotoVerification.objects.create(user=self.user)) - self.assertEqual(check_point1.photo_verification.count(), 1) + # make an attempt for the 'first_checkpoint' + first_checkpoint.add_verification_attempt(SoftwareSecurePhotoVerification.objects.create(user=self.user)) + self.assertEqual(first_checkpoint.photo_verification.count(), 1) - # Make an other attempt and added to the checkpoint1. - check_point1.add_verification_attempt(SoftwareSecurePhotoVerification.objects.create(user=self.user)) - self.assertEqual(check_point1.photo_verification.count(), 2) + # make another attempt for the 'first_checkpoint' + first_checkpoint.add_verification_attempt(SoftwareSecurePhotoVerification.objects.create(user=self.user)) + self.assertEqual(first_checkpoint.photo_verification.count(), 2) - # make new attempt and adding to the checkpoint2 + # make new attempt for the 'second_checkpoint' attempt = SoftwareSecurePhotoVerification.objects.create(user=self.user) - check_point2.add_verification_attempt(attempt) - self.assertEqual(check_point2.photo_verification.count(), 1) + second_checkpoint.add_verification_attempt(attempt) + self.assertEqual(second_checkpoint.photo_verification.count(), 1) - # remove the attempt from checkpoint2 - check_point2.photo_verification.remove(attempt) - self.assertEqual(check_point2.photo_verification.count(), 0) + # remove the attempt from 'second_checkpoint' + second_checkpoint.photo_verification.remove(attempt) + self.assertEqual(second_checkpoint.photo_verification.count(), 0) @ddt.ddt @@ -559,15 +581,20 @@ class VerificationStatusTest(ModuleStoreTestCase): super(VerificationStatusTest, self).setUp() self.user = UserFactory.create() self.course = CourseFactory.create() - self.check_point1 = VerificationCheckpoint.objects.create(course_id=self.course.id, checkpoint_name="midterm") - self.check_point2 = VerificationCheckpoint.objects.create(course_id=self.course.id, checkpoint_name="final") - self.dummy_reverification_item_id_1 = 'i4x://{}/{}/edx-reverification-block/related_assessment_1'.format( - self.course.location.org, - self.course.location.course + + self.first_checkpoint_location = u'i4x://{org}/{course}/edx-reverification-block/first_checkpoint_uuid'.format( + org=self.course.id.org, course=self.course.id.course ) - self.dummy_reverification_item_id_2 = 'i4x://{}/{}/edx-reverification-block/related_assessment_2'.format( - self.course.location.org, - self.course.location.course + self.first_checkpoint = VerificationCheckpoint.objects.create( + course_id=self.course.id, + checkpoint_location=self.first_checkpoint_location + ) + + self.second_checkpoint_location = u'i4x://{org}/{course}/edx-reverification-block/second_checkpoint_uuid'.\ + format(org=self.course.id.org, course=self.course.id.course) + self.second_checkpoint = VerificationCheckpoint.objects.create( + course_id=self.course.id, + checkpoint_location=self.second_checkpoint_location ) @ddt.data('submitted', "approved", "denied", "error") @@ -576,149 +603,159 @@ class VerificationStatusTest(ModuleStoreTestCase): # adding verification status VerificationStatus.add_verification_status( - checkpoint=self.check_point1, + checkpoint=self.first_checkpoint, user=self.user, - status=status, - location_id=self.dummy_reverification_item_id_1 + status=status ) - # getting the status from db - result = VerificationStatus.objects.filter(checkpoint=self.check_point1)[0] + # test the status from database + result = VerificationStatus.objects.filter(checkpoint=self.first_checkpoint)[0] self.assertEqual(result.status, status) self.assertEqual(result.user, self.user) @ddt.data("approved", "denied", "error") def test_add_status_from_checkpoints(self, status): - """ Adding verification status for checkpoints list after submitting sspv. """ + """Test verification status for reverification checkpoints after + submitting software secure photo verification. + """ # add initial verification status for checkpoints initial_status = "submitted" VerificationStatus.add_verification_status( - checkpoint=self.check_point1, + checkpoint=self.first_checkpoint, user=self.user, - status=initial_status, - location_id=self.dummy_reverification_item_id_1 + status=initial_status ) VerificationStatus.add_verification_status( - checkpoint=self.check_point2, + checkpoint=self.second_checkpoint, user=self.user, - status=initial_status, - location_id=self.dummy_reverification_item_id_2 + status=initial_status ) # now add verification status for multiple checkpoint points VerificationStatus.add_status_from_checkpoints( - checkpoints=[self.check_point1, self.check_point2], user=self.user, status=status + checkpoints=[self.first_checkpoint, self.second_checkpoint], user=self.user, status=status ) # test that verification status entries with new status have been added - # for both checkpoints and all entries have related 'location_id'. - result = VerificationStatus.objects.filter(user=self.user, checkpoint=self.check_point1) - self.assertEqual(len(result), len(self.check_point1.checkpoint_status.all())) + # for both checkpoints + result = VerificationStatus.objects.filter(user=self.user, checkpoint=self.first_checkpoint) + self.assertEqual(len(result), len(self.first_checkpoint.checkpoint_status.all())) self.assertEqual( - list(result.values_list('location_id', flat=True)), - list(self.check_point1.checkpoint_status.all().values_list('location_id', flat=True)) + list(result.values_list('checkpoint__checkpoint_location', flat=True)), + list(self.first_checkpoint.checkpoint_status.values_list('checkpoint__checkpoint_location', flat=True)) ) - result = VerificationStatus.objects.filter(user=self.user, checkpoint=self.check_point2) - self.assertEqual(len(result), len(self.check_point2.checkpoint_status.all())) + + result = VerificationStatus.objects.filter(user=self.user, checkpoint=self.second_checkpoint) + self.assertEqual(len(result), len(self.second_checkpoint.checkpoint_status.all())) self.assertEqual( - list(result.values_list('location_id', flat=True)), - list(self.check_point2.checkpoint_status.all().values_list('location_id', flat=True)) + list(result.values_list('checkpoint__checkpoint_location', flat=True)), + list(self.second_checkpoint.checkpoint_status.values_list('checkpoint__checkpoint_location', flat=True)) ) def test_get_location_id(self): - """ Getting location id for a specific checkpoint """ + """ + Getting location id for a specific checkpoint. + """ # creating software secure attempt against checkpoint - self.check_point1.add_verification_attempt(SoftwareSecurePhotoVerification.objects.create(user=self.user)) + self.first_checkpoint.add_verification_attempt(SoftwareSecurePhotoVerification.objects.create(user=self.user)) # add initial verification status for checkpoint VerificationStatus.add_verification_status( - checkpoint=self.check_point1, + checkpoint=self.first_checkpoint, user=self.user, status='submitted', - location_id=self.dummy_reverification_item_id_1 ) - attempt = SoftwareSecurePhotoVerification.objects.filter(user=self.user) self.assertIsNotNone(VerificationStatus.get_location_id(attempt)) self.assertEqual(VerificationStatus.get_location_id(None), '') def test_get_user_attempts(self): - - # adding verification status + """ + Test adding verification status. + """ VerificationStatus.add_verification_status( - checkpoint=self.check_point1, + checkpoint=self.first_checkpoint, user=self.user, - status='submitted', - location_id=self.dummy_reverification_item_id_1 + status='submitted' ) - self.assertEqual(VerificationStatus.get_user_attempts( - course_key=self.course.id, - user_id=self.user.id, - related_assessment='midterm', location_id=self.dummy_reverification_item_id_1), 1) + self.assertEqual( + VerificationStatus.get_user_attempts( + user_id=self.user.id, + course_key=self.course.id, + related_assessment_location=self.first_checkpoint_location + ), + 1 + ) class SkippedReverificationTest(ModuleStoreTestCase): - """Tests for the SkippedReverification model. """ + """ + Tests for the SkippedReverification model. + """ def setUp(self): super(SkippedReverificationTest, self).setUp() self.user = UserFactory.create() self.course = CourseFactory.create() - self.checkpoint = VerificationCheckpoint.objects.create(course_id=self.course.id, checkpoint_name="midterm") + dummy_checkpoint_location = u'i4x://edX/DemoX/edx-reverification-block/midterm_uuid' + self.checkpoint = VerificationCheckpoint.objects.create( + course_id=self.course.id, + checkpoint_location=dummy_checkpoint_location + ) def test_add_skipped_attempts(self): - """adding skipped re-verification object using class method.""" + """ + Test 'add_skipped_reverification_attempt' method. + """ - # adding verification status + # add verification status SkippedReverification.add_skipped_reverification_attempt( checkpoint=self.checkpoint, user_id=self.user.id, course_id=unicode(self.course.id) ) - # getting the status from db + # test the status of skipped reverification from database result = SkippedReverification.objects.filter(course_id=self.course.id)[0] self.assertEqual(result.checkpoint, self.checkpoint) self.assertEqual(result.user, self.user) self.assertEqual(result.course_id, self.course.id) def test_unique_constraint(self): - """adding skipped re-verification with same user and course id will - raise integrity exception + """Test that adding skipped re-verification with same user and course + id will raise 'IntegrityError' exception. """ - - # adding verification object + # add verification object SkippedReverification.add_skipped_reverification_attempt( checkpoint=self.checkpoint, user_id=self.user.id, course_id=unicode(self.course.id) ) - with self.assertRaises(IntegrityError): SkippedReverification.add_skipped_reverification_attempt( checkpoint=self.checkpoint, user_id=self.user.id, course_id=unicode(self.course.id) ) - # Create skipped attempt for different user + # create skipped attempt for different user user2 = UserFactory.create() SkippedReverification.add_skipped_reverification_attempt( checkpoint=self.checkpoint, user_id=user2.id, course_id=unicode(self.course.id) ) - # getting the status from db + # test the status of skipped reverification from database result = SkippedReverification.objects.filter(user=user2)[0] self.assertEqual(result.checkpoint, self.checkpoint) self.assertEqual(result.user, user2) self.assertEqual(result.course_id, self.course.id) def test_check_user_skipped_reverification_exists(self): - """Checking check_user_skipped_reverification_exists method returns boolean status""" - - # adding verification status + """ + Test the 'check_user_skipped_reverification_exists' method's response. + """ + # add verification status SkippedReverification.add_skipped_reverification_attempt( checkpoint=self.checkpoint, user_id=self.user.id, course_id=unicode(self.course.id) ) - self.assertTrue( SkippedReverification.check_user_skipped_reverification_exists(course_id=self.course.id, user=self.user) ) diff --git a/lms/djangoapps/verify_student/tests/test_services.py b/lms/djangoapps/verify_student/tests/test_services.py index 6a89a3efbd..8ff66d4de9 100644 --- a/lms/djangoapps/verify_student/tests/test_services.py +++ b/lms/djangoapps/verify_student/tests/test_services.py @@ -31,89 +31,101 @@ class TestReverificationService(ModuleStoreTestCase): min_price=100, ) self.item = ItemFactory.create(parent=course, category='chapter', display_name='Test Section') + self.final_checkpoint_location = u'i4x://{org}/{course}/edx-reverification-block/final_uuid'.format( + org=self.course_key.org, course=self.course_key.course + ) - @ddt.data("final_term", "mid_term") + @ddt.data('final', 'midterm') def test_start_verification(self, checkpoint_name): - """ - Test the 'start_verification' service method. If checkpoint exists for - a specific course then return the checkpoint otherwise create that - checkpoint. + """Test the 'start_verification' service method. + + Check that if a reverification checkpoint exists for a specific course + then 'start_verification' method returns that checkpoint otherwise it + creates that checkpoint. """ reverification_service = ReverificationService() - reverification_service.start_verification(unicode(self.course_key), checkpoint_name, self.item.location) + checkpoint_location = u'i4x://{org}/{course}/edx-reverification-block/{checkpoint}'.format( + org=self.course_key.org, course=self.course_key.course, checkpoint=checkpoint_name + ) expected_url = ( '/verify_student/reverify' '/{course_key}' - '/{checkpoint_name}' - '/{usage_id}/' - ).format(course_key=unicode(self.course_key), checkpoint_name=checkpoint_name, usage_id=self.item.location) + '/{checkpoint_location}/' + ).format(course_key=unicode(self.course_key), checkpoint_location=checkpoint_location) self.assertEqual( - expected_url, - reverification_service.start_verification(unicode(self.course_key), checkpoint_name, self.item.location) + reverification_service.start_verification(unicode(self.course_key), checkpoint_location), + expected_url ) def test_get_status(self): + """Test the verification statuses of a user for a given 'checkpoint' + and 'course_id'. """ - Test the verification statuses of a user for a given 'checkpoint' and - 'course_id'. - """ - checkpoint_name = 'final_term' reverification_service = ReverificationService() - self.assertIsNone(reverification_service.get_status(self.user.id, unicode(self.course_key), checkpoint_name)) + self.assertIsNone( + reverification_service.get_status(self.user.id, unicode(self.course_key), self.final_checkpoint_location) + ) checkpoint_obj = VerificationCheckpoint.objects.create( course_id=unicode(self.course_key), - checkpoint_name=checkpoint_name + checkpoint_location=self.final_checkpoint_location ) - VerificationStatus.objects.create(checkpoint=checkpoint_obj, user=self.user, status='submitted') self.assertEqual( - reverification_service.get_status(self.user.id, unicode(self.course_key), checkpoint_name), + reverification_service.get_status(self.user.id, unicode(self.course_key), self.final_checkpoint_location), 'submitted' ) - VerificationStatus.objects.create(checkpoint=checkpoint_obj, user=self.user, status='submitted') + VerificationStatus.objects.create(checkpoint=checkpoint_obj, user=self.user, status='approved') self.assertEqual( - reverification_service.get_status(self.user.id, unicode(self.course_key), checkpoint_name), - 'submitted' + reverification_service.get_status(self.user.id, unicode(self.course_key), self.final_checkpoint_location), + 'approved' ) def test_skip_verification(self): """ - Adding the test skip verification attempt for the user + Test adding skip attempt of a user for a reverification checkpoint. """ - checkpoint_name = 'final_term' reverification_service = ReverificationService() VerificationCheckpoint.objects.create( course_id=unicode(self.course_key), - checkpoint_name=checkpoint_name + checkpoint_location=self.final_checkpoint_location ) - reverification_service.skip_verification(checkpoint_name, self.user.id, unicode(self.course_key)) - self.assertEqual(1, SkippedReverification.objects.filter(user=self.user, course_id=self.course_key).count()) + reverification_service.skip_verification(self.user.id, unicode(self.course_key), self.final_checkpoint_location) + self.assertEqual( + SkippedReverification.objects.filter(user=self.user, course_id=self.course_key).count(), + 1 + ) - reverification_service.skip_verification(checkpoint_name, self.user.id, unicode(self.course_key)) - self.assertEqual(1, SkippedReverification.objects.filter(user=self.user, course_id=self.course_key).count()) + # now test that a user can have only one entry for a skipped + # reverification for a course + reverification_service.skip_verification(self.user.id, unicode(self.course_key), self.final_checkpoint_location) + self.assertEqual( + SkippedReverification.objects.filter(user=self.user, course_id=self.course_key).count(), + 1 + ) def test_get_attempts(self): - """ - Check verification attempts count against a user for a given + """Check verification attempts count against a user for a given 'checkpoint' and 'course_id'. """ - checkpoint_name = 'final_term' reverification_service = ReverificationService() course_id = unicode(self.course_key) self.assertEqual( - reverification_service.get_attempts(self.user.id, course_id, checkpoint_name, location_id=None), + reverification_service.get_attempts(self.user.id, course_id, self.final_checkpoint_location), 0 ) # now create a checkpoint and add user's entry against it then test - # that the 'get_attempts' service method returns count accordingly - checkpoint_obj = VerificationCheckpoint.objects.create(course_id=course_id, checkpoint_name=checkpoint_name) + # that the 'get_attempts' service method returns correct count + checkpoint_obj = VerificationCheckpoint.objects.create( + course_id=course_id, + checkpoint_location=self.final_checkpoint_location + ) VerificationStatus.objects.create(checkpoint=checkpoint_obj, user=self.user, status='submitted') self.assertEqual( - reverification_service.get_attempts(self.user.id, course_id, checkpoint_name, location_id=None), + reverification_service.get_attempts(self.user.id, course_id, self.final_checkpoint_location), 1 ) diff --git a/lms/djangoapps/verify_student/tests/test_views.py b/lms/djangoapps/verify_student/tests/test_views.py index 2627c40795..265d736c04 100644 --- a/lms/djangoapps/verify_student/tests/test_views.py +++ b/lms/djangoapps/verify_student/tests/test_views.py @@ -2,42 +2,41 @@ """ Tests of verify_student views. """ + import json import urllib from datetime import timedelta, datetime from uuid import uuid4 -from django.test.utils import override_settings -import mock -from mock import patch, Mock, ANY -from django.utils import timezone -import pytz import ddt -from django.test.client import Client -from django.test import TestCase +import httpretty +import mock +import pytz +from bs4 import BeautifulSoup +from mock import patch, Mock, ANY + from django.conf import settings from django.core.urlresolvers import reverse from django.core.exceptions import ObjectDoesNotExist from django.core import mail -import httpretty -from bs4 import BeautifulSoup -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory -from xmodule.modulestore.django import modulestore -from xmodule.modulestore import ModuleStoreEnum -from xmodule.modulestore.tests.factories import check_mongo_calls +from django.test import TestCase +from django.test.client import Client +from django.test.utils import override_settings +from django.utils import timezone + from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys.edx.locator import CourseLocator -from microsite_configuration import microsite -from openedx.core.djangoapps.user_api.accounts.api import get_account_settings +from course_modes.models import CourseMode +from course_modes.tests.factories import CourseModeFactory from commerce.tests import TEST_PAYMENT_DATA, TEST_API_URL, TEST_API_SIGNING_KEY +from embargo.test_utils import restrict_course +from microsite_configuration import microsite +from openedx.core.djangoapps.user_api.accounts.api import get_account_settings +from shoppingcart.models import Order, CertificateItem from student.tests.factories import UserFactory, CourseEnrollmentFactory from student.models import CourseEnrollment -from course_modes.tests.factories import CourseModeFactory -from course_modes.models import CourseMode -from shoppingcart.models import Order, CertificateItem -from embargo.test_utils import restrict_course +from util.date_utils import get_default_time_display from util.testing import UrlResetMixin from verify_student.views import ( checkout_with_ecommerce_service, @@ -48,7 +47,11 @@ from verify_student.models import ( SoftwareSecurePhotoVerification, VerificationCheckpoint, InCourseReverificationConfiguration, VerificationStatus ) -from util.date_utils import get_default_time_display +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.modulestore.django import modulestore +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.tests.factories import check_mongo_calls def mock_render_to_response(*args, **kwargs): @@ -1598,16 +1601,9 @@ class TestPhotoVerificationResultsCallback(ModuleStoreTestCase): mock_send_email.assert_called_once_with(self.user.id, subject, ANY) def create_reverification_xblock(self): - """ Create the reverification xblock - """ - # Create checkpoint - checkpoint = VerificationCheckpoint(course_id=self.course_id, checkpoint_name="midterm") - checkpoint.save() - - # Add a re-verification attempt - checkpoint.add_verification_attempt(self.attempt) - + Create the reverification XBlock. + """ # Create the 'edx-reverification-block' in course tree section = ItemFactory.create(parent=self.course, category='chapter', display_name='Test Section') subsection = ItemFactory.create(parent=section, category='sequential', display_name='Test Subsection') @@ -1618,13 +1614,20 @@ class TestPhotoVerificationResultsCallback(ModuleStoreTestCase): display_name='Test Verification Block' ) + # Create checkpoint + checkpoint = VerificationCheckpoint(course_id=self.course_id, checkpoint_location=reverification.location) + checkpoint.save() + + # Add a re-verification attempt + checkpoint.add_verification_attempt(self.attempt) + # Add a re-verification attempt status for the user - VerificationStatus.add_verification_status(checkpoint, self.user, "submitted", reverification.location) + VerificationStatus.add_verification_status(checkpoint, self.user, "submitted") class TestReverifyView(ModuleStoreTestCase): """ - Tests for the reverification views + Tests for the reverification views. """ def setUp(self): super(TestReverifyView, self).setUp() @@ -1675,7 +1678,6 @@ class TestInCourseReverifyView(ModuleStoreTestCase): Tests for the incourse reverification views. """ IMAGE_DATA = "abcd,1234" - MIDTERM = "midterm" def build_course(self): """ @@ -1695,7 +1697,7 @@ class TestInCourseReverifyView(ModuleStoreTestCase): section = ItemFactory.create(parent=self.course, category='chapter', display_name='Test Section') subsection = ItemFactory.create(parent=section, category='sequential', display_name='Test Subsection') vertical = ItemFactory.create(parent=subsection, category='vertical', display_name='Test Unit') - reverification = ItemFactory.create( + self.reverification = ItemFactory.create( parent=vertical, category='edx-reverification-block', display_name='Test Verification Block' @@ -1703,7 +1705,8 @@ class TestInCourseReverifyView(ModuleStoreTestCase): self.section_location = section.location self.subsection_location = subsection.location self.vertical_location = vertical.location - self.reverification_location = reverification.location + self.reverification_location = unicode(self.reverification.location) + self.reverification_assessment = self.reverification.related_assessment def setUp(self): super(TestInCourseReverifyView, self).setUp() @@ -1727,12 +1730,12 @@ class TestInCourseReverifyView(ModuleStoreTestCase): def test_incourse_reverify_feature_flag_get(self): self.config.enabled = False self.config.save() - response = self.client.get(self._get_url(self.course_key, self.MIDTERM)) + response = self.client.get(self._get_url(self.course_key, self.reverification_location)) self.assertEquals(response.status_code, 404) @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': True}) def test_incourse_reverify_invalid_course_get(self): - response = self.client.get(self._get_url("invalid/course/key", self.MIDTERM)) + response = self.client.get(self._get_url("invalid/course/key", self.reverification_location)) self.assertEquals(response.status_code, 404) @@ -1744,7 +1747,7 @@ class TestInCourseReverifyView(ModuleStoreTestCase): @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': True}) def test_incourse_reverify_initial_redirect_get(self): self._create_checkpoint() - response = self.client.get(self._get_url(self.course_key, self.MIDTERM)) + response = self.client.get(self._get_url(self.course_key, self.reverification_location)) url = reverse('verify_student_verify_now', kwargs={"course_id": unicode(self.course_key)}) self.assertRedirects(response, url) @@ -1752,20 +1755,24 @@ class TestInCourseReverifyView(ModuleStoreTestCase): @override_settings(SEGMENT_IO_LMS_KEY="foobar") @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': True, 'SEGMENT_IO_LMS': True}) def test_incourse_reverify_get(self): + """ + Test incourse reverification. + """ self._create_checkpoint() self._create_initial_verification() - response = self.client.get(self._get_url(self.course_key, self.MIDTERM)) + response = self.client.get(self._get_url(self.course_key, self.reverification_location)) self.assertEquals(response.status_code, 200) - #Verify Google Analytics event fired after successfully submiting the picture + # verify that Google Analytics event fires after successfully + # submitting the photo verification self.mock_tracker.track.assert_called_once_with( # pylint: disable=no-member self.user.id, # pylint: disable=no-member EVENT_NAME_USER_ENTERED_INCOURSE_REVERIFY_VIEW, { 'category': "verification", 'label': unicode(self.course_key), - 'checkpoint': self.MIDTERM + 'checkpoint': self.reverification_assessment }, context={ @@ -1776,22 +1783,19 @@ class TestInCourseReverifyView(ModuleStoreTestCase): self.mock_tracker.reset_mock() @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': True}) - @patch('verify_student.views.render_to_response', render_mock) - def test_invalid_checkpoint_post(self): - - response = self.client.post(self._get_url(self.course_key, self.MIDTERM)) - self.assertEquals(response.status_code, 200) - ((template, context), _kwargs) = render_mock.call_args # pylint: disable=unpacking-non-sequence - self.assertIn('incourse_reverify', template) - self.assertTrue(context['error']) + def test_checkpoint_post(self): + """Verify that POST requests including an invalid checkpoint location + results in a 400 response. + """ + response = self.client.post(self._get_url(self.course_key, self.reverification_location)) + self.assertEquals(response.status_code, 400) @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': True}) def test_incourse_reverify_initial_redirect_post(self): self._create_checkpoint() + response = self.client.post(self._get_url(self.course_key, self.reverification_location)) - response = self.client.post(self._get_url(self.course_key, self.MIDTERM)) url = reverse('verify_student_verify_now', kwargs={"course_id": unicode(self.course_key)}) - self.assertRedirects(response, url) @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': True}) @@ -1799,7 +1803,7 @@ class TestInCourseReverifyView(ModuleStoreTestCase): self._create_checkpoint() self._create_initial_verification() - response = self.client.post(self._get_url(self.course_key, self.MIDTERM), {"face_image": ""}) + response = self.client.post(self._get_url(self.course_key, self.reverification_location), {"face_image": ""}) self.assertEqual(response.status_code, 400) @override_settings(SEGMENT_IO_LMS_KEY="foobar") @@ -1808,19 +1812,22 @@ class TestInCourseReverifyView(ModuleStoreTestCase): self._create_checkpoint() self._create_initial_verification() - response = self.client.post(self._get_url(self.course_key, self.MIDTERM), {"face_image": self.IMAGE_DATA}) + response = self.client.post( + self._get_url(self.course_key, self.reverification_location), + {"face_image": self.IMAGE_DATA} + ) self.assertEqual(response.status_code, 200) - #Verify Google Analytics event fired after successfully submiting the picture + # test that Google Analytics event firs after successfully submitting + # photo verification self.mock_tracker.track.assert_called_once_with( # pylint: disable=no-member - self.user.id, # pylint: disable=no-member + self.user.id, EVENT_NAME_USER_SUBMITTED_INCOURSE_REVERIFY, { 'category': "verification", 'label': unicode(self.course_key), - 'checkpoint': self.MIDTERM + 'checkpoint': self.reverification_assessment }, - context={ 'Google Analytics': {'clientId': None} @@ -1833,37 +1840,43 @@ class TestInCourseReverifyView(ModuleStoreTestCase): self.config.enabled = False self.config.save() - response = self.client.post(self._get_url(self.course_key, self.MIDTERM)) + response = self.client.post(self._get_url(self.course_key, self.reverification_location)) self.assertEquals(response.status_code, 404) def _create_checkpoint(self): - """helper method for creating checkpoint""" - checkpoint = VerificationCheckpoint(course_id=self.course_key, checkpoint_name=self.MIDTERM) + """ + Helper method for creating a reverification checkpoint. + """ + checkpoint = VerificationCheckpoint(course_id=self.course_key, checkpoint_location=self.reverification_location) checkpoint.save() def _create_initial_verification(self): - """helper method for initial verification""" + """ + Helper method for initial verification. + """ attempt = SoftwareSecurePhotoVerification(user=self.user) attempt.mark_ready() attempt.save() attempt.submit() - def _get_url(self, course_key, checkpoint): - """contruct the url. + def _get_url(self, course_key, checkpoint_location): + """ + Construct the reverification url. + + Arguments: + course_key (unicode): The ID of the course + checkpoint_location (str): Location of verification checkpoint - Arguments: - course_key (unicode): The ID of the course. - checkpoint (str): The verification checkpoint Returns: url - """ - return reverse('verify_student_incourse_reverify', - kwargs={ - "course_id": unicode(course_key), - "checkpoint_name": checkpoint, - "usage_id": unicode(self.reverification_location) - }) + return reverse( + 'verify_student_incourse_reverify', + kwargs={ + "course_id": unicode(course_key), + "usage_id": checkpoint_location + } + ) class TestEmailMessageWithCustomICRVBlock(ModuleStoreTestCase): @@ -1875,8 +1888,6 @@ class TestEmailMessageWithCustomICRVBlock(ModuleStoreTestCase): """ Build up a course tree with a Reverificaiton xBlock. """ - # pylint: disable=attribute-defined-outside-init - self.course_key = SlashSeparatedCourseKey("Robot", "999", "Test_Course") self.course = CourseFactory.create(org='Robot', number='999', display_name='Test Course') self.due_date = datetime(2015, 6, 22, tzinfo=pytz.UTC) @@ -1901,40 +1912,41 @@ class TestEmailMessageWithCustomICRVBlock(ModuleStoreTestCase): self.section_location = section.location self.subsection_location = subsection.location self.vertical_location = vertical.location - self.reverification_location = self.reverification.location - self.assessment = "midterm" + self.reverification_location = unicode(self.reverification.location) + self.assessment = self.reverification.related_assessment self.re_verification_link = reverse( 'verify_student_incourse_reverify', args=( unicode(self.course_key), - unicode(self.assessment), - unicode(self.reverification_location) + self.reverification_location ) ) def setUp(self): + """ + Setup method for testing photo verification email messages. + """ super(TestEmailMessageWithCustomICRVBlock, self).setUp() self.build_course() self.check_point = VerificationCheckpoint.objects.create( - course_id=self.course.id, checkpoint_name=self.assessment + course_id=self.course.id, checkpoint_location=self.reverification_location ) - self.check_point.add_verification_attempt(SoftwareSecurePhotoVerification.objects.create(user=self.user)) VerificationStatus.add_verification_status( checkpoint=self.check_point, user=self.user, - status='submitted', - location_id=self.reverification_location + status='submitted' ) - self.attempt = SoftwareSecurePhotoVerification.objects.filter(user=self.user) def test_approved_email_message(self): - + """ + Test email message for approved photo verification. + """ subject, body = _compose_message_reverification_email( - self.course.id, self.user.id, "midterm", self.attempt, "approved", True + self.course.id, self.user.id, self.reverification_location, "approved", True ) self.assertIn( @@ -1944,13 +1956,12 @@ class TestEmailMessageWithCustomICRVBlock(ModuleStoreTestCase): ), body ) - self.assertIn("Re-verification Status", subject) def test_denied_email_message_with_valid_due_date_and_attempts_allowed(self): __, body = _compose_message_reverification_email( - self.course.id, self.user.id, "midterm", self.attempt, "denied", True + self.course.id, self.user.id, self.reverification_location, "denied", True ) self.assertIn( @@ -1975,7 +1986,7 @@ class TestEmailMessageWithCustomICRVBlock(ModuleStoreTestCase): return_value = datetime(2016, 1, 1, tzinfo=timezone.utc) with patch.object(timezone, 'now', return_value=return_value): __, body = _compose_message_reverification_email( - self.course.id, self.user.id, "midterm", self.attempt, "denied", True + self.course.id, self.user.id, self.reverification_location, "denied", True ) self.assertIn( @@ -1991,7 +2002,7 @@ class TestEmailMessageWithCustomICRVBlock(ModuleStoreTestCase): def test_check_num_queries(self): # Get the re-verification block to check the call made with check_mongo_calls(2): - ver_block = modulestore().get_item(self.reverification_location) + ver_block = modulestore().get_item(self.reverification.location) # Expect that the verification block is fetched self.assertIsNotNone(ver_block) @@ -2006,8 +2017,6 @@ class TestEmailMessageWithDefaultICRVBlock(ModuleStoreTestCase): """ Build up a course tree with a Reverificaiton xBlock. """ - # pylint: disable=attribute-defined-outside-init - self.course_key = SlashSeparatedCourseKey("Robot", "999", "Test_Course") self.course = CourseFactory.create(org='Robot', number='999', display_name='Test Course') @@ -2030,15 +2039,14 @@ class TestEmailMessageWithDefaultICRVBlock(ModuleStoreTestCase): self.section_location = section.location self.subsection_location = subsection.location self.vertical_location = vertical.location - self.reverification_location = self.reverification.location - self.assessment = "midterm" + self.reverification_location = unicode(self.reverification.location) + self.assessment = self.reverification.related_assessment self.re_verification_link = reverse( 'verify_student_incourse_reverify', args=( unicode(self.course_key), - unicode(self.assessment), - unicode(self.reverification_location) + self.reverification_location ) ) @@ -2047,7 +2055,7 @@ class TestEmailMessageWithDefaultICRVBlock(ModuleStoreTestCase): self.build_course() self.check_point = VerificationCheckpoint.objects.create( - course_id=self.course.id, checkpoint_name=self.assessment + course_id=self.course.id, checkpoint_location=self.reverification_location ) self.check_point.add_verification_attempt(SoftwareSecurePhotoVerification.objects.create(user=self.user)) self.attempt = SoftwareSecurePhotoVerification.objects.filter(user=self.user) @@ -2057,12 +2065,11 @@ class TestEmailMessageWithDefaultICRVBlock(ModuleStoreTestCase): VerificationStatus.add_verification_status( checkpoint=self.check_point, user=self.user, - status='submitted', - location_id=self.reverification_location + status='submitted' ) __, body = _compose_message_reverification_email( - self.course.id, self.user.id, "midterm", self.attempt, "denied", True + self.course.id, self.user.id, self.reverification_location, "denied", True ) self.assertIn( @@ -2082,11 +2089,10 @@ class TestEmailMessageWithDefaultICRVBlock(ModuleStoreTestCase): VerificationStatus.add_verification_status( checkpoint=self.check_point, user=self.user, - status='submitted', - location_id=self.reverification_location + status='submitted' ) __, body = _compose_message_reverification_email( - self.course.id, self.user.id, "midterm", self.attempt, "denied", True + self.course.id, self.user.id, self.reverification_location, "denied", True ) self.assertIn( @@ -2104,12 +2110,11 @@ class TestEmailMessageWithDefaultICRVBlock(ModuleStoreTestCase): VerificationStatus.add_verification_status( checkpoint=self.check_point, user=self.user, - status='error', - location_id=self.reverification_location + status='error' ) __, body = _compose_message_reverification_email( - self.course.id, self.user.id, "midterm", self.attempt, "denied", True + self.course.id, self.user.id, self.reverification_location, "denied", True ) self.assertIn( @@ -2131,6 +2136,6 @@ class TestEmailMessageWithDefaultICRVBlock(ModuleStoreTestCase): def test_error_on_compose_email(self): resp = _compose_message_reverification_email( - self.course.id, self.user.id, "midterm", self.attempt, "denied", True + self.course.id, self.user.id, u'i4x://edX/DemoX/edx-reverification-block/invalid_location', "denied", True ) self.assertIsNone(resp) diff --git a/lms/djangoapps/verify_student/urls.py b/lms/djangoapps/verify_student/urls.py index fd159e4dfa..59b907f7c2 100644 --- a/lms/djangoapps/verify_student/urls.py +++ b/lms/djangoapps/verify_student/urls.py @@ -116,9 +116,8 @@ urlpatterns = patterns( # Users are sent to this end-point from within courseware # to re-verify their identities by re-submitting face photos. url( - r'^reverify/{course_id}/{checkpoint}/{usage_id}/$'.format( + r'^reverify/{course_id}/{usage_id}/$'.format( course_id=settings.COURSE_ID_PATTERN, - checkpoint=settings.CHECKPOINT_PATTERN, usage_id=settings.USAGE_ID_PATTERN ), views.InCourseReverifyView.as_view(), diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index 2508a7545e..841a8a0d3f 100644 --- a/lms/djangoapps/verify_student/views.py +++ b/lms/djangoapps/verify_student/views.py @@ -1,69 +1,66 @@ """ Views for the verification flow - """ + +import datetime +import decimal import json import logging -import decimal -import datetime from collections import namedtuple - - from pytz import UTC -from django.utils import timezone from ipware.ip import get_ip + from django.conf import settings +from django.contrib.auth.decorators import login_required +from django.core.mail import send_mail from django.core.urlresolvers import reverse from django.http import ( HttpResponse, HttpResponseBadRequest, HttpResponseRedirect, Http404 ) +from django.contrib.auth.models import User from django.shortcuts import redirect +from django.utils import timezone +from django.utils.decorators import method_decorator +from django.utils.translation import ugettext as _, ugettext_lazy from django.views.decorators.csrf import csrf_exempt from django.views.decorators.http import require_POST from django.views.generic.base import View, RedirectView -from django.utils.decorators import method_decorator -from django.utils.translation import ugettext as _, ugettext_lazy -from django.contrib.auth.decorators import login_required -from django.core.mail import send_mail -from ecommerce_api_client.exceptions import SlumberBaseException + +import analytics +from eventtracking import tracker from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys import InvalidKeyError -from xmodule.modulestore.django import modulestore -from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem -from edxmako.shortcuts import render_to_response, render_to_string -from openedx.core.djangoapps.user_api.accounts.api import get_account_settings, update_account_settings -from openedx.core.djangoapps.user_api.accounts import NAME_MIN_LENGTH -from openedx.core.djangoapps.user_api.errors import UserNotFound, AccountValidationError from commerce import ecommerce_api_client from course_modes.models import CourseMode +from courseware.url_helpers import get_redirect_url +from ecommerce_api_client.exceptions import SlumberBaseException +from edxmako.shortcuts import render_to_response, render_to_string +from embargo import api as embargo_api +from microsite_configuration import microsite +from openedx.core.djangoapps.user_api.accounts import NAME_MIN_LENGTH +from openedx.core.djangoapps.user_api.accounts.api import get_account_settings, update_account_settings +from openedx.core.djangoapps.user_api.errors import UserNotFound, AccountValidationError from student.models import CourseEnrollment -from student.views import reverification_info from shoppingcart.models import Order, CertificateItem from shoppingcart.processors import ( get_signed_purchase_params, get_purchase_endpoint ) +from verify_student.ssencrypt import has_valid_signature from verify_student.models import ( SoftwareSecurePhotoVerification, VerificationCheckpoint, VerificationStatus, - InCourseReverificationConfiguration) -from reverification.models import MidcourseReverificationWindow -import ssencrypt -from .exceptions import WindowExpiredException -from microsite_configuration import microsite -from embargo import api as embargo_api + InCourseReverificationConfiguration +) from util.json_request import JsonResponse from util.date_utils import get_default_time_display -from eventtracking import tracker -import analytics -from courseware.url_helpers import get_redirect_url -from django.contrib.auth.models import User +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem + log = logging.getLogger(__name__) - - EVENT_NAME_USER_ENTERED_INCOURSE_REVERIFY_VIEW = 'edx.bi.reverify.started' EVENT_NAME_USER_SUBMITTED_INCOURSE_REVERIFY = 'edx.bi.reverify.submitted' @@ -859,40 +856,40 @@ def submit_photos_for_verification(request): def _compose_message_reverification_email( - course_key, user_id, relates_assessment, photo_verification, status, is_secure + course_key, user_id, related_assessment_location, status, is_secure ): # pylint: disable=invalid-name - """ Composes subject and message for email + """ + Compose subject and message for photo reverification email. Args: course_key(CourseKey): CourseKey object user_id(str): User Id - relates_assessment(str): related assessment name - photo_verification(QuerySet/SoftwareSecure): A query set of SoftwareSecure objects or SoftwareSecure objec - status(str): approval status + related_assessment_location(str): Location of reverification XBlock + photo_verification(QuerySet): Queryset of SoftwareSecure objects + status(str): Approval status is_secure(Bool): Is running on secure protocol or not Returns: None if any error occurred else Tuple of subject and message strings """ try: - location_id = VerificationStatus.get_location_id(photo_verification) - usage_key = UsageKey.from_string(location_id) + usage_key = UsageKey.from_string(related_assessment_location) + reverification_block = modulestore().get_item(usage_key) + course = modulestore().get_course(course_key) redirect_url = get_redirect_url(course_key, usage_key.replace(course_key=course_key)) subject = "Re-verification Status" - context = { "status": status, "course_name": course.display_name_with_default, - "assessment": relates_assessment, + "assessment": reverification_block.related_assessment, "courseware_url": redirect_url } - reverification_block = modulestore().get_item(usage_key) # Allowed attempts is 1 if not set on verification block allowed_attempts = 1 if reverification_block.attempts == 0 else reverification_block.attempts - user_attempts = VerificationStatus.get_user_attempts(user_id, course_key, relates_assessment, location_id) + user_attempts = VerificationStatus.get_user_attempts(user_id, course_key, related_assessment_location) left_attempts = allowed_attempts - user_attempts is_attempt_allowed = left_attempts > 0 verification_open = True @@ -911,8 +908,7 @@ def _compose_message_reverification_email( 'verify_student_incourse_reverify', args=( unicode(course_key), - unicode(relates_assessment), - unicode(location_id) + related_assessment_location ) ) context["reverify_link"] = re_verification_link @@ -972,7 +968,7 @@ def results_callback(request): "Date": request.META.get("HTTP_DATE", "") } - sig_valid = ssencrypt.has_valid_signature( + has_valid_signature( "POST", headers, body_dict, @@ -1029,10 +1025,10 @@ def results_callback(request): if checkpoints: user_id = attempt.user.id course_key = checkpoints[0].course_id - relates_assessment = checkpoints[0].checkpoint_name + related_assessment_location = checkpoints[0].checkpoint_location subject, message = _compose_message_reverification_email( - course_key, user_id, relates_assessment, attempt, status, request.is_secure() + course_key, user_id, related_assessment_location, status, request.is_secure() ) _send_email(user_id, subject, message) @@ -1122,11 +1118,20 @@ class InCourseReverifyView(View): Does not need to worry about pricing """ @method_decorator(login_required) - def get(self, request, course_id, checkpoint_name, usage_id): - """ Display the view for face photo submission""" - # Check the in-course re-verification is enabled or not + def get(self, request, course_id, usage_id): + """Display the view for face photo submission. + Args: + request(HttpRequest): HttpRequest object + course_id(str): A string of course id + usage_id(str): Location of Reverification XBlock in courseware + + Returns: + HttpResponse + """ + # Check that in-course re-verification is enabled or not incourse_reverify_enabled = InCourseReverificationConfiguration.current().enabled + if not incourse_reverify_enabled: log.error( u"In-course reverification is not enabled. " @@ -1139,49 +1144,49 @@ class InCourseReverifyView(View): course_key = CourseKey.from_string(course_id) course = modulestore().get_course(course_key) if course is None: - log.error(u"Could not find course %s for in-course reverification.", course_key) + log.error(u"Could not find course '%s' for in-course reverification.", course_key) raise Http404 - checkpoint = VerificationCheckpoint.get_verification_checkpoint(course_key, checkpoint_name) + checkpoint = VerificationCheckpoint.get_verification_checkpoint(course_key, usage_id) if checkpoint is None: log.error( u"No verification checkpoint exists for the " - u"course %s and checkpoint name %s.", - course_key, checkpoint_name + u"course '%s' and checkpoint location '%s'.", + course_key, usage_id ) raise Http404 - init_verification = SoftwareSecurePhotoVerification.get_initial_verification(user) - if not init_verification: + initial_verification = SoftwareSecurePhotoVerification.get_initial_verification(user) + if not initial_verification: return self._redirect_no_initial_verification(user, course_key) # emit the reverification event self._track_reverification_events( - EVENT_NAME_USER_ENTERED_INCOURSE_REVERIFY_VIEW, user.id, course_id, checkpoint_name + EVENT_NAME_USER_ENTERED_INCOURSE_REVERIFY_VIEW, user.id, course_id, checkpoint.checkpoint_name ) context = { 'course_key': unicode(course_key), 'course_name': course.display_name_with_default, - 'checkpoint_name': checkpoint_name, + 'checkpoint_name': checkpoint.checkpoint_name, 'platform_name': settings.PLATFORM_NAME, 'usage_id': usage_id } return render_to_response("verify_student/incourse_reverify.html", context) @method_decorator(login_required) - def post(self, request, course_id, checkpoint_name, usage_id): - """Submits the re-verification attempt to SoftwareSecure + def post(self, request, course_id, usage_id): + """Submits the re-verification attempt to SoftwareSecure. Args: request(HttpRequest): HttpRequest object course_id(str): Course Id - checkpoint_name(str): Checkpoint name + usage_id(str): Location of Reverification XBlock in courseware Returns: HttpResponse with status_code 400 if photo is missing or any error - or redirect to the verification flow if initial verification doesn't exist otherwise - HttpsResponse with status code 200 + or redirect to the verification flow if initial verification + doesn't exist otherwise HttpResponse with status code 200 """ # Check the in-course re-verification is enabled or not incourse_reverify_enabled = InCourseReverificationConfiguration.current().enabled @@ -1196,20 +1201,18 @@ class InCourseReverifyView(View): raise Http404(u"Invalid course_key or usage_key") course = modulestore().get_course(course_key) - checkpoint = VerificationCheckpoint.get_verification_checkpoint(course_key, checkpoint_name) + if course is None: + log.error(u"Invalid course id '%s'", course_id) + return HttpResponseBadRequest(_("Invalid course location.")) + + checkpoint = VerificationCheckpoint.get_verification_checkpoint(course_key, usage_id) if checkpoint is None: - log.error("Checkpoint is not defined. Could not submit verification attempt for user %s", - request.user.id) - context = { - 'course_key': unicode(course_key), - 'course_name': course.display_name_with_default, - 'checkpoint_name': checkpoint_name, - 'error': True, - 'errorMsg': _("No checkpoint found"), - 'platform_name': settings.PLATFORM_NAME, - 'usage_id': usage_id - } - return render_to_response("verify_student/incourse_reverify.html", context) + log.error( + u"Checkpoint is not defined. Could not submit verification attempt" + u" for user '%s', course '%s' and checkpoint location '%s'.", + request.user.id, course_key, usage_id + ) + return HttpResponseBadRequest(_("Invalid checkpoint location.")) init_verification = SoftwareSecurePhotoVerification.get_initial_verification(user) if not init_verification: @@ -1220,47 +1223,51 @@ class InCourseReverifyView(View): request.user, request.POST['face_image'], init_verification.photo_id_key ) checkpoint.add_verification_attempt(attempt) - VerificationStatus.add_verification_status(checkpoint, user, "submitted", usage_id) + VerificationStatus.add_verification_status(checkpoint, user, "submitted") # emit the reverification event self._track_reverification_events( - EVENT_NAME_USER_SUBMITTED_INCOURSE_REVERIFY, user.id, course_id, checkpoint_name + EVENT_NAME_USER_SUBMITTED_INCOURSE_REVERIFY, user.id, course_id, checkpoint.checkpoint_name ) - try: - redirect_url = get_redirect_url(course_key, usage_key) - except (ItemNotFoundError, NoPathToItem): - log.warning( - u"Could not find redirect URL for location %s in course %s", - course_key, usage_key - ) - redirect_url = reverse("courseware", args=(unicode(course_key),)) + redirect_url = get_redirect_url(course_key, usage_key) + response = JsonResponse({'url': redirect_url}) + + except (ItemNotFoundError, NoPathToItem): + log.warning(u"Could not find redirect URL for location %s in course %s", course_key, usage_key) + redirect_url = reverse("courseware", args=(unicode(course_key),)) + response = JsonResponse({'url': redirect_url}) - return JsonResponse({'url': redirect_url}) except Http404 as expt: log.exception("Invalid location during photo verification.") - return HttpResponseBadRequest(expt.message) + response = HttpResponseBadRequest(expt.message) + except IndexError: log.exception("Invalid image data during photo verification.") - return HttpResponseBadRequest(_("Invalid image data during photo verification.")) + response = HttpResponseBadRequest(_("Invalid image data during photo verification.")) + except Exception: # pylint: disable=broad-except log.exception("Could not submit verification attempt for user %s.", request.user.id) msg = _("Could not submit photos") - return HttpResponseBadRequest(msg) + response = HttpResponseBadRequest(msg) + + return response def _track_reverification_events(self, event_name, user_id, course_id, checkpoint): # pylint: disable=invalid-name - """Track re-verification events for user against course checkpoints + """Track re-verification events for a user against a reverification + checkpoint of a course. Arguments: - user_id (str): The ID of the user generting the certificate. - course_id (unicode): id associated with the course - checkpoint (str): checkpoint name + event_name (str): Name of event being tracked + user_id (str): The ID of the user + course_id (unicode): ID associated with the course + checkpoint (str): Checkpoint name + Returns: None - """ log.info( - u"In-course reverification: event %s occurred for user %s in course %s at checkpoint %s", + u"In-course reverification: event %s occurred for user '%s' in course '%s' at checkpoint '%s'", event_name, user_id, course_id, checkpoint ) diff --git a/lms/static/js/verify_student/incourse_reverify.js b/lms/static/js/verify_student/incourse_reverify.js index 61bb2e0ae6..cad681e848 100644 --- a/lms/static/js/verify_student/incourse_reverify.js +++ b/lms/static/js/verify_student/incourse_reverify.js @@ -20,7 +20,6 @@ return new edx.verify_student.InCourseReverifyView({ courseKey: el.data('course-key'), - checkpointName: el.data('checkpoint-name'), platformName: el.data('platform-name'), usageId: el.data('usage-id'), errorModel: errorView.model diff --git a/lms/static/js/verify_student/models/reverification_model.js b/lms/static/js/verify_student/models/reverification_model.js index 566dfe375f..def5f8c110 100644 --- a/lms/static/js/verify_student/models/reverification_model.js +++ b/lms/static/js/verify_student/models/reverification_model.js @@ -16,7 +16,6 @@ var edx = edx || {}; defaults: { courseKey: '', - checkpointName: '', faceImage: '', usageId: '' }, @@ -28,9 +27,8 @@ var edx = edx || {}; face_image: model.get( 'faceImage' ) }, url = _.str.sprintf( - '/verify_student/reverify/%(courseKey)s/%(checkpointName)s/%(usageId)s/', { + '/verify_student/reverify/%(courseKey)s/%(usageId)s/', { courseKey: model.get('courseKey'), - checkpointName: model.get('checkpointName'), usageId: model.get('usageId') } ); diff --git a/lms/static/js/verify_student/views/incourse_reverify_view.js b/lms/static/js/verify_student/views/incourse_reverify_view.js index 621a530f9d..ac8c29088d 100644 --- a/lms/static/js/verify_student/views/incourse_reverify_view.js +++ b/lms/static/js/verify_student/views/incourse_reverify_view.js @@ -26,14 +26,12 @@ this.errorModel = obj.errorModel || null; this.courseKey = obj.courseKey || null; - this.checkpointName = obj.checkpointName || null; this.platformName = obj.platformName || null; this.usageId = obj.usageId || null; this.model = new edx.verify_student.ReverificationModel({ courseKey: this.courseKey, - checkpointName: this.checkpointName, usageId: this.usageId }); @@ -47,7 +45,6 @@ $( this.templateId ).html(), { courseKey: this.courseKey, - checkpointName: this.checkpointName, platformName: this.platformName } ); diff --git a/lms/templates/verify_student/incourse_reverify.html b/lms/templates/verify_student/incourse_reverify.html index 723e8b7363..00e5ac5c4f 100644 --- a/lms/templates/verify_student/incourse_reverify.html +++ b/lms/templates/verify_student/incourse_reverify.html @@ -43,7 +43,6 @@ from verify_student.views import PayAndVerifyView
diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 010d875c54..f1e9f03d4c 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -50,7 +50,7 @@ git+https://github.com/hmarr/django-debug-toolbar-mongo.git@b0686a76f1ce3532088c git+https://github.com/edx/edx-lint.git@8bf82a32ecb8598c415413df66f5232ab8d974e9#egg=edx_lint==0.2.1 -e git+https://github.com/edx/xblock-utils.git@db22bc40fd2a75458a3c66d057f88aff5a7383e6#egg=xblock-utils -e git+https://github.com/edx-solutions/xblock-google-drive.git@138e6fa0bf3a2013e904a085b9fed77dab7f3f21#egg=xblock-google-drive --e git+https://github.com/edx/edx-reverification-block.git@2ff0d21f6614874067168bd244e68d8215041f3b#egg=edx-reverification-block +-e git+https://github.com/edx/edx-reverification-block.git@03da85753d5f563a22c1282c0e89fcb2e828b8c1#egg=edx-reverification-block git+https://github.com/edx/ecommerce-api-client.git@1.0.0#egg=ecommerce-api-client==1.0.0 # Third Party XBlocks