From 805c325653a5e9ce107daa9fcc4d4befd00e9d00 Mon Sep 17 00:00:00 2001 From: Julia Hansbrough Date: Fri, 8 Aug 2014 17:51:22 +0000 Subject: [PATCH 1/5] Update redirect and wording for prof ed - Changes registraiton button text for professional ed courses - Professional ed courses do not see "choose your track" page; go straight to verification flow --- .../course_modes/tests/test_views.py | 42 +++++++++++++++++++ common/djangoapps/course_modes/views.py | 18 +++++++- lms/djangoapps/courseware/views.py | 2 +- .../courseware/mktg_course_about.html | 10 ++++- 4 files changed, 68 insertions(+), 4 deletions(-) diff --git a/common/djangoapps/course_modes/tests/test_views.py b/common/djangoapps/course_modes/tests/test_views.py index 4df40ab2af..25dfad2a2b 100644 --- a/common/djangoapps/course_modes/tests/test_views.py +++ b/common/djangoapps/course_modes/tests/test_views.py @@ -98,3 +98,45 @@ class CourseModeViewTest(TestCase): self.assertEquals(response.status_code, 200) # TODO: Fix it so that response.templates works w/ mako templates, and then assert # that the right template rendered + + +class ProfessionalModeViewTest(TestCase): + """ + Tests for redirects specific to the 'professional' course mode. + Can't really put this in the ddt-style tests in CourseModeViewTest, + since 'professional' mode implies it is the *only* mode for a course + """ + def setUp(self): + self.course_id = SlashSeparatedCourseKey('org', 'course', 'run') + CourseModeFactory(mode_slug='professional', course_id=self.course_id) + self.user = UserFactory() + + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') + def test_professional_registration(self): + self.client.login( + username=self.user.username, + password='test' + ) + + response = self.client.get( + reverse('course_modes_choose', args=[self.course_id.to_deprecated_string()]), + follow=False, + ) + + self.assertEquals(response.status_code, 302) + self.assertTrue(response['Location'].endswith(reverse('verify_student_show_requirements', args=[unicode(self.course_id)]))) + + CourseEnrollmentFactory( + user=self.user, + is_active=True, + mode="professional", + course_id=unicode(self.course_id), + ) + + response = self.client.get( + reverse('course_modes_choose', args=[self.course_id.to_deprecated_string()]), + follow=False, + ) + + self.assertEquals(response.status_code, 302) + self.assertTrue(response['Location'].endswith(reverse('dashboard'))) diff --git a/common/djangoapps/course_modes/views.py b/common/djangoapps/course_modes/views.py index 83c12ab69f..a773c77f62 100644 --- a/common/djangoapps/course_modes/views.py +++ b/common/djangoapps/course_modes/views.py @@ -41,12 +41,26 @@ class ChooseModeView(View): request.session['attempting_upgrade'] = upgrade # Inactive users always need to re-register - # verified users do not need to register or upgrade + # verified and professional users do not need to register or upgrade # registered users who are not trying to upgrade do not need to re-register - if is_active and (upgrade is False or enrollment_mode == 'verified'): + if is_active and (upgrade is False or enrollment_mode == 'verified' or enrollment_mode == 'professional'): return redirect(reverse('dashboard')) modes = CourseMode.modes_for_course_dict(course_key) + + # We assume that, if 'professional' is one of the modes, it is the *only* mode. + # If we offer more modes alongside 'professional' in the future, this will need to route + # to the usual "choose your track" page. + if "professional" in modes: + return redirect( + reverse( + 'verify_student_show_requirements', + kwargs={'course_id': course_key.to_deprecated_string()} + ) + ) + + + donation_for_course = request.session.get("donation_for_course", {}) chosen_price = donation_for_course.get(course_key, None) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 8c23a0512b..baadac464c 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -708,7 +708,7 @@ def mktg_course_about(request, course_id): show_courseware_link = (has_access(request.user, 'load', course) or settings.FEATURES.get('ENABLE_LMS_MIGRATION')) - course_modes = CourseMode.modes_for_course(course.id) + course_modes = CourseMode.modes_for_course_dict(course.id) return render_to_response('courseware/mktg_course_about.html', { 'course': course, diff --git a/lms/templates/courseware/mktg_course_about.html b/lms/templates/courseware/mktg_course_about.html index e59ebb0f69..cb195b47f1 100644 --- a/lms/templates/courseware/mktg_course_about.html +++ b/lms/templates/courseware/mktg_course_about.html @@ -55,7 +55,15 @@ ${_("Register for")} ${course.display_number_with_default | h} %if len(course_modes) > 1: - and choose your student track + ## Translators: This is the second line on a button users can click. The first line is "Register for COURSE_NAME" + ## The "choose your student track" means users can select between taking the course as an auditor, as a verified student, etc + ${_("and choose your student track")} + + %elif "professional" in course_modes: + + ## Translators: This is the second line on a button users can click. The first line is "Register for COURSE_NAME" + ## 'Verification' here refers to verifying one's identity in order to receive a verified certificate. + ${_("and proceed to verification")} %endif From 9aa095dd2109deec934f3b7d3ff8b1168294a077 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 8 Aug 2014 17:12:47 -0400 Subject: [PATCH 2/5] Adapt flow for new 'professional' mode. ECOM-41 --- common/djangoapps/course_modes/models.py | 15 ++++ .../course_modes/tests/test_models.py | 14 ++++ common/djangoapps/course_modes/views.py | 7 -- lms/djangoapps/shoppingcart/models.py | 11 ++- .../shoppingcart/tests/test_reports.py | 2 +- lms/djangoapps/verify_student/views.py | 75 ++++++++++++------- .../verify_student/photo_verification.html | 32 ++++++-- lms/templates/verify_student/verified.html | 5 +- 8 files changed, 116 insertions(+), 45 deletions(-) diff --git a/common/djangoapps/course_modes/models.py b/common/djangoapps/course_modes/models.py index ccffd47568..2bd63ccf0d 100644 --- a/common/djangoapps/course_modes/models.py +++ b/common/djangoapps/course_modes/models.py @@ -112,6 +112,21 @@ class CourseMode(models.Model): else: return None + @classmethod + def verified_mode_for_course(cls, course_id): + """ + Since we have two separate modes that can go through the verify flow, + we want to be able to select the 'correct' verified mode for a given course. + + Currently, we prefer to return the professional mode over the verified one + if both exist for the given course. + """ + modes_dict = cls.modes_for_course_dict(course_id) + verified_mode = modes_dict.get('verified', None) + professional_mode = modes_dict.get('professional', None) + # we prefer professional over verify + return professional_mode if professional_mode else verified_mode + @classmethod def min_course_price_for_verified_for_currency(cls, course_id, currency): """ diff --git a/common/djangoapps/course_modes/tests/test_models.py b/common/djangoapps/course_modes/tests/test_models.py index 933faf8c66..c369aaaf0f 100644 --- a/common/djangoapps/course_modes/tests/test_models.py +++ b/common/djangoapps/course_modes/tests/test_models.py @@ -113,3 +113,17 @@ class CourseModeModelTest(TestCase): modes = CourseMode.modes_for_course(SlashSeparatedCourseKey('TestOrg', 'TestCourse', 'TestRun')) self.assertEqual([CourseMode.DEFAULT_MODE], modes) + + def test_verified_mode_for_course(self): + self.create_mode('verified', 'Verified Certificate') + + mode = CourseMode.verified_mode_for_course(self.course_key) + + self.assertEqual(mode.slug, 'verified') + + # verify that the professional mode is preferred + self.create_mode('professional', 'Professional Education Verified Certificate') + + mode = CourseMode.verified_mode_for_course(self.course_key) + + self.assertEqual(mode.slug, 'professional') diff --git a/common/djangoapps/course_modes/views.py b/common/djangoapps/course_modes/views.py index a773c77f62..0aa242b4b2 100644 --- a/common/djangoapps/course_modes/views.py +++ b/common/djangoapps/course_modes/views.py @@ -59,8 +59,6 @@ class ChooseModeView(View): ) ) - - donation_for_course = request.session.get("donation_for_course", {}) chosen_price = donation_for_course.get(course_key, None) @@ -135,11 +133,6 @@ class ChooseModeView(View): donation_for_course = request.session.get("donation_for_course", {}) donation_for_course[course_key] = amount_value request.session["donation_for_course"] = donation_for_course - if SoftwareSecurePhotoVerification.user_has_valid_or_pending(request.user): - return redirect( - reverse('verify_student_verified', - kwargs={'course_id': course_key.to_deprecated_string()}) + "?upgrade={}".format(upgrade) - ) return redirect( reverse('verify_student_show_requirements', diff --git a/lms/djangoapps/shoppingcart/models.py b/lms/djangoapps/shoppingcart/models.py index eda01f7c17..47d5d327f1 100644 --- a/lms/djangoapps/shoppingcart/models.py +++ b/lms/djangoapps/shoppingcart/models.py @@ -701,8 +701,13 @@ class CertificateItem(OrderItem): item.qty = 1 item.unit_cost = cost course_name = modulestore().get_course(course_id).display_name - item.line_desc = _("Certificate of Achievement, {mode_name} for course {course}").format(mode_name=mode_info.name, - course=course_name) + # Translators: In this particular case, mode_name refers to a + # particular mode (i.e. Honor Code Certificate, Verified Certificate, etc) + # by which a user could enroll in the given course. + item.line_desc = _("{mode_name} for course {course}").format( + mode_name=mode_info.name, + course=course_name + ) item.currency = currency order.currency = currency order.save() @@ -725,7 +730,7 @@ class CertificateItem(OrderItem): @property def single_item_receipt_template(self): - if self.mode == 'verified': + if self.mode in ('verified', 'professional'): return 'shoppingcart/verified_cert_receipt.html' else: return super(CertificateItem, self).single_item_receipt_template diff --git a/lms/djangoapps/shoppingcart/tests/test_reports.py b/lms/djangoapps/shoppingcart/tests/test_reports.py index 22ae61fb19..e3f69dee9c 100644 --- a/lms/djangoapps/shoppingcart/tests/test_reports.py +++ b/lms/djangoapps/shoppingcart/tests/test_reports.py @@ -222,7 +222,7 @@ class ItemizedPurchaseReportTest(ModuleStoreTestCase): self.CORRECT_CSV = dedent(""" Purchase Time,Order ID,Status,Quantity,Unit Cost,Total Cost,Currency,Description,Comments {time_str},1,purchased,1,40,40,usd,Registration for Course: Robot Super Course,Ba\xc3\xbc\xe5\x8c\x85 - {time_str},1,purchased,1,40,40,usd,"Certificate of Achievement, verified cert for course Robot Super Course", + {time_str},1,purchased,1,40,40,usd,verified cert for course Robot Super Course, """.format(time_str=str(self.now))) def test_purchased_items_btw_dates(self): diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index f87033441c..0400f7c269 100644 --- a/lms/djangoapps/verify_student/views.py +++ b/lms/djangoapps/verify_student/views.py @@ -74,18 +74,27 @@ class VerifyView(View): # bookkeeping-wise just to start over. progress_state = "start" - modes_dict = CourseMode.modes_for_course_dict(course_id) - verify_mode = modes_dict.get('verified', None) + # we prefer professional over verify + current_mode = CourseMode.verified_mode_for_course(course_id) + # if the course doesn't have a verified mode, we want to kick them # from the flow - if not verify_mode: + if not current_mode: return redirect(reverse('dashboard')) if course_id.to_deprecated_string() in request.session.get("donation_for_course", {}): chosen_price = request.session["donation_for_course"][course_id.to_deprecated_string()] else: - chosen_price = verify_mode.min_price + chosen_price = current_mode.min_price course = modulestore().get_course(course_id) + if current_mode.suggested_prices != '': + suggested_prices = [ + decimal.Decimal(price) + for price in current_mode.suggested_prices.split(",") + ] + else: + suggested_prices = [] + context = { "progress_state": progress_state, "user_full_name": request.user.profile.name, @@ -95,15 +104,13 @@ class VerifyView(View): "course_org": course.display_org_with_default, "course_num": course.display_number_with_default, "purchase_endpoint": get_purchase_endpoint(), - "suggested_prices": [ - decimal.Decimal(price) - for price in verify_mode.suggested_prices.split(",") - ], - "currency": verify_mode.currency.upper(), + "suggested_prices": suggested_prices, + "currency": current_mode.currency.upper(), "chosen_price": chosen_price, - "min_price": verify_mode.min_price, + "min_price": current_mode.min_price, "upgrade": upgrade == u'True', - "can_audit": "audit" in modes_dict, + "can_audit": CourseMode.mode_for_course(course_id, 'audit') is not None, + "modes_dict": CourseMode.modes_for_course_dict(course_id), } return render_to_response('verify_student/photo_verification.html', context) @@ -124,19 +131,20 @@ class VerifiedView(View): if CourseEnrollment.enrollment_mode_for_user(request.user, course_id) == ('verified', True): return redirect(reverse('dashboard')) + modes_dict = CourseMode.modes_for_course_dict(course_id) - verify_mode = modes_dict.get('verified', None) - if verify_mode is None: + # we prefer professional over verify + current_mode = CourseMode.verified_mode_for_course(course_id) + + # if the course doesn't have a verified mode, we want to kick them + # from the flow + if not current_mode: return redirect(reverse('dashboard')) - - chosen_price = request.session.get( - "donation_for_course", - {} - ).get( - course_id.to_deprecated_string(), - verify_mode.min_price - ) + if course_id.to_deprecated_string() in request.session.get("donation_for_course", {}): + chosen_price = request.session["donation_for_course"][course_id.to_deprecated_string()] + else: + chosen_price = current_mode.min_price course = modulestore().get_course(course_id) context = { @@ -146,11 +154,12 @@ class VerifiedView(View): "course_org": course.display_org_with_default, "course_num": course.display_number_with_default, "purchase_endpoint": get_purchase_endpoint(), - "currency": verify_mode.currency.upper(), + "currency": current_mode.currency.upper(), "chosen_price": chosen_price, "create_order_url": reverse("verify_student_create_order"), "upgrade": upgrade == u'True', "can_audit": "audit" in modes_dict, + "modes_dict": modes_dict, } return render_to_response('verify_student/verified.html', context) @@ -185,19 +194,24 @@ def create_order(request): donation_for_course[course_id] = amount request.session['donation_for_course'] = donation_for_course - verified_mode = CourseMode.modes_for_course_dict(course_id).get('verified', None) + # 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 verified_mode: + if not current_mode: return HttpResponseBadRequest(_("This course doesn't support verified certificates")) - if amount < verified_mode.min_price: + if amount < current_mode.min_price: return HttpResponseBadRequest(_("No selected price or selected price is below minimum.")) # I know, we should check this is valid. All kinds of stuff missing here cart = Order.get_cart_for_user(request.user) cart.clear() - CertificateItem.add_to_order(cart, course_id, amount, 'verified') + enrollment_mode = current_mode.slug + CertificateItem.add_to_order(cart, course_id, amount, enrollment_mode) params = get_signed_purchase_params(cart) @@ -288,12 +302,20 @@ def show_requirements(request, course_id): """ Show the requirements necessary for the verification flow. """ + # TODO: seems borked for professional; we're told we need to take photos even if there's a pending verification course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id) + upgrade = request.GET.get('upgrade', False) if CourseEnrollment.enrollment_mode_for_user(request.user, course_id) == ('verified', True): return redirect(reverse('dashboard')) + if SoftwareSecurePhotoVerification.user_has_valid_or_pending(request.user): + return redirect( + reverse('verify_student_verified', + kwargs={'course_id': course_id.to_deprecated_string()}) + "?upgrade={}".format(upgrade) + ) upgrade = request.GET.get('upgrade', False) course = modulestore().get_course(course_id) + modes_dict = CourseMode.modes_for_course_dict(course_id) context = { "course_id": course_id.to_deprecated_string(), "course_modes_choose_url": reverse("course_modes_choose", kwargs={'course_id': course_id.to_deprecated_string()}), @@ -303,6 +325,7 @@ def show_requirements(request, course_id): "course_num": course.display_number_with_default, "is_not_active": not request.user.is_active, "upgrade": upgrade == u'True', + "modes_dict": modes_dict, } return render_to_response("verify_student/show_requirements.html", context) diff --git a/lms/templates/verify_student/photo_verification.html b/lms/templates/verify_student/photo_verification.html index 17170d04ef..f9338944a8 100644 --- a/lms/templates/verify_student/photo_verification.html +++ b/lms/templates/verify_student/photo_verification.html @@ -177,12 +177,13 @@
${_("What do you do with this picture?")}
${_("We only use it to verify your identity. It is not displayed anywhere.")}
-
${_("What if my camera isn't working?")}
- - %if upgrade: -
${_("You can always continue to audit the course without verifying.")}
- %else: -
${_("You can always {a_start} audit the course for free {a_end} without verifying.").format(a_start=''.format(course_modes_choose_url), a_end="")}
+ %if "professional" not in modes_dict: +
${_("What if my camera isn't working?")}
+ %if upgrade: +
${_("You can always continue to audit the course without verifying.")}
+ %else: +
${_("You can always {a_start} audit the course for free {a_end} without verifying.").format(a_start=''.format(course_modes_choose_url), a_end="")}
+ %endif %endif @@ -366,6 +367,7 @@ + %if len(suggested_prices) > 0:

  • ${_("Check Your Contribution Level")}

    @@ -376,12 +378,28 @@ <%include file="/course_modes/_contribution.html" args="suggested_prices=suggested_prices, currency=currency, chosen_price=chosen_price, min_price=min_price"/>
    • + %else: +
  • +

    ${_("Your Course Total")}

    +
    +

    ${_("To complete your registration, you will need to pay:")}

    +
    +
      +
    • + $ + ${chosen_price} + ${currency} + +
      • +
    +
    • + %endif