Merge pull request #9093 from edx/will/ecom-1905

Allow users to submit initial verification at in-course checkpoints.

lms/djangoapps/verify_student/admin.py

 from ratelimitbackend import admin
-from config_models.admin import ConfigurationModelAdmin
 from verify_student.models import (
     SoftwareSecurePhotoVerification,
-    InCourseReverificationConfiguration,
     VerificationStatus,
     SkippedReverification,
 )
@@ -40,10 +38,6 @@ class VerificationStatusAdmin(admin.ModelAdmin):
             return self.readonly_fields + ('status', 'checkpoint', 'user', 'response', 'error')
         return self.readonly_fields
 
-    def has_delete_permission(self, request, obj=None):
-        """The verification status table is append-only. """
-        return False
-
 
 class SkippedReverificationAdmin(admin.ModelAdmin):
     """Admin for the SkippedReverification table. """
@@ -58,6 +52,5 @@ class SkippedReverificationAdmin(admin.ModelAdmin):
 
 
 admin.site.register(SoftwareSecurePhotoVerification, SoftwareSecurePhotoVerificationAdmin)
-admin.site.register(InCourseReverificationConfiguration, ConfigurationModelAdmin)
 admin.site.register(SkippedReverification, SkippedReverificationAdmin)
 admin.site.register(VerificationStatus, VerificationStatusAdmin)

lms/djangoapps/verify_student/image.py

+"""
+Image encoding helpers for the verification app.
+"""
+import logging
+
+
+log = logging.getLogger(__name__)
+
+
+class InvalidImageData(Exception):
+    """
+    The provided image data could not be decoded.
+    """
+    pass
+
+
+def decode_image_data(data):
+    """
+    Decode base64-encoded image data.
+
+    Arguments:
+        data (str): The raw image data, base64-encoded.
+
+    Returns:
+        str
+
+    Raises:
+        InvalidImageData: The image data could not be decoded.
+
+    """
+    try:
+        return (data.split(",")[1]).decode("base64")
+    except (IndexError, UnicodeEncodeError):
+        log.exception("Could not decode image data")
+        raise InvalidImageData

lms/djangoapps/verify_student/services.py

@@ -60,10 +60,9 @@ class ReverificationService(object):
             Re-verification link
         """
         course_key = CourseKey.from_string(course_id)
-        VerificationCheckpoint.objects.get_or_create(
-            course_id=course_key,
-            checkpoint_location=related_assessment_location
-        )
+
+        # Get-or-create the verification checkpoint
+        VerificationCheckpoint.get_or_create_verification_checkpoint(course_key, related_assessment_location)
 
         re_verification_link = reverse(
             'verify_student_incourse_reverify',

lms/djangoapps/verify_student/tests/fake_software_secure.py

@@ -40,7 +40,7 @@ class SoftwareSecureFakeView(View):
         }
 
         try:
-            most_recent = SoftwareSecurePhotoVerification.original_verification(user)
+            most_recent = SoftwareSecurePhotoVerification.objects.filter(user=user).order_by("-updated_at")[0]
             context["receipt_id"] = most_recent.receipt_id
         except:  # pylint: disable=bare-except
             pass

lms/djangoapps/verify_student/tests/test_models.py

@@ -220,18 +220,6 @@ class TestPhotoVerification(ModuleStoreTestCase):
 
         return attempt
 
-    def test_fetch_photo_id_image(self):
-        user = UserFactory.create()
-        orig_attempt = SoftwareSecurePhotoVerification(user=user)
-        orig_attempt.save()
-
-        old_key = orig_attempt.photo_id_key
-
-        new_attempt = SoftwareSecurePhotoVerification(user=user)
-        new_attempt.save()
-        new_attempt.fetch_photo_id_image()
-        assert_equals(new_attempt.photo_id_key, old_key)
-
     def test_submissions(self):
         """Test that we set our status correctly after a submission."""
         # Basic case, things go well.
@@ -501,7 +489,7 @@ class VerificationCheckpointTest(ModuleStoreTestCase):
         )
 
     @ddt.data('midterm', 'final')
-    def test_get_verification_checkpoint(self, checkpoint):
+    def test_get_or_create_verification_checkpoint(self, checkpoint):
         """
         Test that a reverification checkpoint is created properly.
         """
@@ -514,26 +502,40 @@ class VerificationCheckpointTest(ModuleStoreTestCase):
             checkpoint_location=checkpoint_location
         )
         self.assertEqual(
-            VerificationCheckpoint.get_verification_checkpoint(self.course.id, checkpoint_location),
+            VerificationCheckpoint.get_or_create_verification_checkpoint(self.course.id, checkpoint_location),
             verification_checkpoint
         )
 
-    def test_get_verification_checkpoint_for_not_existing_values(self):
-        """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_location=self.checkpoint_midterm)
+    def test_get_or_create_verification_checkpoint_for_not_existing_values(self):
+        # Retrieving a checkpoint that doesn't yet exist will create it
+        location = u'i4x://edX/DemoX/edx-reverification-block/invalid_location'
+        checkpoint = VerificationCheckpoint.get_or_create_verification_checkpoint(self.course.id, location)
 
