From 06351ebd14a3af66aa31abbfe739bbcd3a409b6d Mon Sep 17 00:00:00 2001 From: Jesse Zoldak Date: Fri, 27 May 2016 15:44:01 -0400 Subject: [PATCH 01/10] Mark test as flaky TNL-4683 --- common/test/acceptance/tests/lms/test_learner_profile.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/common/test/acceptance/tests/lms/test_learner_profile.py b/common/test/acceptance/tests/lms/test_learner_profile.py index 3881c06478..4c07224edc 100644 --- a/common/test/acceptance/tests/lms/test_learner_profile.py +++ b/common/test/acceptance/tests/lms/test_learner_profile.py @@ -4,8 +4,9 @@ End-to-end tests for Student's Profile Page. """ from contextlib import contextmanager -from datetime import datetime from bok_choy.web_app_test import WebAppTest +from datetime import datetime +from flaky import flaky from nose.plugins.attrib import attr from ...pages.common.logout import LogoutPage @@ -296,6 +297,7 @@ class OwnLearnerProfilePageTest(LearnerProfileTestMixin, WebAppTest): self.verify_profile_page_is_private(profile_page) self.verify_profile_page_view_event(username, user_id, visibility=self.PRIVACY_PRIVATE) + @flaky # TODO fix this, see TNL-4683 def test_fields_on_my_public_profile(self): """ Scenario: Verify that desired fields are shown when looking at her own public profile. From 40386b311d296764ff306fcfcdd47d849988f08e Mon Sep 17 00:00:00 2001 From: Douglas Hall Date: Thu, 2 Jun 2016 17:00:03 -0400 Subject: [PATCH 02/10] Fix student profile data download for courses with extended profile fields --- lms/djangoapps/instructor_task/api.py | 2 +- lms/djangoapps/instructor_task/tasks_helper.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/instructor_task/api.py b/lms/djangoapps/instructor_task/api.py index 6591202d63..26352f0bf4 100644 --- a/lms/djangoapps/instructor_task/api.py +++ b/lms/djangoapps/instructor_task/api.py @@ -337,7 +337,7 @@ def submit_calculate_students_features_csv(request, course_key, features): """ task_type = 'profile_info_csv' task_class = calculate_students_features_csv - task_input = {'features': features} + task_input = features task_key = "" return submit_task(request, task_type, task_class, course_key, task_input, task_key) diff --git a/lms/djangoapps/instructor_task/tasks_helper.py b/lms/djangoapps/instructor_task/tasks_helper.py index c276d6de90..43f97ad9b9 100644 --- a/lms/djangoapps/instructor_task/tasks_helper.py +++ b/lms/djangoapps/instructor_task/tasks_helper.py @@ -996,7 +996,7 @@ def upload_students_csv(_xmodule_instance_args, _entry_id, course_id, task_input task_progress.update_task_state(extra_meta=current_step) # compute the student features table and format it - query_features = task_input.get('features') + query_features = task_input student_data = enrolled_students_features(course_id, query_features) header, rows = format_dictlist(student_data, query_features) From 745e48159719fb50a159e64e6275dea40c345cb7 Mon Sep 17 00:00:00 2001 From: Syed Hassan Raza Date: Wed, 1 Jun 2016 15:51:38 +0500 Subject: [PATCH 03/10] Fixed logging and unicode handling --- .../lib/xmodule/xmodule/tests/test_video.py | 8 ++++++++ .../xmodule/video_module/video_module.py | 20 ++++++++++--------- .../xmodule/video_module/video_utils.py | 13 ++++++++++++ 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_video.py b/common/lib/xmodule/xmodule/tests/test_video.py index 9b5b49eaba..e11147a43b 100644 --- a/common/lib/xmodule/xmodule/tests/test_video.py +++ b/common/lib/xmodule/xmodule/tests/test_video.py @@ -758,6 +758,14 @@ class VideoExportTestCase(VideoDescriptorTestBase): with self.assertRaises(ValueError): self.descriptor.definition_to_xml(None) + def test_export_to_xml_unicode_characters(self): + """ + Test XML export handles the unicode characters. + """ + self.descriptor.display_name = '这是文' + xml = self.descriptor.definition_to_xml(None) + self.assertEqual(xml.get('display_name'), u'\u8fd9\u662f\u6587') + class VideoDescriptorIndexingTestCase(unittest.TestCase): """ diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py index 59a54ff785..31f9ecc4ad 100644 --- a/common/lib/xmodule/xmodule/video_module/video_module.py +++ b/common/lib/xmodule/xmodule/video_module/video_module.py @@ -38,7 +38,7 @@ from xmodule.exceptions import NotFoundError from xmodule.contentstore.content import StaticContent from .transcripts_utils import VideoTranscriptsMixin, Transcript, get_html5_ids -from .video_utils import create_youtube_string, get_poster, rewrite_video_url +from .video_utils import create_youtube_string, get_poster, rewrite_video_url, format_xml_exception_message from .bumper_utils import bumperize from .video_xfields import VideoFields from .video_handlers import VideoStudentViewHandlers, VideoStudioViewHandlers @@ -563,14 +563,16 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler if key in self.fields and self.fields[key].is_set_on(self): try: xml.set(key, unicode(value)) - except ValueError as exception: - exception_message = "{message}, Block-location:{location}, Key:{key}, Value:{value}".format( - message=exception.message, - location=unicode(self.location), - key=key, - value=unicode(value) - ) - raise ValueError(exception_message) + except UnicodeDecodeError: + exception_message = format_xml_exception_message(self.location, key, value) + log.exception(exception_message) + # If exception is UnicodeDecodeError set value using unicode 'utf-8' scheme. + log.info("Setting xml value using 'utf-8' scheme.") + xml.set(key, unicode(value, 'utf-8')) + except ValueError: + exception_message = format_xml_exception_message(self.location, key, value) + log.exception(exception_message) + raise for source in self.html5_sources: ele = etree.Element('source') diff --git a/common/lib/xmodule/xmodule/video_module/video_utils.py b/common/lib/xmodule/xmodule/video_module/video_utils.py index 0749b54f2d..1c24303ada 100644 --- a/common/lib/xmodule/xmodule/video_module/video_utils.py +++ b/common/lib/xmodule/xmodule/video_module/video_utils.py @@ -98,6 +98,19 @@ def get_poster(video): return poster +def format_xml_exception_message(location, key, value): + """ + Generate exception message for VideoDescriptor class which will use for ValueError and UnicodeDecodeError + when setting xml attributes. + """ + exception_message = "Block-location:{location}, Key:{key}, Value:{value}".format( + location=unicode(location), + key=key, + value=value + ) + return exception_message + + def set_query_parameter(url, param_name, param_value): """ Given a URL, set or replace a query parameter and return the From 9c4a3dc9c7ec624dd76c1a01df7c6a61ef345b8f Mon Sep 17 00:00:00 2001 From: Renzo Lucioni Date: Thu, 2 Jun 2016 16:23:44 -0400 Subject: [PATCH 04/10] Fix program details a11y test --- common/test/acceptance/tests/lms/test_programs.py | 3 --- lms/static/js/spec/learner_dashboard/course_card_view_spec.js | 4 ++-- lms/templates/learner_dashboard/course_card.underscore | 4 ++-- .../learner_dashboard/program_header_view.underscore | 2 +- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/common/test/acceptance/tests/lms/test_programs.py b/common/test/acceptance/tests/lms/test_programs.py index 5a29d1b9b9..4695b583fc 100644 --- a/common/test/acceptance/tests/lms/test_programs.py +++ b/common/test/acceptance/tests/lms/test_programs.py @@ -1,6 +1,4 @@ """Acceptance tests for LMS-hosted Programs pages""" -from unittest import skip - from nose.plugins.attrib import attr from ...fixtures.programs import ProgramsFixture, ProgramsConfigMixin @@ -139,7 +137,6 @@ class ProgramListingPageA11yTest(ProgramPageBase): @attr('a11y') -@skip('The tested page is currently disabled. This test will be re-enabled once a11y failures are resolved.') class ProgramDetailsPageA11yTest(ProgramPageBase): """Test program details page accessibility.""" def setUp(self): diff --git a/lms/static/js/spec/learner_dashboard/course_card_view_spec.js b/lms/static/js/spec/learner_dashboard/course_card_view_spec.js index 5fa267b361..b7de0bc487 100644 --- a/lms/static/js/spec/learner_dashboard/course_card_view_spec.js +++ b/lms/static/js/spec/learner_dashboard/course_card_view_spec.js @@ -61,7 +61,7 @@ define([ expect(view.$('.header-img').attr('src')).toEqual(context.run_modes[0].course_image_url); expect(view.$('.course-details .course-title-link').text().trim()).toEqual(context.display_name); expect(view.$('.course-details .course-title-link').attr('href')).toEqual( - context.run_modes[0].marketing_url); + context.run_modes[0].course_url); expect(view.$('.course-details .course-text .course-key').html()).toEqual(context.key); expect(view.$('.course-details .course-text .run-period').html()) .toEqual(context.run_modes[0].start_date + ' - ' + context.run_modes[0].end_date); @@ -71,7 +71,7 @@ define([ expect(view.$('.header-img').attr('src')).toEqual(context.run_modes[0].course_image_url); expect(view.$('.course-details .course-title-link').text().trim()).toEqual(context.display_name); expect(view.$('.course-details .course-title-link').attr('href')).toEqual( - context.run_modes[0].marketing_url); + context.run_modes[0].course_url); expect(view.$('.course-details .course-text .course-key').html()).toEqual(context.key); expect(view.$('.course-details .course-text .run-period').html()).not.toBeDefined(); }); diff --git a/lms/templates/learner_dashboard/course_card.underscore b/lms/templates/learner_dashboard/course_card.underscore index 1f4a923d15..01ba2dc4f4 100644 --- a/lms/templates/learner_dashboard/course_card.underscore +++ b/lms/templates/learner_dashboard/course_card.underscore @@ -1,6 +1,6 @@
- +

