From d11aaec3a1993e6017f1df7735dcf83411a1b453 Mon Sep 17 00:00:00 2001 From: Waqas Khalid Date: Mon, 18 Aug 2014 20:39:51 +0500 Subject: [PATCH] Handle the indexerror exception on create_order LMS-2646 --- .../verify_student/tests/test_views.py | 92 +++++++++++++++++++ lms/djangoapps/verify_student/views.py | 25 +++-- lms/static/js/spec/photocapture_spec.js | 40 ++++++++ lms/static/js/verify_student/photocapture.js | 57 ++++++++---- .../verify_student/photo_verification.html | 21 ++++- 5 files changed, 205 insertions(+), 30 deletions(-) create mode 100644 lms/static/js/spec/photocapture_spec.js diff --git a/lms/djangoapps/verify_student/tests/test_views.py b/lms/djangoapps/verify_student/tests/test_views.py index d5a7c9233e..29d1d1d808 100644 --- a/lms/djangoapps/verify_student/tests/test_views.py +++ b/lms/djangoapps/verify_student/tests/test_views.py @@ -31,6 +31,7 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey from student.tests.factories import UserFactory from student.models import CourseEnrollment from course_modes.tests.factories import CourseModeFactory +from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE from course_modes.models import CourseMode from verify_student.views import render_to_response from verify_student.models import SoftwareSecurePhotoVerification @@ -65,6 +66,97 @@ class StartView(TestCase): self.assertHttpForbidden(self.client.get(self.start_url())) +@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) +class TestCreateOrderView(TestCase): + """ + Tests for the create_order view of verified course registration process + """ + def setUp(self): + self.user = UserFactory.create(username="rusty", password="test") + self.client.login(username="rusty", password="test") + self.course_id = 'Robot/999/Test_Course' + CourseFactory.create(org='Robot', number='999', display_name='Test Course') + verified_mode = CourseMode( + course_id=SlashSeparatedCourseKey("Robot", "999", 'Test_Course'), + mode_slug="verified", + mode_display_name="Verified Certificate", + min_price=50 + ) + verified_mode.save() + course_mode_post_data = { + 'certificate_mode': 'Select Certificate', + 'contribution': 50, + 'contribution-other-amt': '', + 'explain': '' + } + self.client.post( + reverse("course_modes_choose", kwargs={'course_id': self.course_id}), + course_mode_post_data + ) + + def test_invalid_photos_data(self): + """ + Test that the invalid photo data cannot be submitted + """ + create_order_post_data = { + 'contribution': 50, + 'course_id': self.course_id, + 'face_image': '', + 'photo_id_image': '' + } + response = self.client.post(reverse('verify_student_create_order'), create_order_post_data) + json_response = json.loads(response.content) + self.assertFalse(json_response.get('success')) + + @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': True}) + def test_invalid_amount(self): + """ + Test that the user cannot give invalid amount + """ + create_order_post_data = { + 'contribution': '1.a', + 'course_id': self.course_id, + 'face_image': ',', + 'photo_id_image': ',' + } + response = self.client.post(reverse('verify_student_create_order'), create_order_post_data) + self.assertEquals(response.status_code, 400) + self.assertIn('Selected price is not valid number.', response.content) + + @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': True}) + def test_invalid_mode(self): + """ + Test that the course without verified mode cannot be processed + """ + course_id = 'Fake/999/Test_Course' + CourseFactory.create(org='Fake', number='999', display_name='Test Course') + create_order_post_data = { + 'contribution': '50', + 'course_id': course_id, + 'face_image': ',', + 'photo_id_image': ',' + } + response = self.client.post(reverse('verify_student_create_order'), create_order_post_data) + self.assertEquals(response.status_code, 400) + self.assertIn('This course doesn\'t support verified certificates', response.content) + + @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': True}) + def test_create_order_success(self): + """ + Test that the order is created successfully when given valid data + """ + create_order_post_data = { + 'contribution': 50, + 'course_id': self.course_id, + 'face_image': ',', + 'photo_id_image': ',' + } + response = self.client.post(reverse('verify_student_create_order'), create_order_post_data) + json_response = json.loads(response.content) + self.assertTrue(json_response.get('success')) + self.assertIsNotNone(json_response.get('orderNumber')) + + @override_settings(MODULESTORE=MODULESTORE_CONFIG) class TestVerifyView(ModuleStoreTestCase): def setUp(self): diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index 6377d8f271..862edc2fdb 100644 --- a/lms/djangoapps/verify_student/views.py +++ b/lms/djangoapps/verify_student/views.py @@ -38,6 +38,8 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey from .exceptions import WindowExpiredException from xmodule.modulestore.django import modulestore +from util.json_request import JsonResponse + log = logging.getLogger(__name__) EVENT_NAME_USER_ENTERED_MIDCOURSE_REVERIFY_VIEW = 'edx.course.enrollment.reverify.started' @@ -114,6 +116,7 @@ class VerifyView(View): # TODO (ECOM-16): Remove once the AB test completes "autoreg": request.session.get('auto_register', False), + "retake": request.GET.get('retake', False), } return render_to_response('verify_student/photo_verification.html', context) @@ -177,9 +180,14 @@ def create_order(request): """ if not SoftwareSecurePhotoVerification.user_has_valid_or_pending(request.user): attempt = SoftwareSecurePhotoVerification(user=request.user) - b64_face_image = request.POST['face_image'].split(",")[1] - b64_photo_id_image = request.POST['photo_id_image'].split(",")[1] - + try: + b64_face_image = request.POST['face_image'].split(",")[1] + b64_photo_id_image = request.POST['photo_id_image'].split(",")[1] + except IndexError: + context = { + 'success': False, + } + return JsonResponse(context) attempt.upload_face_image(b64_face_image.decode('base64')) attempt.upload_photo_id_image(b64_photo_id_image.decode('base64')) attempt.mark_ready() @@ -203,13 +211,13 @@ def create_order(request): # prefer professional mode over verified_mode current_mode = CourseMode.verified_mode_for_course(course_id) - if current_mode.slug == 'professional': - amount = current_mode.min_price - # make sure this course has a verified mode if not current_mode: return HttpResponseBadRequest(_("This course doesn't support verified certificates")) + if current_mode.slug == 'professional': + amount = current_mode.min_price + if amount < current_mode.min_price: return HttpResponseBadRequest(_("No selected price or selected price is below minimum.")) @@ -225,7 +233,7 @@ def create_order(request): params = get_signed_purchase_params( cart, callback_url=callback_url ) - + params['success'] = True return HttpResponse(json.dumps(params), content_type="text/json") @@ -484,8 +492,7 @@ def midcourse_reverify_dash(request): try: course_enrollment_pairs.append((modulestore().get_course(enrollment.course_id), enrollment)) except ItemNotFoundError: - log.error("User {0} enrolled in non-existent course {1}" - .format(user.username, enrollment.course_id)) + log.error("User {0} enrolled in non-existent course {1}".format(user.username, enrollment.course_id)) statuses = ["approved", "pending", "must_reverify", "denied"] diff --git a/lms/static/js/spec/photocapture_spec.js b/lms/static/js/spec/photocapture_spec.js new file mode 100644 index 0000000000..844963a4be --- /dev/null +++ b/lms/static/js/spec/photocapture_spec.js @@ -0,0 +1,40 @@ +describe("Photo Verification", function() { + + beforeEach(function() { + setFixtures(''); + }); + + it('retake photo', function() { + spyOn(window,"refereshPageMessage").andCallFake(function(){ + return + }) + spyOn($, "ajax").andCallFake(function(e) { + e.success({"success":false}); + }); + submitToPaymentProcessing(); + expect(window.refereshPageMessage).toHaveBeenCalled(); + }); + + it('successful submission', function() { + spyOn(window,"submitForm").andCallFake(function(){ + return + }) + spyOn($, "ajax").andCallFake(function(e) { + e.success({"success":true}); + }); + submitToPaymentProcessing(); + expect(window.submitForm).toHaveBeenCalled(); + }); + + it('Error during process', function() { + spyOn(window,"showSubmissionError").andCallFake(function(){ + return + }) + spyOn($, "ajax").andCallFake(function(e) { + e.error({}); + }); + submitToPaymentProcessing(); + expect(window.showSubmissionError).toHaveBeenCalled(); + }); + +}); diff --git a/lms/static/js/verify_student/photocapture.js b/lms/static/js/verify_student/photocapture.js index 7fa10fdc33..dc31f86386 100644 --- a/lms/static/js/verify_student/photocapture.js +++ b/lms/static/js/verify_student/photocapture.js @@ -44,6 +44,30 @@ var submitMidcourseReverificationPhotos = function() { $("#reverify_form").submit(); } +function showSubmissionError() { + if (xhr.status == 400) { + $('#order-error .copy p').html(xhr.responseText); + } + $('#order-error').show(); + $("html, body").animate({ scrollTop: 0 }); +} + +function submitForm(data) { + for (prop in data) { + $('').attr({ + type: 'hidden', + name: prop, + value: data[prop] + }).appendTo('#pay_form'); + } + $("#pay_form").submit(); +} + +function refereshPageMessage() { + $('#photo-error').show(); + $("html, body").animate({ scrollTop: 0 }); +} + var submitToPaymentProcessing = function() { var contribution_input = $("input[name='contribution']:checked") var contribution = 0; @@ -54,33 +78,25 @@ var submitToPaymentProcessing = function() { contribution = contribution_input.val(); } var course_id = $("input[name='course_id']").val(); - var xhr = $.post( - "/verify_student/create_order", - { + $.ajax({ + url: "/verify_student/create_order", + type: 'POST', + data: { "course_id" : course_id, "contribution": contribution, "face_image" : $("#face_image")[0].src, "photo_id_image" : $("#photo_id_image")[0].src }, - function(data) { - for (prop in data) { - $('').attr({ - type: 'hidden', - name: prop, - value: data[prop] - }).appendTo('#pay_form'); + success:function(data) { + if (data.success) { + submitForm(data); + } else { + refereshPageMessage(); } + }, + error:function(xhr,status,error) { + showSubmissionError() } - ) - .done(function(data) { - $("#pay_form").submit(); - }) - .fail(function(jqXhr,text_status, error_thrown) { - if(jqXhr.status == 400) { - $('#order-error .copy p').html(jqXhr.responseText); - } - $('#order-error').show(); - $("html, body").animate({ scrollTop: 0 }); }); } @@ -298,6 +314,7 @@ $(document).ready(function() { // when moving between steps $('#face_next_link').click(function(){ analytics.pageview("Capture ID Photo"); + $('#photo-error').hide(); $('body').addClass('step-photos-id').removeClass('step-photos-cam') }) diff --git a/lms/templates/verify_student/photo_verification.html b/lms/templates/verify_student/photo_verification.html index f9338944a8..09d167506a 100644 --- a/lms/templates/verify_student/photo_verification.html +++ b/lms/templates/verify_student/photo_verification.html @@ -1,6 +1,6 @@ <%! from django.utils.translation import ugettext as _ %> <%! from django.core.urlresolvers import reverse %> - +<%! from lms.envs.common import TECH_SUPPORT_EMAIL %> <%inherit file="../main.html" /> <%namespace name='static' file='/static_content.html'/> @@ -57,6 +57,25 @@ + +