Merge pull request #8231 from edx/zub/story/ecom-1612-change-icrv-block-fields

use reverification xblock location as an identifier of reverification…

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. """

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)

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
         )

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(),

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

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')
                     }
                 );

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
                 }
             );

lms/templates/verify_student/incourse_reverify.html

@@ -43,7 +43,6 @@ from verify_student.views import PayAndVerifyView
     <div id="incourse-reverify-container"
     class="incourse-reverify"
     data-course-key='${course_key}'
-    data-checkpoint-name='${checkpoint_name}'
     data-platform-name='${platform_name}'
     data-usage-id='${usage_id}'
   ></div>

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