-        # 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
+        self.assertIsNot(checkpoint, None)
+        self.assertEqual(checkpoint.course_id, self.course.id)
+        self.assertEqual(checkpoint.checkpoint_location, location)
+
+    def test_get_or_create_integrity_error(self):
+        # Create the checkpoint
+        VerificationCheckpoint.objects.create(
+            course_id=self.course.id,
+            checkpoint_location=self.checkpoint_midterm,
         )
 
+        # Simulate that the get-or-create operation raises an IntegrityError
+        # This can happen when two processes both try to get-or-create at the same time
+        # when the database is set to REPEATABLE READ.
+        with patch.object(VerificationCheckpoint.objects, "get_or_create") as mock_get_or_create:
+            mock_get_or_create.side_effect = IntegrityError
+            checkpoint = VerificationCheckpoint.get_or_create_verification_checkpoint(
+                self.course.id,
+                self.checkpoint_midterm
+            )
+
+        # The checkpoint should be retrieved without error
+        self.assertEqual(checkpoint.course_id, self.course.id)
+        self.assertEqual(checkpoint.checkpoint_location, self.checkpoint_midterm)
+
     def test_unique_together_constraint(self):
         """
         Test the unique together constraint.

lms/djangoapps/verify_student/urls.py

@@ -91,7 +91,7 @@ urlpatterns = patterns(
 
     url(
         r'^submit-photos/$',
-        views.submit_photos_for_verification,
+        views.SubmitPhotosView.as_view(),
         name="verify_student_submit_photos"
     ),
 

lms/envs/common.py

@@ -1331,7 +1331,7 @@ incourse_reverify_js = [
     'js/verify_student/views/error_view.js',
     'js/verify_student/views/image_input_view.js',
     'js/verify_student/views/webcam_photo_view.js',
-    'js/verify_student/models/reverification_model.js',
+    'js/verify_student/models/verification_model.js',
     'js/verify_student/views/incourse_reverify_view.js',
     'js/verify_student/incourse_reverify.js',
 ]

lms/static/js/verify_student/models/reverification_model.js

-/**
- * Model for a reverification attempt.
- *
- * The re-verification model is responsible for
- * storing face photo image data and submitting
- * it back to the server.
- */
-var edx = edx || {};
-
-(function( $, _, Backbone ) {
-    'use strict';
-
-    edx.verify_student = edx.verify_student || {};
-
-    edx.verify_student.ReverificationModel = Backbone.Model.extend({
-
-        defaults: {
-            courseKey: '',
-            faceImage: '',
-            usageId: ''
-        },
-
-        sync: function( method ) {
-            var model = this;
-            var headers = { 'X-CSRFToken': $.cookie( 'csrftoken' ) },
-                data = {
-                    face_image: model.get( 'faceImage' )
-                },
-                url = _.str.sprintf(
-                    '/verify_student/reverify/%(courseKey)s/%(usageId)s/', {
-                        courseKey: model.get('courseKey'),
-                        usageId: model.get('usageId')
-                    }
-                );
-
-            $.ajax({
-                url: url,
-                type: 'POST',
-                data: data,
-                headers: headers,
-                success: function(response) {
-                    model.trigger( 'sync', response.url);
-                },
-                error: function( error ) {
-                    model.trigger( 'error', error );
-                }
-            });
-        }
-    });
-
-})( jQuery, _, Backbone );

lms/static/js/verify_student/models/verification_model.js

@@ -8,7 +8,7 @@
  */
  var edx = edx || {};
 
- (function( $, _, Backbone ) {
+ (function( $, Backbone ) {
     'use strict';
 
     edx.verify_student = edx.verify_student || {};
@@ -16,22 +16,40 @@
     edx.verify_student.VerificationModel = Backbone.Model.extend({
 
         defaults: {
+            // If provided, change the user's full name when submitting photos.
             fullName: null,
+
+            // Image data for the user's face photo.
             faceImage: "",
-            identificationImage: ""
+
+            // Image data for the user's ID photo.
+            // In the case of an in-course reverification, we won't
+            // need to send this because we'll send the ID photo that
+            // the user submitted with the initial verification attempt.
+            identificationImage: null,
+
+            // If the verification attempt is associated with a checkpoint
+            // in a course, we send the the course and checkpoint location.
+            courseKey: null,
+            checkpoint: null,
         },
 
         sync: function( method, model ) {
             var headers = { 'X-CSRFToken': $.cookie( 'csrftoken' ) },
-                data = {
-                    face_image: model.get( 'faceImage' ),
-                    photo_id_image: model.get( 'identificationImage' )
-                };
+                data = {};
+
+            data.face_image = model.get("faceImage");
+
+            // The ID photo is optional, since in-course reverification
+            // re-uses the image from the initial verification attempt.
+            if (model.has("identificationImage")) {
+                data.photo_id_image = model.get("identificationImage");
+            }
 
             // Full name is an optional parameter; if not provided,
             // it won't be changed.
-            if ( !_.isEmpty( model.get( 'fullName' ) ) ) {
-                data.full_name = model.get( 'fullName' );
+            if (model.has("fullName")) {
+                data.full_name = model.get("fullName");
 
                 // Track the user's decision to change the name on their account
                 window.analytics.track( 'edx.bi.user.full_name.changed', {
@@ -39,6 +57,14 @@
                 });
             }
 
+            // If the user entered the verification flow from a checkpoint
+            // in a course, we need to send the course and checkpoint
+            // location to associate the attempt with the checkpoint.
+            if (model.has("courseKey") && model.has("checkpoint")) {
+                data.course_key = model.get("courseKey");
+                data.checkpoint = model.get("checkpoint");
+            }
+
             // Submit the request to the server,
             // triggering events on success and error.
             $.ajax({
@@ -46,8 +72,8 @@
                 type: 'POST',
                 data: data,
                 headers: headers,
-                success: function() {
-                    model.trigger( 'sync' );
+                success: function( response ) {
+                    model.trigger( 'sync', response.url );
                 },
                 error: function( error ) {
                     model.trigger( 'error', error );
@@ -56,4 +82,4 @@
         }
     });
 
- })( jQuery, _, Backbone );
+ })( jQuery, Backbone );

lms/static/js/verify_student/pay_and_verify.js

@@ -34,6 +34,8 @@ var edx = edx || {};
         errorModel: errorView.model,
         displaySteps: el.data('display-steps'),
         currentStep: el.data('current-step'),
+        courseKey: el.data('course-key'),
+        checkpointLocation: el.data('checkpoint-location'),
         stepInfo: {
             'intro-step': {
                 courseName: el.data('course-name'),

lms/static/js/verify_student/views/incourse_reverify_view.js

@@ -30,9 +30,9 @@
             this.usageId = obj.usageId || null;
 
 
-            this.model = new edx.verify_student.ReverificationModel({
+            this.model = new edx.verify_student.VerificationModel({
                 courseKey: this.courseKey,
-                usageId: this.usageId
+                checkpoint: this.usageId
             });
 
             this.listenTo( this.model, 'sync', _.bind( this.handleSubmitPhotoSuccess, this ));
@@ -74,8 +74,7 @@
         },
 
         handleSubmitPhotoSuccess: function(redirect_url) {
-            // Eventually this will be a redirect back into the courseware,
-            // but for now we can return to the student dashboard.
+            // Redirect back to the courseware at the checkpoint location
             window.location.href = redirect_url;
         },
 

lms/static/js/verify_student/views/pay_and_verify_view.js

@@ -27,6 +27,9 @@ var edx = edx || {};
         initialize: function( obj ) {
             this.errorModel = obj.errorModel || null;
             this.displaySteps = obj.displaySteps || [];
+            this.courseKey = obj.courseKey || null;
+            this.checkpointLocation = obj.checkpointLocation || null;
+
             this.initializeStepViews( obj.stepInfo || {} );
             this.currentStepIndex = _.indexOf(
                 _.pluck( this.displaySteps, 'name' ),
@@ -61,7 +64,14 @@ var edx = edx || {};
             // among the different steps.  This allows
             // one step to save photos and another step
             // to submit them.
-            verificationModel = new edx.verify_student.VerificationModel();
+            //
+            // We also pass in the course key and checkpoint location.
+            // If we've been provided with a checkpoint in the courseware,
+            // this will associate the verification attempt with the checkpoint.
+            verificationModel = new edx.verify_student.VerificationModel({
+                courseKey: this.courseKey,
+                checkpoint: this.checkpointLocation
+            });
 
             for ( i = 0; i < this.displaySteps.length; i++ ) {
                 stepName = this.displaySteps[i].name;

lms/templates/verify_student/pay_and_verify.html

@@ -75,6 +75,13 @@ from verify_student.views import PayAndVerifyView
       data-already-verified='${already_verified}'
       data-verification-good-until='${verification_good_until}'
       data-capture-sound='${capture_sound}'
+
+      # If we reached the verification flow from an in-course checkpoint,
+      # then pass the checkpoint location in so that we can associate
+      # the attempt with the checkpoint on submission.
+      % if checkpoint_location is not None:
+        data-checkpoint-location='${checkpoint_location}'
+      % endif
     ></div>
 
     % if is_active:

requirements/edx/base.txt

@@ -136,6 +136,7 @@ freezegun==0.1.11
 lettuce==0.2.20
 mock-django==0.6.6
 mock==1.0.1
+moto==0.3.1
 nose-exclude
 nose-ignore-docstring
 nosexcover==1.0.7