- + <%- display_name %>

diff --git a/lms/templates/learner_dashboard/program_header_view.underscore b/lms/templates/learner_dashboard/program_header_view.underscore index a6114b1275..aa9ac727ff 100644 --- a/lms/templates/learner_dashboard/program_header_view.underscore +++ b/lms/templates/learner_dashboard/program_header_view.underscore @@ -5,7 +5,7 @@

<%- name %>

<%- subtitle %>

-<%- gettext('Programs') %> +<%- gettext('Programs') %> <%- StringUtils.interpolate( gettext('{category}\'s program'), {category: category} From c2c2a67753084215e8641fa3bc5b75ce093f09aa Mon Sep 17 00:00:00 2001 From: Matt Drayer Date: Thu, 2 Jun 2016 19:34:21 -0400 Subject: [PATCH 05/10] mattdrayer/WL-493: Enable bulk purchase via Otto for unauthenticated users --- common/djangoapps/course_modes/tests/test_views.py | 6 ++++-- common/djangoapps/course_modes/views.py | 9 ++++++--- common/djangoapps/student/helpers.py | 2 +- lms/djangoapps/verify_student/tests/test_integration.py | 4 ++-- lms/djangoapps/verify_student/views.py | 6 +++++- lms/static/js/student_account/views/FinishAuthView.js | 4 +++- 6 files changed, 21 insertions(+), 10 deletions(-) diff --git a/common/djangoapps/course_modes/tests/test_views.py b/common/djangoapps/course_modes/tests/test_views.py index 0aa9200217..b8c5161ddf 100644 --- a/common/djangoapps/course_modes/tests/test_views.py +++ b/common/djangoapps/course_modes/tests/test_views.py @@ -94,7 +94,8 @@ class CourseModeViewTest(UrlResetMixin, ModuleStoreTestCase): url = reverse('course_modes_choose', args=[unicode(self.course.id)]) response = self.client.get(url) # Check whether we were correctly redirected - start_flow_url = reverse('verify_student_start_flow', args=[unicode(self.course.id)]) + purchase_workflow = "?purchase_workflow=single" + start_flow_url = reverse('verify_student_start_flow', args=[unicode(self.course.id)]) + purchase_workflow self.assertRedirects(response, start_flow_url) def test_no_id_redirect_otto(self): @@ -194,7 +195,8 @@ class CourseModeViewTest(UrlResetMixin, ModuleStoreTestCase): # Since the only available track is professional ed, expect that # we're redirected immediately to the start of the payment flow. - start_flow_url = reverse('verify_student_start_flow', args=[unicode(self.course.id)]) + purchase_workflow = "?purchase_workflow=single" + start_flow_url = reverse('verify_student_start_flow', args=[unicode(self.course.id)]) + purchase_workflow self.assertRedirects(response, start_flow_url) # Now enroll in the course diff --git a/common/djangoapps/course_modes/views.py b/common/djangoapps/course_modes/views.py index 2bf337e1b2..57695c9da2 100644 --- a/common/djangoapps/course_modes/views.py +++ b/common/djangoapps/course_modes/views.py @@ -88,12 +88,15 @@ class ChooseModeView(View): # If there are both modes, default to non-id-professional. has_enrolled_professional = (CourseMode.is_professional_slug(enrollment_mode) and is_active) if CourseMode.has_professional_mode(modes) and not has_enrolled_professional: - redirect_url = reverse('verify_student_start_flow', kwargs={'course_id': unicode(course_key)}) + purchase_workflow = request.GET.get("purchase_workflow", "single") + verify_url = reverse('verify_student_start_flow', kwargs={'course_id': unicode(course_key)}) + redirect_url = "{url}?purchase_workflow={workflow}".format(url=verify_url, workflow=purchase_workflow) if ecommerce_service.is_enabled(request.user): professional_mode = modes.get(CourseMode.NO_ID_PROFESSIONAL_MODE) or modes.get(CourseMode.PROFESSIONAL) - if professional_mode.sku: + if purchase_workflow == "single" and professional_mode.sku: redirect_url = ecommerce_service.checkout_page_url(professional_mode.sku) - + if purchase_workflow == "bulk" and professional_mode.bulk_sku: + redirect_url = ecommerce_service.checkout_page_url(professional_mode.bulk_sku) return redirect(redirect_url) # If there isn't a verified mode available, then there's nothing diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index a8c4e500ff..8ea71514a9 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -200,7 +200,7 @@ def auth_pipeline_urls(auth_entry, redirect_url=None): # Query string parameters that can be passed to the "finish_auth" view to manage # things like auto-enrollment. -POST_AUTH_PARAMS = ('course_id', 'enrollment_action', 'course_mode', 'email_opt_in') +POST_AUTH_PARAMS = ('course_id', 'enrollment_action', 'course_mode', 'email_opt_in', 'purchase_workflow') def get_next_url_for_login_page(request): diff --git a/lms/djangoapps/verify_student/tests/test_integration.py b/lms/djangoapps/verify_student/tests/test_integration.py index c2401b6231..c45c9cd350 100644 --- a/lms/djangoapps/verify_student/tests/test_integration.py +++ b/lms/djangoapps/verify_student/tests/test_integration.py @@ -32,7 +32,7 @@ class TestProfEdVerification(ModuleStoreTestCase): min_price=self.MIN_PRICE, suggested_prices='' ) - + purchase_workflow = "?purchase_workflow=single" self.urls = { 'course_modes_choose': reverse( 'course_modes_choose', @@ -42,7 +42,7 @@ class TestProfEdVerification(ModuleStoreTestCase): 'verify_student_start_flow': reverse( 'verify_student_start_flow', args=[unicode(self.course_key)] - ), + ) + purchase_workflow, } def test_start_flow(self): diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index 3484d9af6e..21715ba979 100644 --- a/lms/djangoapps/verify_student/views.py +++ b/lms/djangoapps/verify_student/views.py @@ -334,6 +334,10 @@ class PayAndVerifyView(View): # Redirect the user to a more appropriate page if the # messaging won't make sense based on the user's # enrollment / payment / verification status. + sku_to_use = relevant_course_mode.sku + purchase_workflow = request.GET.get('purchase_workflow', 'single') + if purchase_workflow == 'bulk' and relevant_course_mode.bulk_sku: + sku_to_use = relevant_course_mode.bulk_sku redirect_response = self._redirect_if_necessary( message, already_verified, @@ -342,7 +346,7 @@ class PayAndVerifyView(View): course_key, user_is_trying_to_pay, request.user, - relevant_course_mode.sku + sku_to_use ) if redirect_response is not None: return redirect_response diff --git a/lms/static/js/student_account/views/FinishAuthView.js b/lms/static/js/student_account/views/FinishAuthView.js index 8ec1da6ab7..8e5c3955da 100644 --- a/lms/static/js/student_account/views/FinishAuthView.js +++ b/lms/static/js/student_account/views/FinishAuthView.js @@ -51,7 +51,8 @@ enrollmentAction: $.url( '?enrollment_action' ), courseId: $.url( '?course_id' ), courseMode: $.url( '?course_mode' ), - emailOptIn: $.url( '?email_opt_in' ) + emailOptIn: $.url( '?email_opt_in' ), + purchaseWorkflow: $.url( '?purchase_workflow' ) }; for (var key in queryParams) { if (queryParams[key]) { @@ -63,6 +64,7 @@ this.courseMode = queryParams.courseMode; this.emailOptIn = queryParams.emailOptIn; this.nextUrl = this.urls.defaultNextUrl; + this.purchaseWorkflow = queryParams.purchaseWorkflow; if (queryParams.next) { // Ensure that the next URL is internal for security reasons if ( ! window.isExternal( queryParams.next ) ) { From fbdce967258e209e72fa8c22022ab7347c3ccd8e Mon Sep 17 00:00:00 2001 From: Chris Rodriguez Date: Fri, 3 Jun 2016 14:26:17 -0400 Subject: [PATCH 06/10] Uses previous commit hash to prevent failures on master --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 59a4da30b6..dc3d117cda 100644 --- a/package.json +++ b/package.json @@ -16,7 +16,7 @@ "underscore.string": "~3.3.4" }, "devDependencies": { - "edx-custom-a11y-rules": "edx/edx-custom-a11y-rules", + "edx-custom-a11y-rules": "git+https://github.com/edx/edx-custom-a11y-rules.git#12d2cae4ffdbb45c5643819211e06c17d6200210", "pa11y": "3.6.0", "pa11y-reporter-1.0-json": "1.0.2", "jasmine-core": "^2.4.1", From ea654200f58936ed9467070a948ed6464940ed76 Mon Sep 17 00:00:00 2001 From: Douglas Hall Date: Thu, 2 Jun 2016 14:15:26 -0400 Subject: [PATCH 07/10] Put shopping cart bulk purchase behind a settings flag --- lms/djangoapps/instructor/views/api.py | 11 +++++++++-- lms/djangoapps/shoppingcart/views.py | 1 + lms/templates/shoppingcart/shopping_cart.html | 4 +++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 26014efc12..ca68e24190 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -1240,8 +1240,15 @@ def get_students_features(request, course_id, csv=False): # pylint: disable=red available_features = instructor_analytics.basic.AVAILABLE_FEATURES - # Allow for microsites to be able to define additional columns (e.g. ) - query_features = microsite.get_value('student_profile_download_fields') + # Allow for microsites to be able to define additional columns. + # Note that adding additional columns has the potential to break + # the student profile report due to a character limit on the + # asynchronous job input which in this case is a JSON string + # containing the list of columns to include in the report. + # TODO: Refactor the student profile report code to remove the list of columns + # that should be included in the report from the asynchronous job input. + # We need to clone the list because we modify it below + query_features = list(microsite.get_value('student_profile_download_fields', [])) if not query_features: query_features = [ diff --git a/lms/djangoapps/shoppingcart/views.py b/lms/djangoapps/shoppingcart/views.py index f9d7f7f6fa..771b1551c9 100644 --- a/lms/djangoapps/shoppingcart/views.py +++ b/lms/djangoapps/shoppingcart/views.py @@ -190,6 +190,7 @@ def show_cart(request): 'form_html': form_html, 'currency_symbol': settings.PAID_COURSE_REGISTRATION_CURRENCY[1], 'currency': settings.PAID_COURSE_REGISTRATION_CURRENCY[0], + 'enable_bulk_purchase': microsite.get_value('ENABLE_SHOPPING_CART_BULK_PURCHASE', True) } return render_to_response("shoppingcart/shopping_cart.html", context) diff --git a/lms/templates/shoppingcart/shopping_cart.html b/lms/templates/shoppingcart/shopping_cart.html index 4f29e9017f..a39cfbb4c7 100644 --- a/lms/templates/shoppingcart/shopping_cart.html +++ b/lms/templates/shoppingcart/shopping_cart.html @@ -101,6 +101,7 @@ from openedx.core.lib.courses import course_image_url % endif
+ % if enable_bulk_purchase:
@@ -116,8 +117,9 @@ from openedx.core.lib.courses import course_image_url -
+ % endif +