From b27401554cb773a00a9ccdc023d820ffbd34094d Mon Sep 17 00:00:00 2001 From: Dennis Jen Date: Tue, 4 Nov 2014 10:58:13 -0500 Subject: [PATCH 01/48] Feature flagged enrollment counts on instructor dashboard. * enrollments counts removed from legacy dashboard --- .../instructor/views/instructor_dashboard.py | 41 ++++++++++----- lms/djangoapps/instructor/views/legacy.py | 7 ++- lms/envs/common.py | 5 +- .../courseware/instructor_dashboard.html | 16 ------ .../instructor_dashboard_2/course_info.html | 51 +++++++++++-------- 5 files changed, 64 insertions(+), 56 deletions(-) diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 315b78413b..f894304954 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -91,11 +91,7 @@ def instructor_dashboard_2(request, course_id): if course_mode_has_price: sections.append(_section_e_commerce(course, access)) - enrollment_count = sections[0]['enrollment_count']['total'] - disable_buttons = False - max_enrollment_for_buttons = settings.FEATURES.get("MAX_ENROLLMENT_INSTR_BUTTONS") - if max_enrollment_for_buttons is not None: - disable_buttons = enrollment_count > max_enrollment_for_buttons + disable_buttons = not _is_small_course(course_key) analytics_dashboard_message = None if settings.ANALYTICS_DASHBOARD_URL: @@ -217,12 +213,19 @@ def _section_course_info(course, access): 'access': access, 'course_id': course_key, 'course_display_name': course.display_name, - 'enrollment_count': CourseEnrollment.enrollment_counts(course_key), 'has_started': course.has_started(), 'has_ended': course.has_ended(), 'list_instructor_tasks_url': reverse('list_instructor_tasks', kwargs={'course_id': course_key.to_deprecated_string()}), } + if settings.FEATURES.get('DISPLAY_ANALYTICS_ENROLLMENTS'): + section_data['enrollment_count'] = CourseEnrollment.enrollment_counts(course_key) + + if settings.ANALYTICS_DASHBOARD_URL: + dashboard_link = _get_dashboard_link(course_key) + message = _("Enrollment data is now available in {dashboard_link}.").format(dashboard_link=dashboard_link) + section_data['enrollment_message'] = message + try: advance = lambda memo, (letter, score): "{}: {}, ".format(letter, score) + memo section_data['grade_cutoffs'] = reduce(advance, course.grade_cutoffs.items(), "")[:-2] @@ -259,14 +262,19 @@ def _section_membership(course, access): return section_data -def _section_student_admin(course, access): - """ Provide data for the corresponding dashboard section """ - course_key = course.id +def _is_small_course(course_key): is_small_course = False enrollment_count = CourseEnrollment.num_enrolled_in(course_key) max_enrollment_for_buttons = settings.FEATURES.get("MAX_ENROLLMENT_INSTR_BUTTONS") if max_enrollment_for_buttons is not None: is_small_course = enrollment_count <= max_enrollment_for_buttons + return is_small_course + + +def _section_student_admin(course, access): + """ Provide data for the corresponding dashboard section """ + course_key = course.id + is_small_course =_is_small_course(course_key) section_data = { 'section_key': 'student_admin', @@ -354,6 +362,16 @@ def _section_send_email(course, access): return section_data +def _get_dashboard_link(course_key): + # Construct a URL to the external analytics dashboard + link = None + if settings.ANALYTICS_DASHBOARD_URL: + analytics_dashboard_url = '{0}/courses/{1}'.format(settings.ANALYTICS_DASHBOARD_URL, unicode(course_key)) + link = "{1}".format(analytics_dashboard_url, + settings.ANALYTICS_DASHBOARD_NAME) + return link + + def _section_analytics(course, access): """ Provide data for the corresponding dashboard section """ course_key = course.id @@ -366,10 +384,7 @@ def _section_analytics(course, access): } if settings.ANALYTICS_DASHBOARD_URL: - # Construct a URL to the external analytics dashboard - analytics_dashboard_url = '{0}/courses/{1}'.format(settings.ANALYTICS_DASHBOARD_URL, unicode(course_key)) - dashboard_link = "{1}".format(analytics_dashboard_url, - settings.ANALYTICS_DASHBOARD_NAME) + dashboard_link = _get_dashboard_link(course_key) message = _("Demographic data is now available in {dashboard_link}.").format(dashboard_link=dashboard_link) section_data['demographic_message'] = message diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index 728b04777a..8bc58c742d 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -134,8 +134,8 @@ def instructor_dashboard(request, course_id): 'header': ['Statistic', 'Value'], 'title': _('Course Statistics At A Glance'), } - data = [['# Enrolled', enrollment_number]] - data += [['Date', timezone.now().isoformat()]] + + data = [['Date', timezone.now().isoformat()]] data += compute_course_stats(course).items() if request.user.is_staff: for field in course.fields.values(): @@ -938,11 +938,10 @@ def instructor_dashboard(request, course_id): "StudentsDailyActivity", # active students by day "StudentsDropoffPerDay", # active students dropoff by day # "OverallGradeDistribution", # overall point distribution for course - "StudentsActive", # num students active in time period (default = 1wk) - "StudentsEnrolled", # num students enrolled # "StudentsPerProblemCorrect", # foreach problem, num students correct "ProblemGradeDistribution", # foreach problem, grade distribution ] + for analytic_name in DASHBOARD_ANALYTICS: analytics_results[analytic_name] = get_analytics_result(analytic_name) diff --git a/lms/envs/common.py b/lms/envs/common.py index f3c427a937..fdf73a86c4 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -286,7 +286,10 @@ FEATURES = { 'ALLOW_AUTOMATED_SIGNUPS': False, # Display demographic data on the analytics tab in the instructor dashboard. - 'DISPLAY_ANALYTICS_DEMOGRAPHICS': True + 'DISPLAY_ANALYTICS_DEMOGRAPHICS': True, + + # Enable display of enrollment counts in instructor and legacy analytics dashboard + 'DISPLAY_ANALYTICS_ENROLLMENTS': True, } # Ignore static asset files on import which match this pattern diff --git a/lms/templates/courseware/instructor_dashboard.html b/lms/templates/courseware/instructor_dashboard.html index 606b5fbc01..70db13b30a 100644 --- a/lms/templates/courseware/instructor_dashboard.html +++ b/lms/templates/courseware/instructor_dashboard.html @@ -609,22 +609,6 @@ function goto( mode)

${_("No Analytics are available at this time.")}

%endif - %if analytics_results.get("StudentsEnrolled"): -

- ${_("Students enrolled (historical count, includes those who have since unenrolled):")} - ${analytics_results["StudentsEnrolled"]['data'][0]['students']} - (${analytics_results["StudentsEnrolled"]['time']}) -

- %endif - - %if analytics_results.get("StudentsActive"): -

- ${_("Students active in the last week:")} - ${analytics_results["StudentsActive"]['data'][0]['active']} - (${analytics_results["StudentsActive"]['time']}) -

- %endif - %if analytics_results.get("StudentsDropoffPerDay"):

${_("Student activity day by day")} diff --git a/lms/templates/instructor/instructor_dashboard_2/course_info.html b/lms/templates/instructor/instructor_dashboard_2/course_info.html index bd6462934f..f71fb53646 100644 --- a/lms/templates/instructor/instructor_dashboard_2/course_info.html +++ b/lms/templates/instructor/instructor_dashboard_2/course_info.html @@ -1,28 +1,35 @@ <%! from django.utils.translation import ugettext as _ %> <%page args="section_data"/> -

-

${_("Enrollment Information")}

- ## Translators: 'track' refers to the enrollment type ('honor', 'verified', or 'audit') - ${_("Number of enrollees (instructors, staff members, and students) by track")} -

- <% modes = section_data['enrollment_count'] %> - - - - - - - - - - - - - -
${_("Verified")}${modes['verified']}
${_("Audit")}${modes['audit']}
${_("Honor")}${modes['honor']}
${_("Total")}${modes['total']}
-
-
+%if settings.FEATURES.get('DISPLAY_ANALYTICS_ENROLLMENTS') or section_data.get('enrollment_message'): +
+

${_("Enrollment Information")}

+ + %if settings.FEATURES.get('DISPLAY_ANALYTICS_ENROLLMENTS'): + ## Translators: 'track' refers to the enrollment type ('honor', 'verified', or 'audit') + ${_("Number of enrollees (instructors, staff members, and students) by track")} +

+ <% modes = section_data['enrollment_count'] %> + + + + + + + + + + + + + +
${_("Verified")}${modes['verified']}
${_("Audit")}${modes['audit']}
${_("Honor")}${modes['honor']}
${_("Total")}${modes['total']}
+ %elif section_data.get('enrollment_message'): +

${section_data['enrollment_message']}

+ %endif +
+
+%endif

${_("Basic Course Information")}

From 067e270a8498e42ccb5de22ba3659b394135fe8c Mon Sep 17 00:00:00 2001 From: Greg Price Date: Wed, 5 Nov 2014 15:48:51 -0500 Subject: [PATCH 02/48] Fix spelling of complementary role --- cms/templates/asset_index.html | 2 +- cms/templates/checklists.html | 2 +- cms/templates/container.html | 2 +- cms/templates/course-create-rerun.html | 2 +- cms/templates/course_outline.html | 2 +- cms/templates/edit-tabs.html | 2 +- cms/templates/export.html | 2 +- cms/templates/export_git.html | 2 +- cms/templates/group_configurations.html | 2 +- cms/templates/import.html | 2 +- cms/templates/index.html | 4 ++-- cms/templates/js/mock/mock-container-page.underscore | 2 +- cms/templates/js/mock/mock-course-outline-page.underscore | 2 +- .../js/mock/mock-group-configuration-page.underscore | 2 +- cms/templates/login.html | 2 +- cms/templates/manage_users.html | 2 +- cms/templates/register.html | 2 +- cms/templates/settings.html | 2 +- cms/templates/settings_advanced.html | 2 +- cms/templates/settings_graders.html | 2 +- cms/templates/textbooks.html | 2 +- cms/templates/ux/reference/container.html | 2 +- cms/templates/ux/reference/course-create-rerun.html | 2 +- cms/templates/ux/reference/outline.html | 2 +- 24 files changed, 25 insertions(+), 25 deletions(-) diff --git a/cms/templates/asset_index.html b/cms/templates/asset_index.html index 7e886b7241..481fb86077 100644 --- a/cms/templates/asset_index.html +++ b/cms/templates/asset_index.html @@ -52,7 +52,7 @@
-
+

${_("Adding Files for Your Course")}

diff --git a/cms/templates/checklists.html b/cms/templates/checklists.html index 75530df5ae..1c95dd7b22 100644 --- a/cms/templates/checklists.html +++ b/cms/templates/checklists.html @@ -42,7 +42,7 @@ -
+

${_("What are course checklists?")}

diff --git a/cms/templates/container.html b/cms/templates/container.html index 4a6e0eaae2..b24cffe2ea 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -106,7 +106,7 @@ templates = ["basic-modal", "modal-button", "edit-xblock-modal",

${_("Loading...")}

-
+
% if not is_unit_page:

${_("Adding components")}

diff --git a/cms/templates/course-create-rerun.html b/cms/templates/course-create-rerun.html index bee893a5c0..c20c85c9c1 100644 --- a/cms/templates/course-create-rerun.html +++ b/cms/templates/course-create-rerun.html @@ -123,7 +123,7 @@ -
+

${_("When will my course re-run start?")}

    diff --git a/cms/templates/course_outline.html b/cms/templates/course_outline.html index 1fe5648c04..10f151bbda 100644 --- a/cms/templates/course_outline.html +++ b/cms/templates/course_outline.html @@ -114,7 +114,7 @@ from contentstore.utils import reverse_usage_url

    ${_("Loading...")}

-
+

${_("Creating your course organization")}

${_("You add sections, subsections, and units directly in the outline.")}

diff --git a/cms/templates/edit-tabs.html b/cms/templates/edit-tabs.html index d36307d073..9223e245ae 100644 --- a/cms/templates/edit-tabs.html +++ b/cms/templates/edit-tabs.html @@ -137,7 +137,7 @@
-
+

${_("What are pages?")}

${_("Pages are listed horizontally at the top of your course. Default pages (Courseware, Course info, Discussion, Wiki, and Progress) are followed by textbooks and custom pages that you create.")}

diff --git a/cms/templates/export.html b/cms/templates/export.html index 0d8f783f2b..a6783b2ad0 100644 --- a/cms/templates/export.html +++ b/cms/templates/export.html @@ -82,7 +82,7 @@
-
+

${_("Why export a course?")}

${_("You may want to edit the XML in your course directly, outside of Studio. You may want to create a backup copy of your course. Or, you may want to create a copy of your course that you can later import into another course instance and customize.")}

diff --git a/cms/templates/export_git.html b/cms/templates/export_git.html index 608663752c..86b5037760 100644 --- a/cms/templates/export_git.html +++ b/cms/templates/export_git.html @@ -57,7 +57,7 @@ % endif
-
+
${_("Your course:")}
${context_course.id | h}
diff --git a/cms/templates/group_configurations.html b/cms/templates/group_configurations.html index 2cdf6ed7ee..d4d8650cef 100644 --- a/cms/templates/group_configurations.html +++ b/cms/templates/group_configurations.html @@ -59,7 +59,7 @@
% endif -
+

${_("What can I do on this page?")}

${_("You can create, edit, and delete group configurations.")}

diff --git a/cms/templates/import.html b/cms/templates/import.html index 2460b74ab9..63ab52ceb7 100644 --- a/cms/templates/import.html +++ b/cms/templates/import.html @@ -127,7 +127,7 @@ -
+

${_("Why import a course?")}

${_("You may want to run a new version of an existing course, or replace an existing course altogether. Or, you may have developed a course outside Studio.")}

diff --git a/cms/templates/index.html b/cms/templates/index.html index 91847cfab0..7a155f9cec 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -357,7 +357,7 @@ % endif -
+

${_('New to edX Studio?')}

${_('Click Help in the upper-right corner to get more information about the Studio page you are viewing. You can also use the links at the bottom of the page to access our continously updated documentation and other Studio resources.')}

@@ -422,7 +422,7 @@
-
+

${_('Need help?')}

${_('Please check your Junk or Spam folders in case our email isn\'t in your INBOX. Still can\'t find the verification email? Request help via the link below.')}

diff --git a/cms/templates/js/mock/mock-container-page.underscore b/cms/templates/js/mock/mock-container-page.underscore index 99d21ddfa8..31dfe8edba 100644 --- a/cms/templates/js/mock/mock-container-page.underscore +++ b/cms/templates/js/mock/mock-container-page.underscore @@ -50,7 +50,7 @@

Loading...

-
+
diff --git a/cms/templates/js/mock/mock-course-outline-page.underscore b/cms/templates/js/mock/mock-course-outline-page.underscore index 58b54c319b..9ca1cc6bd1 100644 --- a/cms/templates/js/mock/mock-course-outline-page.underscore +++ b/cms/templates/js/mock/mock-course-outline-page.underscore @@ -47,7 +47,7 @@

Loading...

-
+

What can I do on this page?

You can create new sections and subsections, set the release date for sections, and create new units in existing subsections. You can set the assignment type for subsections that are to be graded, and you can open a subsection for further editing.

diff --git a/cms/templates/js/mock/mock-group-configuration-page.underscore b/cms/templates/js/mock/mock-group-configuration-page.underscore index 5ccee0fdcb..3236663876 100644 --- a/cms/templates/js/mock/mock-group-configuration-page.underscore +++ b/cms/templates/js/mock/mock-group-configuration-page.underscore @@ -23,7 +23,7 @@

${_("Loading...")}

-
+
diff --git a/cms/templates/login.html b/cms/templates/login.html index 7034cfad48..9f26e8688f 100644 --- a/cms/templates/login.html +++ b/cms/templates/login.html @@ -45,7 +45,7 @@ from django.utils.translation import ugettext as _ -
+

${_("Studio Support")}

diff --git a/cms/templates/manage_users.html b/cms/templates/manage_users.html index 982d84cf94..6dea0d4627 100644 --- a/cms/templates/manage_users.html +++ b/cms/templates/manage_users.html @@ -143,7 +143,7 @@ %endif -
+

${_("Course Team Roles")}

${_("Course team members, or staff, are course co-authors. They have full writing and editing privileges on all course content.")}

diff --git a/cms/templates/register.html b/cms/templates/register.html index dd1e72c5ca..fef84c3070 100644 --- a/cms/templates/register.html +++ b/cms/templates/register.html @@ -77,7 +77,7 @@ -
+

${_("Common Studio Questions")}

diff --git a/cms/templates/settings.html b/cms/templates/settings.html index b9026083e6..4746f6bb91 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -292,7 +292,7 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url}'; % endif -
+

${_("How are these settings used?")}

${_("Your course's schedule determines when students can enroll in and begin a course.")}

diff --git a/cms/templates/settings_advanced.html b/cms/templates/settings_advanced.html index 7a0bca3c16..f4467945ff 100644 --- a/cms/templates/settings_advanced.html +++ b/cms/templates/settings_advanced.html @@ -68,7 +68,7 @@ -
+

${_("What do advanced settings do?")}

${_("Advanced settings control specific course functionality. On this page, you can edit manual policies, which are JSON-based key and value pairs that control specific course settings.")}

diff --git a/cms/templates/settings_graders.html b/cms/templates/settings_graders.html index 7d309b0f55..a6946dc6c4 100644 --- a/cms/templates/settings_graders.html +++ b/cms/templates/settings_graders.html @@ -112,7 +112,7 @@ -
+

${_("What can I do on this page?")}

${_("You can use the slider under Overall Grade Range to specify whether your course is pass/fail or graded by letter, and to establish the thresholds for each grade.")}

diff --git a/cms/templates/textbooks.html b/cms/templates/textbooks.html index 2a28b03c4b..b3a97b2a9c 100644 --- a/cms/templates/textbooks.html +++ b/cms/templates/textbooks.html @@ -54,7 +54,7 @@ CMS.URL.LMS_BASE = "${settings.LMS_BASE}"
-
+

${_("Why should I break my textbook into chapters?")}

${_("Breaking your textbook into multiple chapters reduces loading times for students, especially those with slow Internet connections. Breaking up textbooks into chapters can also help students more easily find topic-based information.")}

diff --git a/cms/templates/ux/reference/container.html b/cms/templates/ux/reference/container.html index e76aec1227..880a53d6fa 100644 --- a/cms/templates/ux/reference/container.html +++ b/cms/templates/ux/reference/container.html @@ -455,7 +455,7 @@ -
+
diff --git a/cms/templates/ux/reference/course-create-rerun.html b/cms/templates/ux/reference/course-create-rerun.html index 32c9477896..1d2e835367 100644 --- a/cms/templates/ux/reference/course-create-rerun.html +++ b/cms/templates/ux/reference/course-create-rerun.html @@ -325,7 +325,7 @@ -
+

When will my course re-run start?

    diff --git a/cms/templates/ux/reference/outline.html b/cms/templates/ux/reference/outline.html index 19ca489886..e2475910b2 100644 --- a/cms/templates/ux/reference/outline.html +++ b/cms/templates/ux/reference/outline.html @@ -757,7 +757,7 @@ from django.core.urlresolvers import reverse -
    +

    What can I do on this page?

    You can create new sections and subsections, set the release date for sections, and create new units in existing subsections. You can set the assignment type for subsections that are to be graded, and you can open a subsection for further editing.

    From 66b3e3828f034fca695676d2a1f481d10f240f3c Mon Sep 17 00:00:00 2001 From: Dennis Jen Date: Wed, 5 Nov 2014 17:14:33 -0500 Subject: [PATCH 03/48] added tests for instructor_dashboard.py, added docstring for _is_small_course, removed check for settings.ANALYTICS_DASHBOARD_URL when creating dashboard link --- .../tests/test_instructor_dashboard.py | 125 ++++++++++++++++++ .../instructor/views/instructor_dashboard.py | 9 +- 2 files changed, 129 insertions(+), 5 deletions(-) create mode 100644 lms/djangoapps/instructor/tests/test_instructor_dashboard.py diff --git a/lms/djangoapps/instructor/tests/test_instructor_dashboard.py b/lms/djangoapps/instructor/tests/test_instructor_dashboard.py new file mode 100644 index 0000000000..957c577433 --- /dev/null +++ b/lms/djangoapps/instructor/tests/test_instructor_dashboard.py @@ -0,0 +1,125 @@ +""" +Unit tests for instructor_dashboard.py. +""" +from mock import patch + +from django.conf import settings +from django.core.urlresolvers import reverse +from django.test.utils import override_settings +from courseware.tests.helpers import LoginEnrollmentTestCase + +from student.tests.factories import AdminFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + + +class TestInstructorDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): + """ + Tests for the instructor dashboard (not legacy). + """ + + def setUp(self): + """ + Set up tests + """ + self.course = CourseFactory.create() + + # Create instructor account + instructor = AdminFactory.create() + self.client.login(username=instructor.username, password="test") + + # URL for instructor dash + self.url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id.to_deprecated_string()}) + + def tearDown(self): + """ + Undo patches. + """ + patch.stopall() + + def get_dashboard_enrollment_message(self): + return 'Enrollment data is now available in Example.'.format(unicode(self.course.id)) + + def get_dashboard_demographic_message(self): + return 'Demographic data is now available in Example.'.format(unicode(self.course.id)) + + @patch.dict(settings.FEATURES, {'DISPLAY_ANALYTICS_ENROLLMENTS': False}) + @override_settings(ANALYTICS_DASHBOARD_URL='') + def test_no_enrollments(self): + """ + Test enrollment section is hidden. + """ + response = self.client.get(self.url) + # no enrollment information should be visible + self.assertFalse('

    Enrollment Information

    ' in response.content) + + @patch.dict(settings.FEATURES, {'DISPLAY_ANALYTICS_ENROLLMENTS': True}) + @override_settings(ANALYTICS_DASHBOARD_URL='') + def test_show_enrollments_data(self): + """ + Test enrollment data is shown. + """ + response = self.client.get(self.url) + + # enrollment information visible + self.assertTrue('

    Enrollment Information

    ' in response.content) + self.assertTrue('Verified' in response.content) + self.assertTrue('Audit' in response.content) + self.assertTrue('Honor' in response.content) + + # dashboard link hidden + self.assertFalse(self.get_dashboard_enrollment_message() in response.content) + + @patch.dict(settings.FEATURES, {'DISPLAY_ANALYTICS_ENROLLMENTS': False}) + @override_settings(ANALYTICS_DASHBOARD_URL='http://example.com') + @override_settings(ANALYTICS_DASHBOARD_NAME='Example') + def test_show_dashboard_enrollment_message(self): + """ + Test enrollment dashboard message is shown and data is hidden. + """ + response = self.client.get(self.url) + + # enrollment information hidden + self.assertFalse('Verified' in response.content) + self.assertFalse('Audit' in response.content) + self.assertFalse('Honor' in response.content) + + # link to dashboard shown + expected_message = self.get_dashboard_enrollment_message() + self.assertTrue(expected_message in response.content) + + @patch.dict(settings.FEATURES, {'DISPLAY_ANALYTICS_DEMOGRAPHICS': True}) + @override_settings(ANALYTICS_DASHBOARD_URL='') + @override_settings(ANALYTICS_DASHBOARD_NAME='') + def test_show_dashboard_demographic_data(self): + """ + Test enrollment demographic data is shown. + """ + response = self.client.get(self.url) + # demographic information displayed + self.assertTrue('data-feature="year_of_birth"' in response.content) + self.assertTrue('data-feature="gender"' in response.content) + self.assertTrue('data-feature="level_of_education"' in response.content) + + # dashboard link hidden + self.assertFalse(self.get_dashboard_demographic_message() in response.content) + + @patch.dict(settings.FEATURES, {'DISPLAY_ANALYTICS_DEMOGRAPHICS': False}) + @override_settings(ANALYTICS_DASHBOARD_URL='http://example.com') + @override_settings(ANALYTICS_DASHBOARD_NAME='Example') + def test_show_dashboard_demographic_message(self): + """ + Test enrollment demographic dashboard message is shown and data is hidden. + """ + response = self.client.get(self.url) + + # demographics are hidden + self.assertFalse('data-feature="year_of_birth"' in response.content) + self.assertFalse('data-feature="gender"' in response.content) + self.assertFalse('data-feature="level_of_education"' in response.content) + + # link to dashboard shown + expected_message = self.get_dashboard_demographic_message() + self.assertTrue(expected_message in response.content) diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index f894304954..585b2fffb6 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -263,6 +263,7 @@ def _section_membership(course, access): def _is_small_course(course_key): + """ Compares against MAX_ENROLLMENT_INSTR_BUTTONS to determine if course enrollment is considered small. """ is_small_course = False enrollment_count = CourseEnrollment.num_enrolled_in(course_key) max_enrollment_for_buttons = settings.FEATURES.get("MAX_ENROLLMENT_INSTR_BUTTONS") @@ -364,11 +365,9 @@ def _section_send_email(course, access): def _get_dashboard_link(course_key): # Construct a URL to the external analytics dashboard - link = None - if settings.ANALYTICS_DASHBOARD_URL: - analytics_dashboard_url = '{0}/courses/{1}'.format(settings.ANALYTICS_DASHBOARD_URL, unicode(course_key)) - link = "{1}".format(analytics_dashboard_url, - settings.ANALYTICS_DASHBOARD_NAME) + analytics_dashboard_url = '{0}/courses/{1}'.format(settings.ANALYTICS_DASHBOARD_URL, unicode(course_key)) + link = "{1}".format(analytics_dashboard_url, + settings.ANALYTICS_DASHBOARD_NAME) return link From 24ad0c31ea6975aa69a226bc18def34a1675e52e Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Fri, 31 Oct 2014 11:30:12 -0400 Subject: [PATCH 04/48] Pylint: ignore TODO comments --- pylintrc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pylintrc b/pylintrc index 21966c7f8a..1d4902e732 100644 --- a/pylintrc +++ b/pylintrc @@ -41,7 +41,8 @@ disable= # W0142: Used * or ** magic # R0921: Abstract class not referenced # R0922: Abstract class is only referenced 1 times - I0011,C0301,W0141,W0142,R0921,R0922, +# W0511: TODO comments + I0011,C0301,W0141,W0142,R0921,R0922,W0511, # Django makes classes that trigger these # W0232: Class has no __init__ method From 58fd09182325a275a7a4e990c0b61b42a5e55306 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Mon, 3 Nov 2014 11:36:24 -0500 Subject: [PATCH 05/48] Disable TODO comments in `paver quality` only --- pavelib/quality.py | 6 ++++-- pylintrc | 3 +-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/pavelib/quality.py b/pavelib/quality.py index 887b0997a8..ad617bf17f 100644 --- a/pavelib/quality.py +++ b/pavelib/quality.py @@ -30,7 +30,9 @@ def run_pylint(options): # This makes the folder if it doesn't already exist. report_dir = (Env.REPORT_DIR / system).makedirs_p() - flags = '-E' if errors else '' + flags = ["--disable=fixme"] + if errors: + flags.append("--errors-only") apps = [system] @@ -51,7 +53,7 @@ def run_pylint(options): "{pythonpath_prefix} pylint {flags} -f parseable {apps} | " "tee {report_dir}/pylint.report".format( pythonpath_prefix=pythonpath_prefix, - flags=flags, + flags=" ".join(flags), apps=apps_list, report_dir=report_dir ) diff --git a/pylintrc b/pylintrc index 1d4902e732..21966c7f8a 100644 --- a/pylintrc +++ b/pylintrc @@ -41,8 +41,7 @@ disable= # W0142: Used * or ** magic # R0921: Abstract class not referenced # R0922: Abstract class is only referenced 1 times -# W0511: TODO comments - I0011,C0301,W0141,W0142,R0921,R0922,W0511, + I0011,C0301,W0141,W0142,R0921,R0922, # Django makes classes that trigger these # W0232: Class has no __init__ method From 9902dcb10f7580538ad9cf86751868f8ed173fb8 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Tue, 4 Nov 2014 14:42:54 -0500 Subject: [PATCH 06/48] Disable FIXME errors on `paver run_quality` Not on `paver run_pylint` --- pavelib/quality.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pavelib/quality.py b/pavelib/quality.py index ad617bf17f..a919f3b6f4 100644 --- a/pavelib/quality.py +++ b/pavelib/quality.py @@ -30,7 +30,7 @@ def run_pylint(options): # This makes the folder if it doesn't already exist. report_dir = (Env.REPORT_DIR / system).makedirs_p() - flags = ["--disable=fixme"] + flags = [] if errors: flags.append("--errors-only") @@ -180,12 +180,15 @@ def run_quality(options): try: sh( - "{pythonpath_prefix} diff-quality --violations=pylint {pylint_reports} {percentage_string} " - "--html-report {dquality_dir}/diff_quality_pylint.html".format( + "{pythonpath_prefix} diff-quality --violations=pylint " + "{pylint_reports} {percentage_string} " + "--html-report {dquality_dir}/diff_quality_pylint.html " + "--options='{pylint_options}'".format( pythonpath_prefix=pythonpath_prefix, pylint_reports=pylint_reports, percentage_string=percentage_string, - dquality_dir=dquality_dir + dquality_dir=dquality_dir, + pylint_options="--disable=fixme", ) ) except BuildFailure, error_message: From d3e051ce2973b05698e906ded8a2660779b10199 Mon Sep 17 00:00:00 2001 From: Dennis Jen Date: Thu, 6 Nov 2014 13:42:00 -0500 Subject: [PATCH 07/48] fixed quaility errors in instructor_dashboard.py, moved test_instructor_dashboard under tests/views directory --- .../tests/{ => views}/test_instructor_dashboard.py | 6 ++++++ lms/djangoapps/instructor/views/instructor_dashboard.py | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) rename lms/djangoapps/instructor/tests/{ => views}/test_instructor_dashboard.py (96%) diff --git a/lms/djangoapps/instructor/tests/test_instructor_dashboard.py b/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py similarity index 96% rename from lms/djangoapps/instructor/tests/test_instructor_dashboard.py rename to lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py index 957c577433..535676a320 100644 --- a/lms/djangoapps/instructor/tests/test_instructor_dashboard.py +++ b/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py @@ -38,10 +38,16 @@ class TestInstructorDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): patch.stopall() def get_dashboard_enrollment_message(self): + """ + Returns expected dashboard enrollment message with link to Insights. + """ return 'Enrollment data is now available in Example.'.format(unicode(self.course.id)) def get_dashboard_demographic_message(self): + """ + Returns expected dashboard demographic message with link to Insights. + """ return 'Demographic data is now available in Example.'.format(unicode(self.course.id)) diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 585b2fffb6..8495e0c5de 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -275,7 +275,7 @@ def _is_small_course(course_key): def _section_student_admin(course, access): """ Provide data for the corresponding dashboard section """ course_key = course.id - is_small_course =_is_small_course(course_key) + is_small_course = _is_small_course(course_key) section_data = { 'section_key': 'student_admin', @@ -364,7 +364,7 @@ def _section_send_email(course, access): def _get_dashboard_link(course_key): - # Construct a URL to the external analytics dashboard + """ Construct a URL to the external analytics dashboard """ analytics_dashboard_url = '{0}/courses/{1}'.format(settings.ANALYTICS_DASHBOARD_URL, unicode(course_key)) link = "{1}".format(analytics_dashboard_url, settings.ANALYTICS_DASHBOARD_NAME) From 18083e98796ea75fd762146ddbd10b4ce238f773 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Mon, 10 Nov 2014 12:07:22 -0500 Subject: [PATCH 08/48] Use @require_POST decorator for password reset view --- common/djangoapps/student/views.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index c51f995012..388a2b8bfe 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -1790,11 +1790,9 @@ def activate_account(request, key): @csrf_exempt +@require_POST def password_reset(request): """ Attempts to send a password reset e-mail. """ - if request.method != "POST": - raise Http404 - # Add some rate limiting here by re-using the RateLimitMixin as a helper class limiter = BadRequestRateLimiter() if limiter.is_rate_limit_exceeded(request): From cc48800d545afb6a91ce0bf54e2c7d09eb7aba1e Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Mon, 10 Nov 2014 11:05:56 -0500 Subject: [PATCH 09/48] add logging for orders when 'purchase' is called on them multiple times ECOM-630 --- lms/djangoapps/shoppingcart/models.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lms/djangoapps/shoppingcart/models.py b/lms/djangoapps/shoppingcart/models.py index 37a32dfe0c..1b33941a03 100644 --- a/lms/djangoapps/shoppingcart/models.py +++ b/lms/djangoapps/shoppingcart/models.py @@ -352,6 +352,9 @@ class Order(models.Model): """ if self.status == 'purchased': + log.error( + u"`purchase` method called on order {}, but order is already purchased.".format(self.id) # pylint: disable=E1101 + ) return self.status = 'purchased' self.purchase_time = datetime.now(pytz.utc) From c550963fb99cc2ff4acfbd44bf1686fd8606abe7 Mon Sep 17 00:00:00 2001 From: cahrens Date: Fri, 7 Nov 2014 09:52:54 -0500 Subject: [PATCH 10/48] Set up validation_messages so that it goes through the RequireJS optimizer. --- cms/static/build.js | 3 +- cms/static/coffee/spec/main.coffee | 2 + cms/static/js/factories/xblock_validation.js | 15 ++++ .../spec/factories/xblock_validation_spec.js | 78 +++++++++++++++++++ cms/templates/studio_xblock_wrapper.html | 33 +++----- 5 files changed, 106 insertions(+), 25 deletions(-) create mode 100644 cms/static/js/factories/xblock_validation.js create mode 100644 cms/static/js/spec/factories/xblock_validation_spec.js diff --git a/cms/static/build.js b/cms/static/build.js index d211b2a0e6..4999c9ab88 100644 --- a/cms/static/build.js +++ b/cms/static/build.js @@ -45,7 +45,8 @@ 'js/factories/settings', 'js/factories/settings_advanced', 'js/factories/settings_graders', - 'js/factories/textbooks' + 'js/factories/textbooks', + 'js/factories/xblock_validation' ]), /** * By default all the configuration for optimization happens from the command diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee index 45ff7269d6..62829c9b7f 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -244,6 +244,8 @@ define([ "js/spec/views/modals/edit_xblock_spec", "js/spec/views/modals/validation_error_modal_spec", + "js/spec/factories/xblock_validation_spec", + "js/spec/xblock/cms.runtime.v1_spec", # these tests are run separately in the cms-squire suite, due to process diff --git a/cms/static/js/factories/xblock_validation.js b/cms/static/js/factories/xblock_validation.js new file mode 100644 index 0000000000..61e0d8b91a --- /dev/null +++ b/cms/static/js/factories/xblock_validation.js @@ -0,0 +1,15 @@ +define(["js/views/xblock_validation", "js/models/xblock_validation"], +function (XBlockValidationView, XBlockValidationModel) { + 'use strict'; + return function (validationMessages, hasEditingUrl, isRoot, validationEle) { + if (hasEditingUrl && !isRoot) { + validationMessages.showSummaryOnly = true; + } + + var model = new XBlockValidationModel(validationMessages, {parse: true}); + + if (!model.get("empty")) { + new XBlockValidationView({el: validationEle, model: model, root: isRoot}).render(); + } + }; +}); diff --git a/cms/static/js/spec/factories/xblock_validation_spec.js b/cms/static/js/spec/factories/xblock_validation_spec.js new file mode 100644 index 0000000000..19dc94d71e --- /dev/null +++ b/cms/static/js/spec/factories/xblock_validation_spec.js @@ -0,0 +1,78 @@ +define(['jquery', 'js/factories/xblock_validation', 'js/common_helpers/template_helpers'], + function($, XBlockValidationFactory, TemplateHelpers) { + + describe('XBlockValidationFactory', function() { + var messageDiv; + + beforeEach(function () { + TemplateHelpers.installTemplate('xblock-validation-messages'); + appendSetFixtures($('
    ')); + messageDiv = $('.messages'); + }); + + it('Does not attach a view if messages is empty', function() { + XBlockValidationFactory({"empty": true}, false, false, messageDiv); + expect(messageDiv.children().length).toEqual(0); + }); + + it('Does attach a view if messages are not empty', function() { + XBlockValidationFactory({"empty": false}, false, false, messageDiv); + expect(messageDiv.children().length).toEqual(1); + }); + + it('Passes through the root property to the view.', function() { + var noContainerContent = "no-container-content"; + + var notConfiguredMessages = { + "empty": false, + "summary": {"text": "my summary", "type": "not-configured"}, + "messages": [], + "xblock_id": "id" + }; + // Root is false, will not add noContainerContent. + XBlockValidationFactory(notConfiguredMessages, true, false, messageDiv); + expect(messageDiv.find('.validation')).not.toHaveClass(noContainerContent); + + // Root is true, will add noContainerContent. + XBlockValidationFactory(notConfiguredMessages, true, true, messageDiv); + expect(messageDiv.find('.validation')).toHaveClass(noContainerContent); + }); + + describe('Controls display of detailed messages based on url and root property', function() { + var messagesWithSummary, checkDetailedMessages; + + beforeEach(function () { + messagesWithSummary = { + "empty": false, + "summary": {"text": "my summary"}, + "messages": [{"text": "one", "type": "warning"}, {"text": "two", "type": "error"}], + "xblock_id": "id" + }; + }); + + checkDetailedMessages = function (expectedDetailedMessages) { + expect(messageDiv.children().length).toEqual(1); + expect(messageDiv.find('.xblock-message-item').length).toBe(expectedDetailedMessages); + }; + + it('Does not show details if xblock has an editing URL and it is not rendered as root', function() { + XBlockValidationFactory(messagesWithSummary, true, false, messageDiv); + checkDetailedMessages(0); + }); + + it('Shows details if xblock does not have its own editing URL, regardless of root value', function() { + XBlockValidationFactory(messagesWithSummary, false, false, messageDiv); + checkDetailedMessages(2); + + XBlockValidationFactory(messagesWithSummary, false, true, messageDiv); + checkDetailedMessages(2); + }); + + it('Shows details if xblock has its own editing URL and is rendered as root', function() { + XBlockValidationFactory(messagesWithSummary, true, true, messageDiv); + checkDetailedMessages(2); + }); + }); + }); + } +); diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index 1573773561..d6c707b661 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -1,4 +1,3 @@ - <%! from django.utils.translation import ugettext as _ from contentstore.views.helpers import xblock_studio_url @@ -21,31 +20,17 @@ messages = json.dumps(xblock.validate().to_json()) - - % if not is_root: % if is_reorderable:
  • From 99c4413bd08a0a13c65f5d9ff00dd766480e6d2a Mon Sep 17 00:00:00 2001 From: cahrens Date: Mon, 10 Nov 2014 09:52:21 -0500 Subject: [PATCH 11/48] Simply check for existing text and type. --- cms/static/js/models/xblock_validation.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cms/static/js/models/xblock_validation.js b/cms/static/js/models/xblock_validation.js index f48f593afd..3821b0fd19 100644 --- a/cms/static/js/models/xblock_validation.js +++ b/cms/static/js/models/xblock_validation.js @@ -18,10 +18,10 @@ define(["backbone", "gettext", "underscore"], function (Backbone, gettext, _) { if (!response.empty) { var summary = "summary" in response ? response.summary : {}; var messages = "messages" in response ? response.messages : []; - if (!(_.has(summary, "text")) || !summary.text) { + if (!summary.text) { summary.text = gettext("This component has validation issues."); } - if (!(_.has(summary, "type")) || !summary.type) { + if (!summary.type) { summary.type = this.WARNING; // Possible types are ERROR, WARNING, and NOT_CONFIGURED. NOT_CONFIGURED is treated as a warning. _.find(messages, function (message) { From ddc0015cdfda2f05eabed48c1b5975f0e2c1fd91 Mon Sep 17 00:00:00 2001 From: cahrens Date: Mon, 10 Nov 2014 13:42:11 -0500 Subject: [PATCH 12/48] Clarify types of Python values. --- cms/templates/studio_xblock_wrapper.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index d6c707b661..4ad50666d5 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -24,8 +24,8 @@ messages = json.dumps(xblock.validate().to_json()) require(["jquery", "js/factories/xblock_validation"], function($, XBlockValidationFactory) { XBlockValidationFactory( ${messages}, - $.parseJSON("${bool(xblock_url)}".toLowerCase()), - $.parseJSON("${is_root == True}".toLowerCase()), + $.parseJSON("${bool(xblock_url)}".toLowerCase()), // xblock_url will be None or a string + $.parseJSON("${bool(is_root)}".toLowerCase()), // is_root will be None or a boolean $('div.xblock-validation-messages[data-locator="${xblock.location | h}"]') ); }); From d87cfa90bf5f759bc9e5a8240ddb7f601dbdfb67 Mon Sep 17 00:00:00 2001 From: stv Date: Sat, 8 Nov 2014 17:15:11 -0800 Subject: [PATCH 13/48] Fix PEP8: W292 no newline at end of file --- common/djangoapps/monitoring/startup.py | 2 +- common/djangoapps/request_cache/middleware.py | 2 +- common/djangoapps/student/forms.py | 2 +- .../djangoapps/student/migrations/0041_add_dashboard_config.py | 2 +- common/djangoapps/util/query.py | 2 +- ...ld_optout_course_id__add_field_courseemail_template_name_.py | 2 +- lms/djangoapps/courseware/features/gst.py | 2 +- .../shoppingcart/migrations/0018_auto__add_donation.py | 2 +- .../migrations/0019_auto__add_donationconfiguration.py | 2 +- ...rseregcodeitem__add_courseregcodeitemannotation__add_fiel.py | 2 +- ...add_field_orderitem_created__add_field_orderitem_modified.py | 2 +- 11 files changed, 11 insertions(+), 11 deletions(-) diff --git a/common/djangoapps/monitoring/startup.py b/common/djangoapps/monitoring/startup.py index 6c3e73a183..46cd211347 100644 --- a/common/djangoapps/monitoring/startup.py +++ b/common/djangoapps/monitoring/startup.py @@ -1,3 +1,3 @@ # Register signal handlers import signals -import exceptions \ No newline at end of file +import exceptions diff --git a/common/djangoapps/request_cache/middleware.py b/common/djangoapps/request_cache/middleware.py index 9d3dffdf27..ef8ec75489 100644 --- a/common/djangoapps/request_cache/middleware.py +++ b/common/djangoapps/request_cache/middleware.py @@ -17,4 +17,4 @@ class RequestCache(object): def process_response(self, request, response): self.clear_request_cache() - return response \ No newline at end of file + return response diff --git a/common/djangoapps/student/forms.py b/common/djangoapps/student/forms.py index ec30aae4de..19a3d5fdf4 100644 --- a/common/djangoapps/student/forms.py +++ b/common/djangoapps/student/forms.py @@ -22,4 +22,4 @@ class PasswordResetFormNoActive(PasswordResetForm): if any((user.password == UNUSABLE_PASSWORD) for user in self.users_cache): raise forms.ValidationError(self.error_messages['unusable']) - return email \ No newline at end of file + return email diff --git a/common/djangoapps/student/migrations/0041_add_dashboard_config.py b/common/djangoapps/student/migrations/0041_add_dashboard_config.py index a8f8af5fb5..13c10d584a 100644 --- a/common/djangoapps/student/migrations/0041_add_dashboard_config.py +++ b/common/djangoapps/student/migrations/0041_add_dashboard_config.py @@ -176,4 +176,4 @@ class Migration(SchemaMigration): } } - complete_apps = ['student'] \ No newline at end of file + complete_apps = ['student'] diff --git a/common/djangoapps/util/query.py b/common/djangoapps/util/query.py index 6800bc45ab..6e8182e6a8 100644 --- a/common/djangoapps/util/query.py +++ b/common/djangoapps/util/query.py @@ -6,4 +6,4 @@ def use_read_replica_if_available(queryset): """ If there is a database called 'read_replica', use that database for the queryset. """ - return queryset.using("read_replica") if "read_replica" in settings.DATABASES else queryset \ No newline at end of file + return queryset.using("read_replica") if "read_replica" in settings.DATABASES else queryset diff --git a/lms/djangoapps/bulk_email/migrations/0010_auto__chg_field_optout_course_id__add_field_courseemail_template_name_.py b/lms/djangoapps/bulk_email/migrations/0010_auto__chg_field_optout_course_id__add_field_courseemail_template_name_.py index 90e472fb99..b9d77f7dd0 100644 --- a/lms/djangoapps/bulk_email/migrations/0010_auto__chg_field_optout_course_id__add_field_courseemail_template_name_.py +++ b/lms/djangoapps/bulk_email/migrations/0010_auto__chg_field_optout_course_id__add_field_courseemail_template_name_.py @@ -126,4 +126,4 @@ class Migration(SchemaMigration): } } - complete_apps = ['bulk_email'] \ No newline at end of file + complete_apps = ['bulk_email'] diff --git a/lms/djangoapps/courseware/features/gst.py b/lms/djangoapps/courseware/features/gst.py index a8693ead30..bbb0c76f6a 100644 --- a/lms/djangoapps/courseware/features/gst.py +++ b/lms/djangoapps/courseware/features/gst.py @@ -73,4 +73,4 @@ class GraphicalSliderToolSteps(object): world.retry_on_exception(try_move) -GraphicalSliderToolSteps() \ No newline at end of file +GraphicalSliderToolSteps() diff --git a/lms/djangoapps/shoppingcart/migrations/0018_auto__add_donation.py b/lms/djangoapps/shoppingcart/migrations/0018_auto__add_donation.py index 2107814d39..fd89917904 100644 --- a/lms/djangoapps/shoppingcart/migrations/0018_auto__add_donation.py +++ b/lms/djangoapps/shoppingcart/migrations/0018_auto__add_donation.py @@ -188,4 +188,4 @@ class Migration(SchemaMigration): } } - complete_apps = ['shoppingcart'] \ No newline at end of file + complete_apps = ['shoppingcart'] diff --git a/lms/djangoapps/shoppingcart/migrations/0019_auto__add_donationconfiguration.py b/lms/djangoapps/shoppingcart/migrations/0019_auto__add_donationconfiguration.py index 8970f82b83..78c0bdde14 100644 --- a/lms/djangoapps/shoppingcart/migrations/0019_auto__add_donationconfiguration.py +++ b/lms/djangoapps/shoppingcart/migrations/0019_auto__add_donationconfiguration.py @@ -196,4 +196,4 @@ class Migration(SchemaMigration): } } - complete_apps = ['shoppingcart'] \ No newline at end of file + complete_apps = ['shoppingcart'] diff --git a/lms/djangoapps/shoppingcart/migrations/0020_auto__add_courseregcodeitem__add_courseregcodeitemannotation__add_fiel.py b/lms/djangoapps/shoppingcart/migrations/0020_auto__add_courseregcodeitem__add_courseregcodeitemannotation__add_fiel.py index ce99fdb642..941fcd0100 100644 --- a/lms/djangoapps/shoppingcart/migrations/0020_auto__add_courseregcodeitem__add_courseregcodeitemannotation__add_fiel.py +++ b/lms/djangoapps/shoppingcart/migrations/0020_auto__add_courseregcodeitem__add_courseregcodeitemannotation__add_fiel.py @@ -281,4 +281,4 @@ class Migration(SchemaMigration): } } - complete_apps = ['shoppingcart'] \ No newline at end of file + complete_apps = ['shoppingcart'] diff --git a/lms/djangoapps/shoppingcart/migrations/0021_auto__add_field_orderitem_created__add_field_orderitem_modified.py b/lms/djangoapps/shoppingcart/migrations/0021_auto__add_field_orderitem_created__add_field_orderitem_modified.py index f6870ff5a4..c4ca4bc4c8 100644 --- a/lms/djangoapps/shoppingcart/migrations/0021_auto__add_field_orderitem_created__add_field_orderitem_modified.py +++ b/lms/djangoapps/shoppingcart/migrations/0021_auto__add_field_orderitem_created__add_field_orderitem_modified.py @@ -221,4 +221,4 @@ class Migration(SchemaMigration): } } - complete_apps = ['shoppingcart'] \ No newline at end of file + complete_apps = ['shoppingcart'] From d5d6e3f0d8775e9cb705d8ddd661bc30aec869c4 Mon Sep 17 00:00:00 2001 From: stv Date: Sat, 8 Nov 2014 17:32:25 -0800 Subject: [PATCH 14/48] Fix PEP8: W293 blank line contains whitespace --- common/djangoapps/request_cache/middleware.py | 2 +- common/djangoapps/user_api/tests/test_account_api.py | 2 +- docs/shared/conf.py | 2 +- lms/djangoapps/courseware/features/events.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/common/djangoapps/request_cache/middleware.py b/common/djangoapps/request_cache/middleware.py index ef8ec75489..6640d1a38a 100644 --- a/common/djangoapps/request_cache/middleware.py +++ b/common/djangoapps/request_cache/middleware.py @@ -7,7 +7,7 @@ class RequestCache(object): @classmethod def get_request_cache(cls): return _request_cache_threadlocal - + def clear_request_cache(self): _request_cache_threadlocal.data = {} diff --git a/common/djangoapps/user_api/tests/test_account_api.py b/common/djangoapps/user_api/tests/test_account_api.py index 2acfbd09cf..3ab1aac029 100644 --- a/common/djangoapps/user_api/tests/test_account_api.py +++ b/common/djangoapps/user_api/tests/test_account_api.py @@ -298,7 +298,7 @@ class AccountApiTest(TestCase): if create_inactive_account: # Create an account, but do not activate it account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - + account_api.request_password_change(self.EMAIL, self.ORIG_HOST, self.IS_SECURE) # Verify that no email messages have been sent diff --git a/docs/shared/conf.py b/docs/shared/conf.py index 5f0a2abd89..91ce9ef1da 100644 --- a/docs/shared/conf.py +++ b/docs/shared/conf.py @@ -33,7 +33,7 @@ def add_base(paths): """ return [os.path.join(BASEDIR, x) for x in paths] - + # If extensions (or modules to document with autodoc) are in another directory, diff --git a/lms/djangoapps/courseware/features/events.py b/lms/djangoapps/courseware/features/events.py index cc81c129f7..b4b1d5001a 100644 --- a/lms/djangoapps/courseware/features/events.py +++ b/lms/djangoapps/courseware/features/events.py @@ -40,7 +40,7 @@ def reset_between_outline_scenarios(_scenario, order, outline, reasons_to_fail): def course_url_event_is_emitted(_step, url_regex): event_type = url_regex.format(world.scenario_dict['COURSE'].id) n_events_are_emitted(_step, 1, event_type, "server") - + @step(r'([aA]n?|\d+) "(.*)" (server|browser) events? is emitted$') def n_events_are_emitted(_step, count, event_type, event_source): From d9502e48deb10bb19f72361f0d221f58537d4b1f Mon Sep 17 00:00:00 2001 From: stv Date: Sat, 8 Nov 2014 20:09:33 -0800 Subject: [PATCH 15/48] Fix PEP8: E301 expected 1 blank line, found 0 --- common/lib/xmodule/xmodule/modulestore/split_mongo/split.py | 1 + .../modulestore/tests/test_split_modulestore_bulk_operations.py | 1 + common/lib/xmodule/xmodule/timeinfo.py | 1 + 3 files changed, 3 insertions(+) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 0b3882ede0..8b94851480 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -1442,6 +1442,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): return new_course DEFAULT_ROOT_BLOCK_ID = 'course' + def create_course( self, org, course, run, user_id, master_branch=None, fields=None, versions_dict=None, search_targets=None, root_category='course', diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore_bulk_operations.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore_bulk_operations.py index 5ff112c961..58f211b5e2 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore_bulk_operations.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore_bulk_operations.py @@ -439,6 +439,7 @@ class TestBulkWriteMixinFindMethods(TestBulkWriteMixin): def db_structure(_id): previous, _, current = _id.partition('.') return {'db': 'structure', 'previous_version': previous, '_id': current} + def active_structure(_id): previous, _, current = _id.partition('.') return {'active': 'structure', 'previous_version': previous, '_id': current} diff --git a/common/lib/xmodule/xmodule/timeinfo.py b/common/lib/xmodule/xmodule/timeinfo.py index 76f24a0b23..a55cbe3a63 100644 --- a/common/lib/xmodule/xmodule/timeinfo.py +++ b/common/lib/xmodule/xmodule/timeinfo.py @@ -14,6 +14,7 @@ class TimeInfo(object): """ _delta_standin = Timedelta() + def __init__(self, due_date, grace_period_string_or_timedelta): if due_date is not None: self.display_due_date = due_date From cf9308144d762a567a59bc223d262186928e3183 Mon Sep 17 00:00:00 2001 From: stv Date: Sat, 8 Nov 2014 17:18:52 -0800 Subject: [PATCH 16/48] Fix PEP8: E302 expected 2 blank lines, found 1 --- .../contentstore/management/commands/delete_course.py | 1 + .../contentstore/management/commands/populate_creators.py | 2 ++ cms/djangoapps/contentstore/tests/test_transcripts_utils.py | 1 + cms/djangoapps/contentstore/tests/test_utils.py | 1 + cms/djangoapps/contentstore/views/checklist.py | 1 + cms/djangoapps/contentstore/views/course.py | 1 + cms/djangoapps/contentstore/views/tabs.py | 1 + cms/djangoapps/models/settings/course_details.py | 1 + common/djangoapps/contentserver/middleware.py | 1 + common/djangoapps/course_groups/cohorts.py | 1 + common/djangoapps/course_groups/tests/test_views.py | 1 + common/djangoapps/django_comment_common/tests.py | 1 + common/djangoapps/edxmako/paths.py | 1 + common/djangoapps/edxmako/shortcuts.py | 2 ++ common/djangoapps/edxmako/tests.py | 1 + common/djangoapps/embargo/tests/test_middleware.py | 1 + common/djangoapps/external_auth/tests/test_ssl.py | 1 + common/djangoapps/heartbeat/tests/test_heartbeat.py | 1 + .../microsite_configuration/templatetags/microsite.py | 1 + common/djangoapps/monitoring/signals.py | 1 + common/djangoapps/request_cache/middleware.py | 1 + common/djangoapps/student/middleware.py | 1 + common/djangoapps/student/models.py | 4 ++++ common/djangoapps/student/roles.py | 1 + common/djangoapps/student/tests/test_auto_auth.py | 1 + common/djangoapps/student/tests/test_microsite.py | 3 +++ common/djangoapps/student/tests/test_password_policy.py | 1 + common/djangoapps/terrain/stubs/comments.py | 1 + common/djangoapps/terrain/stubs/lti.py | 1 + common/djangoapps/terrain/stubs/tests/test_lti_stub.py | 1 + common/djangoapps/terrain/stubs/video_source.py | 1 + common/djangoapps/user_api/tests/factories.py | 1 + common/djangoapps/util/bad_request_rate_limiter.py | 1 + common/djangoapps/util/string_utils.py | 1 + common/djangoapps/util/tests/test_string_utils.py | 1 + common/lib/calc/calc/tests/test_calc.py | 1 + common/lib/capa/capa/registry.py | 1 + common/lib/capa/capa/safe_exec/lazymod.py | 1 + common/lib/capa/capa/tests/test_input_templates.py | 2 ++ common/lib/django_startup.py | 1 + common/lib/symmath/symmath/test_symmath_check.py | 1 + common/lib/tempdir.py | 2 ++ common/lib/xmodule/xmodule/annotator_mixin.py | 1 + common/lib/xmodule/xmodule/course_module.py | 2 ++ common/lib/xmodule/xmodule/lti_module.py | 1 + common/lib/xmodule/xmodule/modulestore/split_mongo/split.py | 1 + .../xmodule/xmodule/modulestore/tests/persistent_factories.py | 1 + .../xmodule/modulestore/tests/test_modulestore_settings.py | 1 + common/lib/xmodule/xmodule/modulestore/xml_importer.py | 1 + .../open_ended_grading_classes/controller_query_service.py | 1 + common/lib/xmodule/xmodule/split_test_module.py | 1 + common/lib/xmodule/xmodule/tabs.py | 1 + common/lib/xmodule/xmodule/tests/rendering/core.py | 1 + common/lib/xmodule/xmodule/tests/test_bulk_assertions.py | 1 + common/lib/xmodule/xmodule/tests/test_capa_module.py | 1 + common/lib/xmodule/xmodule/tests/test_content.py | 1 + common/lib/xmodule/xmodule/tests/test_utils_django.py | 1 + common/lib/xmodule/xmodule/timeinfo.py | 1 + common/lib/xmodule/xmodule/video_module/transcripts_utils.py | 3 +++ common/test/acceptance/fixtures/discussion.py | 1 + common/test/acceptance/pages/studio/overview.py | 2 ++ common/test/acceptance/performance/test_studio_performance.py | 1 + common/test/data/uploads/python_lib_zip/number_helpers.py | 1 + docs/en_us/developers/source/conf.py | 1 + docs/en_us/platform_api/source/conf.py | 1 + docs/shared/conf.py | 1 + lms/djangoapps/courseware/courses.py | 1 + lms/djangoapps/courseware/features/events.py | 1 + lms/djangoapps/courseware/features/gst.py | 1 + lms/djangoapps/courseware/grades.py | 1 + lms/djangoapps/courseware/middleware.py | 1 + lms/djangoapps/courseware/module_render.py | 1 + lms/djangoapps/courseware/tests/test_access.py | 1 + lms/djangoapps/courseware/tests/test_microsites.py | 1 + lms/djangoapps/courseware/views.py | 1 + lms/djangoapps/dashboard/git_import.py | 1 + .../dashboard/management/commands/git_add_course.py | 1 + lms/djangoapps/django_comment_client/forum/tests.py | 1 + lms/djangoapps/django_comment_client/tests/group_id.py | 1 + lms/djangoapps/django_comment_client/tests/unicode.py | 1 + lms/djangoapps/django_comment_client/tests/utils.py | 1 + .../instructor/management/commands/openended_post.py | 1 + lms/djangoapps/instructor/offline_gradecalc.py | 1 + lms/djangoapps/instructor/views/legacy.py | 1 + lms/djangoapps/instructor_task/admin.py | 1 + lms/djangoapps/notification_prefs/views.py | 1 + lms/djangoapps/open_ended_grading/staff_grading_service.py | 1 + lms/djangoapps/shoppingcart/exceptions.py | 1 + lms/djangoapps/shoppingcart/processors/exceptions.py | 1 + lms/djangoapps/shoppingcart/tests/test_models.py | 1 + lms/djangoapps/shoppingcart/views.py | 1 + lms/djangoapps/staticbook/views.py | 1 + lms/djangoapps/verify_student/tests/test_ssencrypt.py | 2 ++ manage.py | 1 + pavelib/acceptance_test.py | 1 + pavelib/i18n.py | 1 + pavelib/paver_tests/test_paver_quality.py | 1 + pavelib/quality.py | 4 ++++ pavelib/utils/envs.py | 1 + pavelib/utils/test/utils.py | 1 + scripts/run_watch_data.py | 1 + scripts/runone.py | 2 ++ 102 files changed, 120 insertions(+) diff --git a/cms/djangoapps/contentstore/management/commands/delete_course.py b/cms/djangoapps/contentstore/management/commands/delete_course.py index 6f77c8a048..2ae3318a72 100644 --- a/cms/djangoapps/contentstore/management/commands/delete_course.py +++ b/cms/djangoapps/contentstore/management/commands/delete_course.py @@ -9,6 +9,7 @@ from opaque_keys import InvalidKeyError from opaque_keys.edx.locations import SlashSeparatedCourseKey from xmodule.modulestore import ModuleStoreEnum + class Command(BaseCommand): help = '''Delete a MongoDB backed course''' diff --git a/cms/djangoapps/contentstore/management/commands/populate_creators.py b/cms/djangoapps/contentstore/management/commands/populate_creators.py index 900d1960cc..f56b3a635e 100644 --- a/cms/djangoapps/contentstore/management/commands/populate_creators.py +++ b/cms/djangoapps/contentstore/management/commands/populate_creators.py @@ -11,6 +11,8 @@ from django.db.utils import IntegrityError from student.roles import CourseInstructorRole, CourseStaffRole #------------ to run: ./manage.py cms populate_creators --settings=dev + + class Command(BaseCommand): """ Script for granting existing course instructors course creator privileges. diff --git a/cms/djangoapps/contentstore/tests/test_transcripts_utils.py b/cms/djangoapps/contentstore/tests/test_transcripts_utils.py index de7762d108..858f07a8b3 100644 --- a/cms/djangoapps/contentstore/tests/test_transcripts_utils.py +++ b/cms/djangoapps/contentstore/tests/test_transcripts_utils.py @@ -472,6 +472,7 @@ class TestYoutubeTranscripts(unittest.TestCase): self.assertEqual(transcripts, expected_transcripts) mock_get.assert_called_with('http://video.google.com/timedtext', params={'lang': 'en', 'v': 'good_youtube_id'}) + class TestTranscript(unittest.TestCase): """ Tests for Transcript class e.g. different transcript conversions. diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 5d3e3e2c18..8698feb6d9 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -86,6 +86,7 @@ class LMSLinksTestCase(TestCase): link = utils.get_lms_link_for_item(location) self.assertEquals(link, "//localhost:8000/courses/mitX/101/test/jump_to/i4x://mitX/101/course/test") + class ExtraPanelTabTestCase(TestCase): """ Tests adding and removing extra course tabs. """ diff --git a/cms/djangoapps/contentstore/views/checklist.py b/cms/djangoapps/contentstore/views/checklist.py index 1b12e93aa3..0d58ee8a36 100644 --- a/cms/djangoapps/contentstore/views/checklist.py +++ b/cms/djangoapps/contentstore/views/checklist.py @@ -124,6 +124,7 @@ def expand_checklist_action_url(course_module, checklist): return expanded_checklist + def localize_checklist_text(checklist): """ Localize texts for a given checklist and returns the modified version. diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 77f6c95b58..5d5529aaf3 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -261,6 +261,7 @@ def course_rerun_handler(request, course_key_string): 'allow_unicode_course_id': settings.FEATURES.get('ALLOW_UNICODE_COURSE_ID', False) }) + def _course_outline_json(request, course_module): """ Returns a JSON representation of the course module and recursively all of its children. diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py index e251e3dca8..bc4636a030 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/cms/djangoapps/contentstore/views/tabs.py @@ -20,6 +20,7 @@ from ..utils import get_lms_link_for_item __all__ = ['tabs_handler'] + @expect_json @login_required @ensure_csrf_cookie diff --git a/cms/djangoapps/models/settings/course_details.py b/cms/djangoapps/models/settings/course_details.py index 6c61cd8419..e8080846a8 100644 --- a/cms/djangoapps/models/settings/course_details.py +++ b/cms/djangoapps/models/settings/course_details.py @@ -11,6 +11,7 @@ from models.settings import course_grading from xmodule.fields import Date from xmodule.modulestore.django import modulestore + class CourseDetails(object): def __init__(self, org, course_id, run): # still need these for now b/c the client's screen shows these 3 fields diff --git a/common/djangoapps/contentserver/middleware.py b/common/djangoapps/contentserver/middleware.py index 4ece37e656..d84e48ad1a 100644 --- a/common/djangoapps/contentserver/middleware.py +++ b/common/djangoapps/contentserver/middleware.py @@ -22,6 +22,7 @@ from xmodule.exceptions import NotFoundError log = logging.getLogger(__name__) + class StaticContentServer(object): def process_request(self, request): # look to see if the request is prefixed with an asset prefix tag diff --git a/common/djangoapps/course_groups/cohorts.py b/common/djangoapps/course_groups/cohorts.py index f691ef5191..37c0abf959 100644 --- a/common/djangoapps/course_groups/cohorts.py +++ b/common/djangoapps/course_groups/cohorts.py @@ -322,6 +322,7 @@ def add_cohort(course_key, name): ) return cohort + def add_user_to_cohort(cohort, username_or_email): """ Look up the given user, and if successful, add them to the specified cohort. diff --git a/common/djangoapps/course_groups/tests/test_views.py b/common/djangoapps/course_groups/tests/test_views.py index 819190237f..78fe1b623b 100644 --- a/common/djangoapps/course_groups/tests/test_views.py +++ b/common/djangoapps/course_groups/tests/test_views.py @@ -209,6 +209,7 @@ class ListCohortsTestCase(CohortViewsTestCase): actual_cohorts, ) + class AddCohortTestCase(CohortViewsTestCase): """ Tests the `add_cohort` view. diff --git a/common/djangoapps/django_comment_common/tests.py b/common/djangoapps/django_comment_common/tests.py index 158f8836c9..a57aa1f4df 100644 --- a/common/djangoapps/django_comment_common/tests.py +++ b/common/djangoapps/django_comment_common/tests.py @@ -4,6 +4,7 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey from django_comment_common.models import Role from student.models import CourseEnrollment, User + class RoleAssignmentTest(TestCase): """ Basic checks to make sure our Roles get assigned and unassigned as students diff --git a/common/djangoapps/edxmako/paths.py b/common/djangoapps/edxmako/paths.py index 518f8a530b..d14773954d 100644 --- a/common/djangoapps/edxmako/paths.py +++ b/common/djangoapps/edxmako/paths.py @@ -32,6 +32,7 @@ def clear_lookups(namespace): if namespace in LOOKUP: del LOOKUP[namespace] + def add_lookup(namespace, directory, package=None, prepend=False): """ Adds a new mako template lookup directory to the given namespace. diff --git a/common/djangoapps/edxmako/shortcuts.py b/common/djangoapps/edxmako/shortcuts.py index c17337c9c3..1e2566e184 100644 --- a/common/djangoapps/edxmako/shortcuts.py +++ b/common/djangoapps/edxmako/shortcuts.py @@ -76,6 +76,7 @@ def marketing_link_context_processor(request): ] ) + def open_source_footer_context_processor(request): """ Checks the site name to determine whether to use the edX.org footer or the Open Source Footer. @@ -97,6 +98,7 @@ def microsite_footer_context_processor(request): ] ) + def render_to_string(template_name, dictionary, context=None, namespace='main'): # see if there is an override template defined in the microsite diff --git a/common/djangoapps/edxmako/tests.py b/common/djangoapps/edxmako/tests.py index 40da9b63b8..318af76cb8 100644 --- a/common/djangoapps/edxmako/tests.py +++ b/common/djangoapps/edxmako/tests.py @@ -19,6 +19,7 @@ from edxmako.shortcuts import ( from student.tests.factories import UserFactory from util.testing import UrlResetMixin + @ddt.ddt class ShortcutsTests(UrlResetMixin, TestCase): """ diff --git a/common/djangoapps/embargo/tests/test_middleware.py b/common/djangoapps/embargo/tests/test_middleware.py index 48fe895f9c..a02f4d31b8 100644 --- a/common/djangoapps/embargo/tests/test_middleware.py +++ b/common/djangoapps/embargo/tests/test_middleware.py @@ -28,6 +28,7 @@ from embargo.models import EmbargoedCourse, EmbargoedState, IPFilter # that disables the XML modulestore. MODULESTORE_CONFIG = mixed_store_config(settings.COMMON_TEST_DATA_ROOT, {}, include_xml=False) + @ddt.ddt @override_settings(MODULESTORE=MODULESTORE_CONFIG) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') diff --git a/common/djangoapps/external_auth/tests/test_ssl.py b/common/djangoapps/external_auth/tests/test_ssl.py index 2ffcee8d51..a81168d98e 100644 --- a/common/djangoapps/external_auth/tests/test_ssl.py +++ b/common/djangoapps/external_auth/tests/test_ssl.py @@ -37,6 +37,7 @@ FEATURES_WITHOUT_SSL_AUTH['AUTH_USE_CERTIFICATES'] = False TEST_DATA_MIXED_MODULESTORE = mixed_store_config(settings.COMMON_TEST_DATA_ROOT, {}) + @override_settings(FEATURES=FEATURES_WITH_SSL_AUTH) class SSLClientTest(ModuleStoreTestCase): """ diff --git a/common/djangoapps/heartbeat/tests/test_heartbeat.py b/common/djangoapps/heartbeat/tests/test_heartbeat.py index 5ac580fb2c..7addee256f 100644 --- a/common/djangoapps/heartbeat/tests/test_heartbeat.py +++ b/common/djangoapps/heartbeat/tests/test_heartbeat.py @@ -8,6 +8,7 @@ from django.db.utils import DatabaseError import mock from django.test.testcases import TestCase + class HeartbeatTestCase(TestCase): """ Test the heartbeat diff --git a/common/djangoapps/microsite_configuration/templatetags/microsite.py b/common/djangoapps/microsite_configuration/templatetags/microsite.py index 46731ceec2..1aea3c4944 100644 --- a/common/djangoapps/microsite_configuration/templatetags/microsite.py +++ b/common/djangoapps/microsite_configuration/templatetags/microsite.py @@ -23,6 +23,7 @@ def page_title_breadcrumbs(*crumbs, **kwargs): else: return platform_name() + @register.simple_tag(name="page_title_breadcrumbs", takes_context=True) def page_title_breadcrumbs_tag(context, *crumbs): """ diff --git a/common/djangoapps/monitoring/signals.py b/common/djangoapps/monitoring/signals.py index 89cf7cc94b..f636e7ae51 100644 --- a/common/djangoapps/monitoring/signals.py +++ b/common/djangoapps/monitoring/signals.py @@ -78,6 +78,7 @@ def post_save_metrics(sender, **kwargs): tags = _database_tags(action, sender, kwargs) dog_stats_api.increment('edxapp.db.model', tags=tags) + @receiver(post_delete, dispatch_uid='edxapp.monitoring.post_delete_metrics') def post_delete_metrics(sender, **kwargs): """ diff --git a/common/djangoapps/request_cache/middleware.py b/common/djangoapps/request_cache/middleware.py index 6640d1a38a..4b5ce114b0 100644 --- a/common/djangoapps/request_cache/middleware.py +++ b/common/djangoapps/request_cache/middleware.py @@ -3,6 +3,7 @@ import threading _request_cache_threadlocal = threading.local() _request_cache_threadlocal.data = {} + class RequestCache(object): @classmethod def get_request_cache(cls): diff --git a/common/djangoapps/student/middleware.py b/common/djangoapps/student/middleware.py index b4f6ad0686..ccffbb1659 100644 --- a/common/djangoapps/student/middleware.py +++ b/common/djangoapps/student/middleware.py @@ -7,6 +7,7 @@ from django.utils.translation import ugettext as _ from django.conf import settings from student.models import UserStanding + class UserStandingMiddleware(object): """ Checks a user's standing on request. Returns a 403 if the user's diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index dda87a80e5..09967cf346 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -665,15 +665,19 @@ class LoginFailures(models.Model): class CourseEnrollmentException(Exception): pass + class NonExistentCourseError(CourseEnrollmentException): pass + class EnrollmentClosedError(CourseEnrollmentException): pass + class CourseFullError(CourseEnrollmentException): pass + class AlreadyEnrolledError(CourseEnrollmentException): pass diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 8fb38b5d14..5165966acb 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -210,6 +210,7 @@ class CourseFinanceAdminRole(CourseRole): def __init__(self, *args, **kwargs): super(CourseFinanceAdminRole, self).__init__(self.ROLE, *args, **kwargs) + class CourseBetaTesterRole(CourseRole): """A course Beta Tester""" ROLE = 'beta_testers' diff --git a/common/djangoapps/student/tests/test_auto_auth.py b/common/djangoapps/student/tests/test_auto_auth.py index 884729a84a..f6123d89c3 100644 --- a/common/djangoapps/student/tests/test_auto_auth.py +++ b/common/djangoapps/student/tests/test_auto_auth.py @@ -11,6 +11,7 @@ from opaque_keys.edx.locator import CourseLocator from mock import patch import ddt + @ddt.ddt class AutoAuthEnabledTestCase(UrlResetMixin, TestCase): """ diff --git a/common/djangoapps/student/tests/test_microsite.py b/common/djangoapps/student/tests/test_microsite.py index 93fa87f8d4..4574c429a8 100644 --- a/common/djangoapps/student/tests/test_microsite.py +++ b/common/djangoapps/student/tests/test_microsite.py @@ -25,6 +25,7 @@ FAKE_MICROSITE = { ] } + def fake_site_name(name, default=None): # pylint: disable=W0613 """ create a fake microsite site name @@ -34,12 +35,14 @@ def fake_site_name(name, default=None): # pylint: disable=W0613 else: return default + def fake_microsite_get_value(name, default=None): # pylint: disable=W0613 """ create a fake microsite site name """ return FAKE_MICROSITE.get(name, default) + class TestMicrosite(TestCase): """Test for Account Creation from a white labeled Micro-Sites""" def setUp(self): diff --git a/common/djangoapps/student/tests/test_password_policy.py b/common/djangoapps/student/tests/test_password_policy.py index 68f776e775..75ad3ce3ac 100644 --- a/common/djangoapps/student/tests/test_password_policy.py +++ b/common/djangoapps/student/tests/test_password_policy.py @@ -15,6 +15,7 @@ from edxmako.tests import mako_middleware_process_request from external_auth.models import ExternalAuthMap from student.views import create_account + @patch.dict("django.conf.settings.FEATURES", {'ENFORCE_PASSWORD_POLICY': True}) class TestPasswordPolicy(TestCase): """ diff --git a/common/djangoapps/terrain/stubs/comments.py b/common/djangoapps/terrain/stubs/comments.py index af4648fa3f..9d8e247a72 100644 --- a/common/djangoapps/terrain/stubs/comments.py +++ b/common/djangoapps/terrain/stubs/comments.py @@ -6,6 +6,7 @@ import re import urlparse from .http import StubHttpRequestHandler, StubHttpService + class StubCommentsServiceHandler(StubHttpRequestHandler): @property diff --git a/common/djangoapps/terrain/stubs/lti.py b/common/djangoapps/terrain/stubs/lti.py index 3bea875b38..b3dbcbafd5 100644 --- a/common/djangoapps/terrain/stubs/lti.py +++ b/common/djangoapps/terrain/stubs/lti.py @@ -21,6 +21,7 @@ import mock import requests from http import StubHttpRequestHandler, StubHttpService + class StubLtiHandler(StubHttpRequestHandler): """ A handler for LTI POST and GET requests. diff --git a/common/djangoapps/terrain/stubs/tests/test_lti_stub.py b/common/djangoapps/terrain/stubs/tests/test_lti_stub.py index 0ea2e2dcd6..34f2445ac2 100644 --- a/common/djangoapps/terrain/stubs/tests/test_lti_stub.py +++ b/common/djangoapps/terrain/stubs/tests/test_lti_stub.py @@ -7,6 +7,7 @@ import urllib2 import requests from terrain.stubs.lti import StubLtiService + class StubLtiServiceTest(unittest.TestCase): """ A stub of the LTI provider that listens on a local diff --git a/common/djangoapps/terrain/stubs/video_source.py b/common/djangoapps/terrain/stubs/video_source.py index 460b2f7560..fa48834546 100644 --- a/common/djangoapps/terrain/stubs/video_source.py +++ b/common/djangoapps/terrain/stubs/video_source.py @@ -9,6 +9,7 @@ import os from logging import getLogger LOGGER = getLogger(__name__) + class VideoSourceRequestHandler(SimpleHTTPRequestHandler): """ Request handler for serving video sources locally. diff --git a/common/djangoapps/user_api/tests/factories.py b/common/djangoapps/user_api/tests/factories.py index 8d32f17142..9aafc0d661 100644 --- a/common/djangoapps/user_api/tests/factories.py +++ b/common/djangoapps/user_api/tests/factories.py @@ -5,6 +5,7 @@ from student.tests.factories import UserFactory from user_api.models import UserPreference, UserCourseTag from opaque_keys.edx.locations import SlashSeparatedCourseKey + # Factories don't have __init__ methods, and are self documenting # pylint: disable=W0232, C0111 class UserPreferenceFactory(DjangoModelFactory): diff --git a/common/djangoapps/util/bad_request_rate_limiter.py b/common/djangoapps/util/bad_request_rate_limiter.py index a8fb0f9180..381289f639 100644 --- a/common/djangoapps/util/bad_request_rate_limiter.py +++ b/common/djangoapps/util/bad_request_rate_limiter.py @@ -4,6 +4,7 @@ which can be used for rate limiting """ from ratelimitbackend.backends import RateLimitMixin + class BadRequestRateLimiter(RateLimitMixin): """ Use the 3rd party RateLimitMixin to help do rate limiting on the Password Reset flows diff --git a/common/djangoapps/util/string_utils.py b/common/djangoapps/util/string_utils.py index e2d547c5e3..43b3a1dc92 100644 --- a/common/djangoapps/util/string_utils.py +++ b/common/djangoapps/util/string_utils.py @@ -2,6 +2,7 @@ Utilities for string manipulation. """ + def str_to_bool(str): """ Converts "true" (case-insensitive) to the boolean True. diff --git a/common/djangoapps/util/tests/test_string_utils.py b/common/djangoapps/util/tests/test_string_utils.py index b3bdea3411..84cbf1e469 100644 --- a/common/djangoapps/util/tests/test_string_utils.py +++ b/common/djangoapps/util/tests/test_string_utils.py @@ -5,6 +5,7 @@ Tests for string_utils.py from django.test import TestCase from util.string_utils import str_to_bool + class StringUtilsTest(TestCase): """ Tests for str_to_bool. diff --git a/common/lib/calc/calc/tests/test_calc.py b/common/lib/calc/calc/tests/test_calc.py index 9d7f3a0199..eacc34fa1c 100644 --- a/common/lib/calc/calc/tests/test_calc.py +++ b/common/lib/calc/calc/tests/test_calc.py @@ -14,6 +14,7 @@ from pyparsing import ParseException # See http://docs.scipy.org/doc/numpy/reference/generated/numpy.seterr.html numpy.seterr(all='ignore') # Also: 'ignore', 'warn' (default), 'raise' + class EvaluatorTest(unittest.TestCase): """ Run tests for calc.evaluator diff --git a/common/lib/capa/capa/registry.py b/common/lib/capa/capa/registry.py index bcc002c046..fedb029fe2 100644 --- a/common/lib/capa/capa/registry.py +++ b/common/lib/capa/capa/registry.py @@ -1,5 +1,6 @@ """A registry for finding classes based on tags in the class.""" + class TagRegistry(object): """ A registry mapping tags to handlers. diff --git a/common/lib/capa/capa/safe_exec/lazymod.py b/common/lib/capa/capa/safe_exec/lazymod.py index cdd8410f2c..d8d6115ca3 100644 --- a/common/lib/capa/capa/safe_exec/lazymod.py +++ b/common/lib/capa/capa/safe_exec/lazymod.py @@ -7,6 +7,7 @@ in the public domain. import sys + class LazyModule(object): """A lazy module proxy.""" diff --git a/common/lib/capa/capa/tests/test_input_templates.py b/common/lib/capa/capa/tests/test_input_templates.py index ac2268ae7c..2dc90237c6 100644 --- a/common/lib/capa/capa/tests/test_input_templates.py +++ b/common/lib/capa/capa/tests/test_input_templates.py @@ -11,6 +11,7 @@ from mako.template import Template as MakoTemplate from mako import exceptions from capa.inputtypes import Status + class TemplateError(Exception): """ Error occurred while rendering a Mako template. @@ -497,6 +498,7 @@ class FormulaEquationInputTemplateTest(TemplateTestCase): self.assert_has_xpath(xml, "//input[@size='40']", self.context) + class AnnotationInputTemplateTest(TemplateTestCase): """ Test mako template for `` input. diff --git a/common/lib/django_startup.py b/common/lib/django_startup.py index 1980420e0d..19f0f8babe 100644 --- a/common/lib/django_startup.py +++ b/common/lib/django_startup.py @@ -5,6 +5,7 @@ Automatic execution of startup modules in Django apps. from importlib import import_module from django.conf import settings + def autostartup(): """ Execute app.startup:run() for all installed django apps diff --git a/common/lib/symmath/symmath/test_symmath_check.py b/common/lib/symmath/symmath/test_symmath_check.py index 3b8f14b0d2..222cbdd0b6 100644 --- a/common/lib/symmath/symmath/test_symmath_check.py +++ b/common/lib/symmath/symmath/test_symmath_check.py @@ -1,6 +1,7 @@ from unittest import TestCase from .symmath_check import symmath_check + class SymmathCheckTest(TestCase): def test_symmath_check_integers(self): number_list = [i for i in range(-100, 100)] diff --git a/common/lib/tempdir.py b/common/lib/tempdir.py index 0acd92ba33..b3f10b22e0 100644 --- a/common/lib/tempdir.py +++ b/common/lib/tempdir.py @@ -5,12 +5,14 @@ import os.path import shutil import tempfile + def mkdtemp_clean(suffix="", prefix="tmp", dir=None): """Just like mkdtemp, but the directory will be deleted when the process ends.""" the_dir = tempfile.mkdtemp(suffix=suffix, prefix=prefix, dir=dir) atexit.register(cleanup_tempdir, the_dir) return the_dir + def cleanup_tempdir(the_dir): """Called on process exit to remove a temp directory.""" if os.path.exists(the_dir): diff --git a/common/lib/xmodule/xmodule/annotator_mixin.py b/common/lib/xmodule/xmodule/annotator_mixin.py index aa0a19d4a8..aa597702db 100644 --- a/common/lib/xmodule/xmodule/annotator_mixin.py +++ b/common/lib/xmodule/xmodule/annotator_mixin.py @@ -7,6 +7,7 @@ from urlparse import urlparse from os.path import splitext, basename from HTMLParser import HTMLParser + def get_instructions(xmltree): """ Removes from the xmltree and returns them as a string, otherwise None. """ instructions = xmltree.find('instructions') diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 3a829d37ca..54f9d0dfa1 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -28,6 +28,7 @@ CATALOG_VISIBILITY_CATALOG_AND_ABOUT = "both" CATALOG_VISIBILITY_ABOUT = "about" CATALOG_VISIBILITY_NONE = "none" + class StringOrDate(Date): def from_json(self, value): """ @@ -589,6 +590,7 @@ class CourseFields(object): {"display_name": _("None"), "value": CATALOG_VISIBILITY_NONE}] ) + class CourseDescriptor(CourseFields, SequenceDescriptor): module_class = SequenceModule diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py index 1da01cd9d4..d929e91e86 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/common/lib/xmodule/xmodule/lti_module.py @@ -842,6 +842,7 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'} return key, secret return '', '' + class LTIDescriptor(LTIFields, MetadataOnlyEditingDescriptor, EmptyDataRawDescriptor): """ Descriptor for LTI Xmodule. diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 8b94851480..8756a39842 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -2690,6 +2690,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): """ self.db_connection.ensure_indexes() + class SparseList(list): """ Enable inserting items into a list in arbitrary order and then retrieving them. diff --git a/common/lib/xmodule/xmodule/modulestore/tests/persistent_factories.py b/common/lib/xmodule/xmodule/modulestore/tests/persistent_factories.py index bf00f73cf5..a292212a1a 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/persistent_factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/persistent_factories.py @@ -8,6 +8,7 @@ from opaque_keys.edx.keys import UsageKey # Factories don't have __init__ methods, and are self documenting # pylint: disable=W0232, C0111 + class SplitFactory(factory.Factory): """ Abstracted superclass which defines modulestore so that there's no dependency on django diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py b/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py index 3889530d25..56cf3bd46e 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py @@ -12,6 +12,7 @@ from xmodule.modulestore.modulestore_settings import ( get_mixed_stores, ) + @ddt.ddt class ModuleStoreSettingsMigration(TestCase): """ diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 1af5ea27b4..92a0bb28bd 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -390,6 +390,7 @@ def _import_static_content_wrapper(static_content_store, do_import_static, cours dest_course_id, subpath=simport, verbose=verbose ) + def _import_module_and_update_references( module, store, user_id, source_course_id, dest_course_id, diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py index bd5c4d592a..b20c0279bc 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py @@ -168,6 +168,7 @@ class MockControllerQueryService(object): """ pass + def convert_seconds_to_human_readable(seconds): if seconds < 60: human_string = "{0} seconds".format(seconds) diff --git a/common/lib/xmodule/xmodule/split_test_module.py b/common/lib/xmodule/xmodule/split_test_module.py index 890afb75dc..fdd4629a9a 100644 --- a/common/lib/xmodule/xmodule/split_test_module.py +++ b/common/lib/xmodule/xmodule/split_test_module.py @@ -357,6 +357,7 @@ class SplitTestModule(SplitTestFields, XModule, StudioEditableModule): """ return self.descriptor.validate() + @XBlock.needs('user_tags') # pylint: disable=abstract-method @XBlock.wants('partitions') @XBlock.wants('user') diff --git a/common/lib/xmodule/xmodule/tabs.py b/common/lib/xmodule/xmodule/tabs.py index 64dfe0ee30..5ad8f58c48 100644 --- a/common/lib/xmodule/xmodule/tabs.py +++ b/common/lib/xmodule/xmodule/tabs.py @@ -225,6 +225,7 @@ class StaffTab(AuthenticatedCourseTab): def can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): # pylint: disable=unused-argument return is_user_staff + class EnrolledOrStaffTab(CourseTab): """ Abstract class for tabs that can be accessed by only users with staff access diff --git a/common/lib/xmodule/xmodule/tests/rendering/core.py b/common/lib/xmodule/xmodule/tests/rendering/core.py index 1324af853c..acae995397 100644 --- a/common/lib/xmodule/xmodule/tests/rendering/core.py +++ b/common/lib/xmodule/xmodule/tests/rendering/core.py @@ -40,6 +40,7 @@ import lxml.etree from singledispatch import singledispatch + @singledispatch def assert_student_view_valid_html(block, html): """ diff --git a/common/lib/xmodule/xmodule/tests/test_bulk_assertions.py b/common/lib/xmodule/xmodule/tests/test_bulk_assertions.py index d796b6b546..a6a2860a4e 100644 --- a/common/lib/xmodule/xmodule/tests/test_bulk_assertions.py +++ b/common/lib/xmodule/xmodule/tests/test_bulk_assertions.py @@ -1,6 +1,7 @@ import ddt from xmodule.tests import BulkAssertionTest + @ddt.ddt class TestBulkAssertionTestCase(BulkAssertionTest): diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 5208b17382..06ea77d152 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -142,6 +142,7 @@ class CapaFactory(object): return module + class CapaFactoryWithFiles(CapaFactory): """ A factory for creating a Capa problem with files attached. diff --git a/common/lib/xmodule/xmodule/tests/test_content.py b/common/lib/xmodule/xmodule/tests/test_content.py index 457a4a22b2..5c9c50c4be 100644 --- a/common/lib/xmodule/xmodule/tests/test_content.py +++ b/common/lib/xmodule/xmodule/tests/test_content.py @@ -44,6 +44,7 @@ Various versions have evolved over the years, sometimes by accident, sometimes o injected humour and the like). """ + class Content: def __init__(self, location, content_type): self.location = location diff --git a/common/lib/xmodule/xmodule/tests/test_utils_django.py b/common/lib/xmodule/xmodule/tests/test_utils_django.py index 36a3b0c272..dfa86a23ea 100644 --- a/common/lib/xmodule/xmodule/tests/test_utils_django.py +++ b/common/lib/xmodule/xmodule/tests/test_utils_django.py @@ -3,6 +3,7 @@ from xmodule.util.django import get_current_request, get_current_request_hostnam from nose.tools import assert_is_none from unittest import TestCase + class UtilDjangoTests(TestCase): """ Tests for methods exposed in util/django diff --git a/common/lib/xmodule/xmodule/timeinfo.py b/common/lib/xmodule/xmodule/timeinfo.py index a55cbe3a63..17ac38de52 100644 --- a/common/lib/xmodule/xmodule/timeinfo.py +++ b/common/lib/xmodule/xmodule/timeinfo.py @@ -2,6 +2,7 @@ import logging from xmodule.fields import Timedelta log = logging.getLogger(__name__) + class TimeInfo(object): """ This is a simple object that calculates and stores datetime information for an XModule diff --git a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py index a6b1da42a3..857cc6b5d2 100644 --- a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py +++ b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py @@ -75,6 +75,7 @@ def save_to_store(content, name, mime_type, location): contentstore().save(content) return content_location + def save_subs_to_store(subs, subs_id, item, language='en'): """ Save transcripts into `StaticContent`. @@ -90,6 +91,7 @@ def save_subs_to_store(subs, subs_id, item, language='en'): filename = subs_filename(subs_id, language) return save_to_store(filedata, filename, 'application/json', item.location) + def get_transcripts_from_youtube(youtube_id, settings, i18n): """ Gets transcripts from youtube for youtube_id. @@ -428,6 +430,7 @@ def get_or_create_sjson(item): sjson_transcript = Transcript.asset(item.location, source_subs_id, item.transcript_language).data return sjson_transcript + class Transcript(object): """ Container for transcript methods. diff --git a/common/test/acceptance/fixtures/discussion.py b/common/test/acceptance/fixtures/discussion.py index a521a36c8e..c14b2cf56e 100644 --- a/common/test/acceptance/fixtures/discussion.py +++ b/common/test/acceptance/fixtures/discussion.py @@ -42,6 +42,7 @@ class Thread(ContentFactory): pinned = False read = False + class Comment(ContentFactory): thread_id = None depth = 0 diff --git a/common/test/acceptance/pages/studio/overview.py b/common/test/acceptance/pages/studio/overview.py index 50d25ba959..21be0b833b 100644 --- a/common/test/acceptance/pages/studio/overview.py +++ b/common/test/acceptance/pages/studio/overview.py @@ -307,6 +307,7 @@ class CourseOutlineChild(PageObject, CourseOutlineItem): grand_locators = [grandkid.locator for grandkid in grandkids] return [descendant for descendant in descendants if not descendant.locator in grand_locators] + class CourseOutlineUnit(CourseOutlineChild): """ PageObject that wraps a unit link on the Studio Course Outline page. @@ -329,6 +330,7 @@ class CourseOutlineUnit(CourseOutlineChild): return self.q(css=self._bounded_selector(self.BODY_SELECTOR)).map( lambda el: CourseOutlineUnit(self.browser, el.get_attribute('data-locator'))).results + class CourseOutlineSubsection(CourseOutlineContainer, CourseOutlineChild): """ :class`.PageObject` that wraps a subsection block on the Studio Course Outline page. diff --git a/common/test/acceptance/performance/test_studio_performance.py b/common/test/acceptance/performance/test_studio_performance.py index 743c51902a..4886e03b45 100644 --- a/common/test/acceptance/performance/test_studio_performance.py +++ b/common/test/acceptance/performance/test_studio_performance.py @@ -6,6 +6,7 @@ from ..pages.studio.auto_auth import AutoAuthPage from ..pages.studio.overview import CourseOutlinePage from nose.plugins.attrib import attr + @attr(har_mode='explicit') class StudioPagePerformanceTest(WebAppTest): """ diff --git a/common/test/data/uploads/python_lib_zip/number_helpers.py b/common/test/data/uploads/python_lib_zip/number_helpers.py index a79b13b529..04db8e9bfe 100644 --- a/common/test/data/uploads/python_lib_zip/number_helpers.py +++ b/common/test/data/uploads/python_lib_zip/number_helpers.py @@ -1,5 +1,6 @@ def seventeen(): return 17 + def fortytwo(x): return 42+x diff --git a/docs/en_us/developers/source/conf.py b/docs/en_us/developers/source/conf.py index 41576c141c..a3b9d6a01a 100644 --- a/docs/en_us/developers/source/conf.py +++ b/docs/en_us/developers/source/conf.py @@ -83,6 +83,7 @@ copyright = u'2014, edX' # Mock all the modules that the readthedocs build can't import + class Mock(object): def __init__(self, *args, **kwargs): pass diff --git a/docs/en_us/platform_api/source/conf.py b/docs/en_us/platform_api/source/conf.py index 0cacceef97..b0fcd45019 100644 --- a/docs/en_us/platform_api/source/conf.py +++ b/docs/en_us/platform_api/source/conf.py @@ -74,6 +74,7 @@ copyright = u'2014, edX' # Mock all the modules that the readthedocs build can't import + class Mock(object): def __init__(self, *args, **kwargs): pass diff --git a/docs/shared/conf.py b/docs/shared/conf.py index 91ce9ef1da..1659291e99 100644 --- a/docs/shared/conf.py +++ b/docs/shared/conf.py @@ -25,6 +25,7 @@ import sys, os BASEDIR = os.path.dirname(os.path.abspath(__file__)) + def add_base(paths): """ Returns a list of paths relative to BASEDIR. diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 33b8548067..616040d13e 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -259,6 +259,7 @@ def get_course_info_section_module(request, course, section_key): static_asset_path=course.static_asset_path ) + def get_course_info_section(request, course, section_key): """ This returns the snippet of html to be rendered on the course info page, diff --git a/lms/djangoapps/courseware/features/events.py b/lms/djangoapps/courseware/features/events.py index b4b1d5001a..4f0d9adccf 100644 --- a/lms/djangoapps/courseware/features/events.py +++ b/lms/djangoapps/courseware/features/events.py @@ -41,6 +41,7 @@ def course_url_event_is_emitted(_step, url_regex): event_type = url_regex.format(world.scenario_dict['COURSE'].id) n_events_are_emitted(_step, 1, event_type, "server") + @step(r'([aA]n?|\d+) "(.*)" (server|browser) events? is emitted$') def n_events_are_emitted(_step, count, event_type, event_source): diff --git a/lms/djangoapps/courseware/features/gst.py b/lms/djangoapps/courseware/features/gst.py index bbb0c76f6a..3ce668e3f1 100644 --- a/lms/djangoapps/courseware/features/gst.py +++ b/lms/djangoapps/courseware/features/gst.py @@ -26,6 +26,7 @@ DEFAULT_DATA = """\ """ + @steps class GraphicalSliderToolSteps(object): COURSE_NUM = 'test_course' diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index 61781dbc78..48e30837ed 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -140,6 +140,7 @@ def answer_distributions(course_key): return answer_counts + @transaction.commit_manually def grade(student, request, course, keep_raw_scores=False): """ diff --git a/lms/djangoapps/courseware/middleware.py b/lms/djangoapps/courseware/middleware.py index 7ef0844db5..60b803601a 100644 --- a/lms/djangoapps/courseware/middleware.py +++ b/lms/djangoapps/courseware/middleware.py @@ -7,6 +7,7 @@ from django.core.urlresolvers import reverse from courseware.courses import UserNotEnrolled + class RedirectUnenrolledMiddleware(object): """ Catch UserNotEnrolled errors thrown by `get_course_with_access` and redirect diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index c948cd3d3d..98d44fe9df 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -72,6 +72,7 @@ XQUEUE_INTERFACE = XQueueInterface( # Some brave person should make the variable names consistently someday, but the code's # coupled enough that it's kind of tricky--you've been warned! + class LmsModuleRenderError(Exception): """ An exception class for exceptions thrown by module_render that don't fit well elsewhere diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index 928fcb6c26..c2f6ea2b81 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -18,6 +18,7 @@ from xmodule.course_module import ( # pylint: disable=C0111 # pylint: disable=W0212 + class AccessTestCase(TestCase): """ Tests for the various access controls on the student dashboard diff --git a/lms/djangoapps/courseware/tests/test_microsites.py b/lms/djangoapps/courseware/tests/test_microsites.py index 2f34de8e14..95c68f8689 100644 --- a/lms/djangoapps/courseware/tests/test_microsites.py +++ b/lms/djangoapps/courseware/tests/test_microsites.py @@ -14,6 +14,7 @@ from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE from xmodule.course_module import ( CATALOG_VISIBILITY_CATALOG_AND_ABOUT, CATALOG_VISIBILITY_NONE) + @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) class TestMicrosites(ModuleStoreTestCase, LoginEnrollmentTestCase): """ diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 6cb05bd0a9..5887aa6489 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -838,6 +838,7 @@ def mktg_course_about(request, course_id): if force_english: translation.deactivate() + @login_required @cache_control(no_cache=True, no_store=True, must_revalidate=True) @transaction.commit_manually diff --git a/lms/djangoapps/dashboard/git_import.py b/lms/djangoapps/dashboard/git_import.py index 1fcf322ec1..0e4b27bb23 100644 --- a/lms/djangoapps/dashboard/git_import.py +++ b/lms/djangoapps/dashboard/git_import.py @@ -51,6 +51,7 @@ class GitImportError(Exception): CANNOT_BRANCH = _('Unable to switch to specified branch. Please check ' 'your branch name.') + def cmd_log(cmd, cwd): """ Helper function to redirect stderr to stdout and log the command diff --git a/lms/djangoapps/dashboard/management/commands/git_add_course.py b/lms/djangoapps/dashboard/management/commands/git_add_course.py index da02a438a6..d97a04b881 100644 --- a/lms/djangoapps/dashboard/management/commands/git_add_course.py +++ b/lms/djangoapps/dashboard/management/commands/git_add_course.py @@ -14,6 +14,7 @@ from xmodule.modulestore.xml import XMLModuleStore log = logging.getLogger(__name__) + class Command(BaseCommand): """ Pull a git repo and import into the mongo based content database. diff --git a/lms/djangoapps/django_comment_client/forum/tests.py b/lms/djangoapps/django_comment_client/forum/tests.py index 7855271be0..2bec1572ed 100644 --- a/lms/djangoapps/django_comment_client/forum/tests.py +++ b/lms/djangoapps/django_comment_client/forum/tests.py @@ -569,6 +569,7 @@ class ForumFormDiscussionGroupIdTestCase(CohortedContentTestCase, CohortedTopicG response, lambda d: d['discussion_data'][0] ) + @patch('lms.lib.comment_client.utils.requests.request') class UserProfileDiscussionGroupIdTestCase(CohortedContentTestCase, CohortedTopicGroupIdTestMixin): cs_endpoint = "/active_threads" diff --git a/lms/djangoapps/django_comment_client/tests/group_id.py b/lms/djangoapps/django_comment_client/tests/group_id.py index 23f965c771..943a257ef4 100644 --- a/lms/djangoapps/django_comment_client/tests/group_id.py +++ b/lms/djangoapps/django_comment_client/tests/group_id.py @@ -3,6 +3,7 @@ import re from course_groups.models import CourseUserGroup + class GroupIdAssertionMixin(object): def _data_or_params_cs_request(self, mock_request): """ diff --git a/lms/djangoapps/django_comment_client/tests/unicode.py b/lms/djangoapps/django_comment_client/tests/unicode.py index 4240d2636e..700d31be15 100644 --- a/lms/djangoapps/django_comment_client/tests/unicode.py +++ b/lms/djangoapps/django_comment_client/tests/unicode.py @@ -1,5 +1,6 @@ # coding=utf-8 + class UnicodeTestMixin(object): def test_ascii(self): self._test_unicode_data(u"This post contains ASCII.") diff --git a/lms/djangoapps/django_comment_client/tests/utils.py b/lms/djangoapps/django_comment_client/tests/utils.py index 2c1cc371ec..38b4c48e6d 100644 --- a/lms/djangoapps/django_comment_client/tests/utils.py +++ b/lms/djangoapps/django_comment_client/tests/utils.py @@ -9,6 +9,7 @@ from student.tests.factories import CourseEnrollmentFactory, UserFactory from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase + @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) class CohortedContentTestCase(ModuleStoreTestCase): """ diff --git a/lms/djangoapps/instructor/management/commands/openended_post.py b/lms/djangoapps/instructor/management/commands/openended_post.py index 8f7992c358..79018de7a5 100644 --- a/lms/djangoapps/instructor/management/commands/openended_post.py +++ b/lms/djangoapps/instructor/management/commands/openended_post.py @@ -112,6 +112,7 @@ def post_submission_for_student(student, course, location, task_number, dry_run= return False + class DummyRequest(object): """Dummy request""" diff --git a/lms/djangoapps/instructor/offline_gradecalc.py b/lms/djangoapps/instructor/offline_gradecalc.py index 9c77bc276e..83bb5f72d8 100644 --- a/lms/djangoapps/instructor/offline_gradecalc.py +++ b/lms/djangoapps/instructor/offline_gradecalc.py @@ -16,6 +16,7 @@ from django.contrib.auth.models import User from instructor.utils import DummyRequest + class MyEncoder(JSONEncoder): def _iterencode(self, obj, markers=None): diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index 728b04777a..c3e2a65fbd 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -1431,6 +1431,7 @@ def get_student_grade_summary_data(request, course, get_grades=True, get_raw_sco #----------------------------------------------------------------------------- # enrollment + def _do_enroll_students(course, course_key, students, secure=False, overload=False, auto_enroll=False, email_students=False, is_shib_course=False): """ Do the actual work of enrolling multiple students, presented as a string diff --git a/lms/djangoapps/instructor_task/admin.py b/lms/djangoapps/instructor_task/admin.py index f453c6dbb5..2d70a065b6 100644 --- a/lms/djangoapps/instructor_task/admin.py +++ b/lms/djangoapps/instructor_task/admin.py @@ -8,6 +8,7 @@ a task. from django.contrib import admin from .models import InstructorTask + class InstructorTaskAdmin(admin.ModelAdmin): list_display = [ 'task_id', diff --git a/lms/djangoapps/notification_prefs/views.py b/lms/djangoapps/notification_prefs/views.py index e1d64cb84f..6bcb60a489 100644 --- a/lms/djangoapps/notification_prefs/views.py +++ b/lms/djangoapps/notification_prefs/views.py @@ -132,6 +132,7 @@ def ajax_disable(request): return HttpResponse(status=204) + @require_GET def ajax_status(request): """ diff --git a/lms/djangoapps/open_ended_grading/staff_grading_service.py b/lms/djangoapps/open_ended_grading/staff_grading_service.py index c6042d3f51..d67376591b 100644 --- a/lms/djangoapps/open_ended_grading/staff_grading_service.py +++ b/lms/djangoapps/open_ended_grading/staff_grading_service.py @@ -278,6 +278,7 @@ def get_next(request, course_id): return HttpResponse(json.dumps(_get_next(course_key, grader_id, location)), mimetype="application/json") + def get_problem_list(request, course_id): """ Get all the problems for the given course id diff --git a/lms/djangoapps/shoppingcart/exceptions.py b/lms/djangoapps/shoppingcart/exceptions.py index 1a1f944e71..6253c3bc46 100644 --- a/lms/djangoapps/shoppingcart/exceptions.py +++ b/lms/djangoapps/shoppingcart/exceptions.py @@ -4,6 +4,7 @@ Exceptions for the shoppingcart app # (Exception Class Names are sort of self-explanatory, so skipping docstring requirement) # pylint: disable=C0111 + class PaymentException(Exception): pass diff --git a/lms/djangoapps/shoppingcart/processors/exceptions.py b/lms/djangoapps/shoppingcart/processors/exceptions.py index 2bcf35e50f..47a51a8b60 100644 --- a/lms/djangoapps/shoppingcart/processors/exceptions.py +++ b/lms/djangoapps/shoppingcart/processors/exceptions.py @@ -16,5 +16,6 @@ class CCProcessorDataException(CCProcessorException): class CCProcessorWrongAmountException(CCProcessorException): pass + class CCProcessorUserCancelled(CCProcessorException): pass diff --git a/lms/djangoapps/shoppingcart/tests/test_models.py b/lms/djangoapps/shoppingcart/tests/test_models.py index d2d992b89f..e1624ee730 100644 --- a/lms/djangoapps/shoppingcart/tests/test_models.py +++ b/lms/djangoapps/shoppingcart/tests/test_models.py @@ -37,6 +37,7 @@ from opaque_keys.edx.locator import CourseLocator # that disables the XML modulestore. MODULESTORE_CONFIG = mixed_store_config(settings.COMMON_TEST_DATA_ROOT, {}, include_xml=False) + @override_settings(MODULESTORE=MODULESTORE_CONFIG) class OrderTest(ModuleStoreTestCase): def setUp(self): diff --git a/lms/djangoapps/shoppingcart/views.py b/lms/djangoapps/shoppingcart/views.py index 53c72219ab..24cfd56fcc 100644 --- a/lms/djangoapps/shoppingcart/views.py +++ b/lms/djangoapps/shoppingcart/views.py @@ -196,6 +196,7 @@ def remove_item(request): return HttpResponse('OK') + def remove_code_redemption(order_item_course_id, item_id, item, user): """ If an item removed from shopping cart then we will remove diff --git a/lms/djangoapps/staticbook/views.py b/lms/djangoapps/staticbook/views.py index eda28d9b7a..4d454f90f8 100644 --- a/lms/djangoapps/staticbook/views.py +++ b/lms/djangoapps/staticbook/views.py @@ -130,6 +130,7 @@ def pdf_index(request, course_id, book_index, chapter=None, page=None): }, ) + @login_required def html_index(request, course_id, book_index, chapter=None): """ diff --git a/lms/djangoapps/verify_student/tests/test_ssencrypt.py b/lms/djangoapps/verify_student/tests/test_ssencrypt.py index 1e9978be7c..c2a6c3646f 100644 --- a/lms/djangoapps/verify_student/tests/test_ssencrypt.py +++ b/lms/djangoapps/verify_student/tests/test_ssencrypt.py @@ -7,6 +7,7 @@ from verify_student.ssencrypt import ( rsa_decrypt, rsa_encrypt, random_aes_key ) + def test_aes(): key_str = "32fe72aaf2abb44de9e161131b5435c8d37cbdb6f5df242ae860b283115f2dae" key = key_str.decode("hex") @@ -28,6 +29,7 @@ def test_aes(): assert_roundtrip("") assert_roundtrip("\xe9\xe1a\x13\x1bT5\xc8") # Random, non-ASCII text + def test_rsa(): # Make up some garbage keys for testing purposes. pub_key_str = """-----BEGIN PUBLIC KEY----- diff --git a/manage.py b/manage.py index c75dbf389e..c14d386e87 100755 --- a/manage.py +++ b/manage.py @@ -21,6 +21,7 @@ import importlib from argparse import ArgumentParser import contracts + def parse_args(): """Parse edx specific arguments to manage.py""" parser = ArgumentParser() diff --git a/pavelib/acceptance_test.py b/pavelib/acceptance_test.py index 1d9ddf2e30..2389af7d3d 100644 --- a/pavelib/acceptance_test.py +++ b/pavelib/acceptance_test.py @@ -12,6 +12,7 @@ except ImportError: __test__ = False # do not collect + @task @needs( 'pavelib.prereqs.install_prereqs', diff --git a/pavelib/i18n.py b/pavelib/i18n.py index aa43d4f0ba..c3dfaf2862 100644 --- a/pavelib/i18n.py +++ b/pavelib/i18n.py @@ -168,6 +168,7 @@ def i18n_ltr(): sh('git add conf/locale') sh('git commit --amend') + @task @needs( "pavelib.i18n.i18n_transifex_pull", diff --git a/pavelib/paver_tests/test_paver_quality.py b/pavelib/paver_tests/test_paver_quality.py index 21682fc72c..018d7e7e68 100644 --- a/pavelib/paver_tests/test_paver_quality.py +++ b/pavelib/paver_tests/test_paver_quality.py @@ -8,6 +8,7 @@ import pavelib.quality import paver.easy from paver.easy import BuildFailure + @ddt class TestPaverQualityViolations(unittest.TestCase): diff --git a/pavelib/quality.py b/pavelib/quality.py index 887b0997a8..3333978087 100644 --- a/pavelib/quality.py +++ b/pavelib/quality.py @@ -65,6 +65,7 @@ def run_pylint(options): raise Exception("Failed. Too many pylint violations. " "The limit is {violations_limit}.".format(violations_limit=violations_limit)) + def _count_pylint_violations(report_file): """ Parses a pylint report line-by-line and determines the number of violations reported @@ -83,6 +84,7 @@ def _count_pylint_violations(report_file): num_violations_report += 1 return num_violations_report + @task @needs('pavelib.prereqs.install_python_prereqs') @cmdopts([ @@ -113,10 +115,12 @@ def run_pep8(options): raise Exception("Failed. Too many pep8 violations. " "The limit is {violations_limit}.".format(violations_limit=violations_limit)) + def _count_pep8_violations(report_file): num_lines = sum(1 for line in open(report_file)) return num_lines + @task @needs('pavelib.prereqs.install_python_prereqs') @cmdopts([ diff --git a/pavelib/utils/envs.py b/pavelib/utils/envs.py index ca4fb12312..b36f8d852d 100644 --- a/pavelib/utils/envs.py +++ b/pavelib/utils/envs.py @@ -9,6 +9,7 @@ from lazy import lazy from path import path import memcache + class Env(object): """ Load information about the execution environment. diff --git a/pavelib/utils/test/utils.py b/pavelib/utils/test/utils.py index 4520afacfb..e92ae7f201 100644 --- a/pavelib/utils/test/utils.py +++ b/pavelib/utils/test/utils.py @@ -54,6 +54,7 @@ def clean_mongo(): repo_root=Env.REPO_ROOT, )) + def check_firefox_version(): """ Check that firefox is the correct version. diff --git a/scripts/run_watch_data.py b/scripts/run_watch_data.py index 7a7b781da6..08481b48de 100755 --- a/scripts/run_watch_data.py +++ b/scripts/run_watch_data.py @@ -22,6 +22,7 @@ EXTENSIONS = ["*", "xml", "js", "css", "coffee", "scss", "html"] WATCH_DIRS = [os.path.abspath(os.path.normpath(dir)) for dir in WATCH_DIRS] + class DjangoEventHandler(FileSystemEventHandler): def __init__(self, process): diff --git a/scripts/runone.py b/scripts/runone.py index e4dcce8dea..ea25c8365c 100755 --- a/scripts/runone.py +++ b/scripts/runone.py @@ -9,6 +9,7 @@ import sys # to become: # test --settings=cms.envs.test --pythonpath=. -s cms/djangoapps/contentstore/tests/test_course_settings.py:CourseDetailsViewTest.test_update_and_fetch + def find_full_path(path_to_file): """Find the full path where we only have a relative path from somewhere in the tree.""" for subdir, dirs, files in os.walk("."): @@ -16,6 +17,7 @@ def find_full_path(path_to_file): if os.path.exists(full): return full + def main(argv): parser = argparse.ArgumentParser(description="Run just one test") parser.add_argument('--nocapture', '-s', action='store_true', help="Don't capture stdout (any stdout output will be printed immediately)") From fb9f324f2fda1ebbf894ecd208cb323c268f8607 Mon Sep 17 00:00:00 2001 From: stv Date: Sat, 8 Nov 2014 17:17:36 -0800 Subject: [PATCH 17/48] Fix PEP8: E303 too many blank lines --- cms/djangoapps/contentstore/features/common.py | 1 - cms/djangoapps/contentstore/tests/test_transcripts_utils.py | 2 -- cms/djangoapps/contentstore/views/user.py | 1 - common/djangoapps/course_modes/tests/test_views.py | 1 - common/djangoapps/dark_lang/tests.py | 1 - common/djangoapps/edxmako/makoloader.py | 1 - common/djangoapps/status/tests.py | 1 - .../student/migrations/0041_add_dashboard_config.py | 2 -- common/djangoapps/terrain/stubs/ora.py | 2 -- common/lib/capa/capa/correctmap.py | 1 - common/lib/capa/capa/customrender.py | 1 - common/lib/capa/capa/inputtypes.py | 1 - common/lib/capa/capa/tests/test_inputtypes.py | 1 - common/lib/capa/capa/tests/test_responsetypes.py | 3 --- common/lib/capa/capa/tests/test_util.py | 1 - common/lib/chem/chem/chemcalc.py | 2 -- common/lib/chem/chem/tests.py | 1 - common/lib/symmath/symmath/formula.py | 1 - common/lib/xmodule/xmodule/exceptions.py | 1 - common/lib/xmodule/xmodule/lti_module.py | 1 - .../lib/xmodule/xmodule/modulestore/split_mongo/__init__.py | 1 - .../xmodule/modulestore/split_mongo/split_mongo_kvs.py | 1 - .../xmodule/modulestore/tests/test_modulestore_settings.py | 1 - common/lib/xmodule/xmodule/modulestore/xml_exporter.py | 1 - common/lib/xmodule/xmodule/randomize_module.py | 2 -- common/lib/xmodule/xmodule/template_module.py | 1 - common/lib/xmodule/xmodule/tests/test_bulk_assertions.py | 1 - common/lib/xmodule/xmodule/tests/test_capa_module.py | 1 - common/lib/xmodule/xmodule/tests/test_import.py | 1 - common/lib/xmodule/xmodule/x_module.py | 3 --- common/test/acceptance/tests/test_annotatable.py | 1 - common/xml_cleanup.py | 1 - docs/en_us/developers/source/conf.py | 2 -- docs/en_us/platform_api/source/conf.py | 1 - docs/shared/conf.py | 1 - ..._optout_course_id__add_field_courseemail_template_name_.py | 4 ---- lms/djangoapps/courseware/tests/test_model_data.py | 1 - .../lms_migration/management/commands/create_user.py | 1 - lms/djangoapps/lms_migration/migrate.py | 1 - lms/djangoapps/notification_prefs/views.py | 1 - .../shoppingcart/migrations/0018_auto__add_donation.py | 2 -- .../migrations/0019_auto__add_donationconfiguration.py | 2 -- ...eregcodeitem__add_courseregcodeitemannotation__add_fiel.py | 2 -- ...d_field_orderitem_created__add_field_orderitem_modified.py | 2 -- .../shoppingcart/processors/tests/test_CyberSource.py | 1 - lms/djangoapps/verify_student/models.py | 2 -- pavelib/paver_tests/test_paver_get_quality_reports.py | 1 - pavelib/utils/test/suites/acceptance_suite.py | 1 - 48 files changed, 66 deletions(-) diff --git a/cms/djangoapps/contentstore/features/common.py b/cms/djangoapps/contentstore/features/common.py index 0ac82a6545..491201bd15 100644 --- a/cms/djangoapps/contentstore/features/common.py +++ b/cms/djangoapps/contentstore/features/common.py @@ -333,7 +333,6 @@ def get_codemirror_value(index=0, find_prefix="$"): ) - def attach_file(filename, sub_path): path = os.path.join(TEST_ROOT, sub_path, filename) world.browser.execute_script("$('input.file-input').css('display', 'block')") diff --git a/cms/djangoapps/contentstore/tests/test_transcripts_utils.py b/cms/djangoapps/contentstore/tests/test_transcripts_utils.py index 858f07a8b3..1a82ba4829 100644 --- a/cms/djangoapps/contentstore/tests/test_transcripts_utils.py +++ b/cms/djangoapps/contentstore/tests/test_transcripts_utils.py @@ -161,7 +161,6 @@ class TestDownloadYoutubeSubs(ModuleStoreTestCase): number = '999' display_name = 'Test course' - def clear_sub_content(self, subs_id): """ Remove, if subtitle content exists. @@ -490,7 +489,6 @@ class TestTranscript(unittest.TestCase): """) - self.sjson_transcript = textwrap.dedent("""\ { "start": [ diff --git a/cms/djangoapps/contentstore/views/user.py b/cms/djangoapps/contentstore/views/user.py index b316de987e..7e3b720e2b 100644 --- a/cms/djangoapps/contentstore/views/user.py +++ b/cms/djangoapps/contentstore/views/user.py @@ -100,7 +100,6 @@ def _course_team_user(request, course_key, email): } return JsonResponse(msg, 400) - try: user = User.objects.get(email=email) except Exception: diff --git a/common/djangoapps/course_modes/tests/test_views.py b/common/djangoapps/course_modes/tests/test_views.py index ed6d817ad3..70ee329610 100644 --- a/common/djangoapps/course_modes/tests/test_views.py +++ b/common/djangoapps/course_modes/tests/test_views.py @@ -150,7 +150,6 @@ class CourseModeViewTest(ModuleStoreTestCase): response = self.client.get(choose_track_url) self.assertRedirects(response, reverse('dashboard')) - # Mapping of course modes to the POST parameters sent # when the user chooses that mode. POST_PARAMS_FOR_COURSE_MODE = { diff --git a/common/djangoapps/dark_lang/tests.py b/common/djangoapps/dark_lang/tests.py index c1174adc8b..0f85fb390f 100644 --- a/common/djangoapps/dark_lang/tests.py +++ b/common/djangoapps/dark_lang/tests.py @@ -144,7 +144,6 @@ class DarkLangMiddlewareTests(TestCase): self.process_request(accept='rel-ter;q=1.0, rel;q=0.5') ) - def assertSessionLangEquals(self, value, request): """ Assert that the 'django_language' set in request.session is equal to value diff --git a/common/djangoapps/edxmako/makoloader.py b/common/djangoapps/edxmako/makoloader.py index 8e741f9946..8c2781cddc 100644 --- a/common/djangoapps/edxmako/makoloader.py +++ b/common/djangoapps/edxmako/makoloader.py @@ -34,7 +34,6 @@ class MakoLoader(object): self.module_directory = module_directory - def __call__(self, template_name, template_dirs=None): return self.load_template(template_name, template_dirs) diff --git a/common/djangoapps/status/tests.py b/common/djangoapps/status/tests.py index bf60017036..e4f750d8c1 100644 --- a/common/djangoapps/status/tests.py +++ b/common/djangoapps/status/tests.py @@ -36,7 +36,6 @@ class TestStatus(TestCase): "edX/toy/2012_Fall" : "A toy story" }""" - # json to use, expected results for course=None (e.g. homepage), # for toy course, for full course. Note that get_site_status_msg # is supposed to return global message even if course=None. The diff --git a/common/djangoapps/student/migrations/0041_add_dashboard_config.py b/common/djangoapps/student/migrations/0041_add_dashboard_config.py index 13c10d584a..dbf6da578c 100644 --- a/common/djangoapps/student/migrations/0041_add_dashboard_config.py +++ b/common/djangoapps/student/migrations/0041_add_dashboard_config.py @@ -18,12 +18,10 @@ class Migration(SchemaMigration): )) db.send_create_signal('student', ['DashboardConfiguration']) - def backwards(self, orm): # Deleting model 'DashboardConfiguration' db.delete_table('student_dashboardconfiguration') - models = { 'auth.group': { 'Meta': {'object_name': 'Group'}, diff --git a/common/djangoapps/terrain/stubs/ora.py b/common/djangoapps/terrain/stubs/ora.py index 1cf2ae43ba..1edf93c484 100644 --- a/common/djangoapps/terrain/stubs/ora.py +++ b/common/djangoapps/terrain/stubs/ora.py @@ -300,7 +300,6 @@ class StubOraHandler(StubHttpRequestHandler): """ self._success_response({'problem_list': self.server.problem_list}) - @require_params('POST', 'grader_id', 'location', 'submission_id', 'score', 'feedback', 'submission_key') def _save_grade(self): """ @@ -421,7 +420,6 @@ class StubOraHandler(StubHttpRequestHandler): ) self.send_response(400) - def _student(self, method, key='student_id'): """ Return the `StudentState` instance for the student ID given diff --git a/common/lib/capa/capa/correctmap.py b/common/lib/capa/capa/correctmap.py index df7efee343..ff6e9ed142 100644 --- a/common/lib/capa/capa/correctmap.py +++ b/common/lib/capa/capa/correctmap.py @@ -165,7 +165,6 @@ class CorrectMap(object): self.cmap.update(other_cmap.get_dict()) self.set_overall_message(other_cmap.get_overall_message()) - def set_overall_message(self, message_str): """ Set a message that applies to the question as a whole, rather than to individual inputs. """ diff --git a/common/lib/capa/capa/customrender.py b/common/lib/capa/capa/customrender.py index 24f8d9b9ec..f56f963577 100644 --- a/common/lib/capa/capa/customrender.py +++ b/common/lib/capa/capa/customrender.py @@ -50,7 +50,6 @@ class MathRenderer(object): mathstr = mathstr.replace(r'\displaystyle', '') self.mathstr = mathstr.replace('mathjaxinline]', '%s]' % mtag) - def get_html(self): """ Return the contents of this tag, rendered to html, as an etree element. diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index 0660340b41..f54a02ca03 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -839,7 +839,6 @@ class MatlabInput(CodeInput): 'No response from Xqueue within {xqueue_timeout} seconds. Aborted.' ).format(xqueue_timeout=XQUEUE_TIMEOUT) - def handle_ajax(self, dispatch, data): """ Handle AJAX calls directed to this input diff --git a/common/lib/capa/capa/tests/test_inputtypes.py b/common/lib/capa/capa/tests/test_inputtypes.py index 8d02edd4e8..b53ce496f4 100644 --- a/common/lib/capa/capa/tests/test_inputtypes.py +++ b/common/lib/capa/capa/tests/test_inputtypes.py @@ -604,7 +604,6 @@ class MatlabTest(unittest.TestCase): the_input = self.input_class(test_capa_system(), elt, state) self.assertEqual(the_input.status, 'queued') - @patch('capa.inputtypes.time.time', return_value=45) def test_matlab_response_timeout_exceeded(self, time): diff --git a/common/lib/capa/capa/tests/test_responsetypes.py b/common/lib/capa/capa/tests/test_responsetypes.py index c1124d187b..970e3ea58e 100644 --- a/common/lib/capa/capa/tests/test_responsetypes.py +++ b/common/lib/capa/capa/tests/test_responsetypes.py @@ -606,7 +606,6 @@ class StringResponseTest(ResponseTest): self.assert_grade(problem, u"î", "incorrect") self.assert_grade(problem, u"o", "incorrect") - def test_backslash_and_unicode_regexps(self): """ Test some special cases of [unicode] regexps. @@ -1042,8 +1041,6 @@ class CodeResponseTest(ResponseTest): self.assertEquals(output[answer_id]['msg'], u'Invalid grader reply. Please contact the course staff.') - - class ChoiceResponseTest(ResponseTest): from capa.tests.response_xml_factory import ChoiceResponseXMLFactory xml_factory_class = ChoiceResponseXMLFactory diff --git a/common/lib/capa/capa/tests/test_util.py b/common/lib/capa/capa/tests/test_util.py index 42c8975cd1..87ef04eaaa 100644 --- a/common/lib/capa/capa/tests/test_util.py +++ b/common/lib/capa/capa/tests/test_util.py @@ -82,7 +82,6 @@ class UtilTest(unittest.TestCase): result = compare_with_tolerance(infinity, infinity, '1.0', False) self.assertTrue(result) - def test_sanitize_html(self): """ Test for html sanitization with bleach. diff --git a/common/lib/chem/chem/chemcalc.py b/common/lib/chem/chem/chemcalc.py index 612e63c0f0..930c5818f4 100644 --- a/common/lib/chem/chem/chemcalc.py +++ b/common/lib/chem/chem/chemcalc.py @@ -192,7 +192,6 @@ def _render_to_html(tree): return children.replace(' ', '') - def render_to_html(eq): ''' Render a chemical equation string to html. @@ -231,7 +230,6 @@ def render_to_html(eq): # only one side return spanify(render_expression(left)) - return spanify(render_expression(left) + render_arrow(arrow) + render_expression(right)) diff --git a/common/lib/chem/chem/tests.py b/common/lib/chem/chem/tests.py index f422fcf0d1..4f000eb3f2 100644 --- a/common/lib/chem/chem/tests.py +++ b/common/lib/chem/chem/tests.py @@ -64,7 +64,6 @@ class Test_Compare_Equations(unittest.TestCase): self.assertFalse(chemical_equations_equal('H2O( -> H2O2', 'H2O -> H2O2')) - self.assertFalse(chemical_equations_equal('H2 + O2 ==> H2O2', # strange arrow '2O2 + 2H2 -> 2H2O2')) diff --git a/common/lib/symmath/symmath/formula.py b/common/lib/symmath/symmath/formula.py index 5d25ad5052..4da7cdc71c 100644 --- a/common/lib/symmath/symmath/formula.py +++ b/common/lib/symmath/symmath/formula.py @@ -257,7 +257,6 @@ class formula(object): fix_pmathml(xml) - def fix_hat(xml): """ hat i is turned into i^ ; mangle diff --git a/common/lib/xmodule/xmodule/exceptions.py b/common/lib/xmodule/xmodule/exceptions.py index 75be380581..a6d3686ca2 100644 --- a/common/lib/xmodule/xmodule/exceptions.py +++ b/common/lib/xmodule/xmodule/exceptions.py @@ -48,7 +48,6 @@ class HeartbeatFailure(Exception): def __unicode__(self, *args, **kwargs): return self.message - def __init__(self, msg, service): """ In addition to a msg, provide the name of the service. diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py index d929e91e86..2cf31195e6 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/common/lib/xmodule/xmodule/lti_module.py @@ -752,7 +752,6 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'} log.debug("[LTI]: Incorrect action.") return Response(response_xml_template.format(**unsupported_values), content_type='application/xml') - @classmethod def parse_grade_xml_body(cls, body): """ diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/__init__.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/__init__.py index 9582fb88c6..06ae542506 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/__init__.py @@ -14,7 +14,6 @@ class BlockKey(namedtuple('BlockKey', 'type id')): def __new__(cls, type, id): return super(BlockKey, cls).__new__(cls, type, id) - @classmethod @contract(usage_key=BlockUsageLocator) def from_usage_key(cls, usage_key): diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py index 0eb4e822f5..3a7766881e 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py @@ -38,7 +38,6 @@ class SplitMongoKVS(InheritanceKeyValueStore): self.parent = parent - def get(self, key): # load the field, if needed if key.field_name not in self._fields: diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py b/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py index 56cf3bd46e..dea9ee8d52 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py @@ -115,7 +115,6 @@ class ModuleStoreSettingsMigration(TestCase): } } - def assertStoreValuesEqual(self, store_setting1, store_setting2): """ Tests whether the fields in the given store_settings are equal. diff --git a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py index 376dc7dc32..d689ed51ad 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py @@ -188,7 +188,6 @@ def adapt_references(subtree, destination_course_key, export_fs): ) - def _export_field_content(xblock_item, item_dir): """ Export all fields related to 'xblock_item' other than 'metadata' and 'data' to json file in provided directory diff --git a/common/lib/xmodule/xmodule/randomize_module.py b/common/lib/xmodule/xmodule/randomize_module.py index 6827a68c35..b0d9b8de6d 100644 --- a/common/lib/xmodule/xmodule/randomize_module.py +++ b/common/lib/xmodule/xmodule/randomize_module.py @@ -77,7 +77,6 @@ class RandomizeModule(RandomizeFields, XModule): return [self.child_descriptor] - def student_view(self, context): if self.child is None: # raise error instead? In fact, could complain on descriptor load... @@ -95,7 +94,6 @@ class RandomizeDescriptor(RandomizeFields, SequenceDescriptor): filename_extension = "xml" - def definition_to_xml(self, resource_fs): xml_object = etree.Element('randomize') diff --git a/common/lib/xmodule/xmodule/template_module.py b/common/lib/xmodule/xmodule/template_module.py index 34ba8f6c69..7587ae8e43 100644 --- a/common/lib/xmodule/xmodule/template_module.py +++ b/common/lib/xmodule/xmodule/template_module.py @@ -60,7 +60,6 @@ class CustomTagDescriptor(RawDescriptor): template = Template(template_module_data) return template.render(**params) - @property def rendered_html(self): return self.render_template(self.system, self.data) diff --git a/common/lib/xmodule/xmodule/tests/test_bulk_assertions.py b/common/lib/xmodule/xmodule/tests/test_bulk_assertions.py index a6a2860a4e..f930b6b5d4 100644 --- a/common/lib/xmodule/xmodule/tests/test_bulk_assertions.py +++ b/common/lib/xmodule/xmodule/tests/test_bulk_assertions.py @@ -22,7 +22,6 @@ class TestBulkAssertionTestCase(BulkAssertionTest): def test_passing_asserts_passthrough(self, assertion, *args): getattr(self, assertion)(*args) - @ddt.data( ('assertTrue', False), ('assertFalse', True), diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 06ea77d152..045af6d390 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -1395,7 +1395,6 @@ class CapaModuleTest(unittest.TestCase): Run the test for each possible rerandomize value """ - def _reset_and_get_seed(module): """ Reset the XModule and return the module's seed diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 92741c6ba5..2d7518b1c6 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -150,7 +150,6 @@ class ImportTestCase(BaseCourseTestCase): self.assertNotEqual(descriptor1.location, descriptor2.location) - def test_reimport(self): '''Make sure an already-exported error xml tag loads properly''' diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index cf5b9fff19..3a7d5699d1 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -211,7 +211,6 @@ class XModuleMixin(XBlockMixin): def runtime(self, value): self._runtime = value - @property def system(self): """ @@ -719,7 +718,6 @@ class XModuleDescriptor(XModuleMixin, HTMLSnippet, ResourceTemplates, XBlock): entry_point = "xmodule.v1" module_class = XModule - # VS[compat]. Backwards compatibility code that can go away after # importing 2012 courses. # A set of metadata key conversions that we want to make @@ -835,7 +833,6 @@ class XModuleDescriptor(XModuleMixin, HTMLSnippet, ResourceTemplates, XBlock): """ pass - # =============================== BUILTIN METHODS ========================== def __eq__(self, other): return (self.scope_ids == other.scope_ids and diff --git a/common/test/acceptance/tests/test_annotatable.py b/common/test/acceptance/tests/test_annotatable.py index 24780369cc..692313fd17 100644 --- a/common/test/acceptance/tests/test_annotatable.py +++ b/common/test/acceptance/tests/test_annotatable.py @@ -29,7 +29,6 @@ class AnnotatableProblemTest(UniqueCourseTest): USERNAME = "STAFF_TESTER" EMAIL = "johndoe@example.com" - DATA_TEMPLATE = dedent("""\ Instruction text diff --git a/common/xml_cleanup.py b/common/xml_cleanup.py index 5f2b527063..9fc8fd00f6 100755 --- a/common/xml_cleanup.py +++ b/common/xml_cleanup.py @@ -86,7 +86,6 @@ def cleanup(filepath, remove_meta): if attr in attrs: del attrs[attr] - with open(filepath, "w") as f: f.write(etree.tostring(xml)) diff --git a/docs/en_us/developers/source/conf.py b/docs/en_us/developers/source/conf.py index a3b9d6a01a..724bee93f2 100644 --- a/docs/en_us/developers/source/conf.py +++ b/docs/en_us/developers/source/conf.py @@ -10,7 +10,6 @@ from path import path on_rtd = os.environ.get('READTHEDOCS', None) == 'True' - sys.path.append('../../../../') from docs.shared.conf import * @@ -188,7 +187,6 @@ def strip_tags(html): return s.get_data() - def process_docstring(app, what, name, obj, options, lines): """Autodoc django models""" diff --git a/docs/en_us/platform_api/source/conf.py b/docs/en_us/platform_api/source/conf.py index b0fcd45019..204089f741 100644 --- a/docs/en_us/platform_api/source/conf.py +++ b/docs/en_us/platform_api/source/conf.py @@ -179,7 +179,6 @@ def strip_tags(html): return s.get_data() - def process_docstring(app, what, name, obj, options, lines): """Autodoc django models""" diff --git a/docs/shared/conf.py b/docs/shared/conf.py index 1659291e99..f020c247cd 100644 --- a/docs/shared/conf.py +++ b/docs/shared/conf.py @@ -36,7 +36,6 @@ def add_base(paths): return [os.path.join(BASEDIR, x) for x in paths] - # If extensions (or modules to document with autodoc) are in another directory, # add these directories to sys.path here. If the directory is relative to the # documentation root, use os.path.abspath to make it absolute, like shown here. diff --git a/lms/djangoapps/bulk_email/migrations/0010_auto__chg_field_optout_course_id__add_field_courseemail_template_name_.py b/lms/djangoapps/bulk_email/migrations/0010_auto__chg_field_optout_course_id__add_field_courseemail_template_name_.py index b9d77f7dd0..3e55dfb226 100644 --- a/lms/djangoapps/bulk_email/migrations/0010_auto__chg_field_optout_course_id__add_field_courseemail_template_name_.py +++ b/lms/djangoapps/bulk_email/migrations/0010_auto__chg_field_optout_course_id__add_field_courseemail_template_name_.py @@ -21,7 +21,6 @@ class Migration(SchemaMigration): self.gf('django.db.models.fields.CharField')(max_length=255, null=True), keep_default=False) - # Changing field 'CourseEmail.course_id' db.alter_column('bulk_email_courseemail', 'course_id', self.gf('xmodule_django.models.CourseKeyField')(max_length=255)) # Adding field 'CourseEmailTemplate.name' @@ -29,7 +28,6 @@ class Migration(SchemaMigration): self.gf('django.db.models.fields.CharField')(max_length=255, unique=True, null=True), keep_default=False) - # Changing field 'CourseAuthorization.course_id' db.alter_column('bulk_email_courseauthorization', 'course_id', self.gf('xmodule_django.models.CourseKeyField')(unique=True, max_length=255)) @@ -43,13 +41,11 @@ class Migration(SchemaMigration): # Deleting field 'CourseEmail.from_addr' db.delete_column('bulk_email_courseemail', 'from_addr') - # Changing field 'CourseEmail.course_id' db.alter_column('bulk_email_courseemail', 'course_id', self.gf('django.db.models.fields.CharField')(max_length=255)) # Deleting field 'CourseEmailTemplate.name' db.delete_column('bulk_email_courseemailtemplate', 'name') - # Changing field 'CourseAuthorization.course_id' db.alter_column('bulk_email_courseauthorization', 'course_id', self.gf('django.db.models.fields.CharField')(max_length=255, unique=True)) diff --git a/lms/djangoapps/courseware/tests/test_model_data.py b/lms/djangoapps/courseware/tests/test_model_data.py index 185806411f..f2f3fe532b 100644 --- a/lms/djangoapps/courseware/tests/test_model_data.py +++ b/lms/djangoapps/courseware/tests/test_model_data.py @@ -238,7 +238,6 @@ class StorageTestBase(object): self.field_data_cache = FieldDataCache([self.mock_descriptor], course_id, self.user) self.kvs = DjangoKeyValueStore(self.field_data_cache) - def test_set_and_get_existing_field(self): self.kvs.set(self.key_factory('existing_field'), 'test_value') self.assertEquals('test_value', self.kvs.get(self.key_factory('existing_field'))) diff --git a/lms/djangoapps/lms_migration/management/commands/create_user.py b/lms/djangoapps/lms_migration/management/commands/create_user.py index 5d96d96a8a..10051b349c 100644 --- a/lms/djangoapps/lms_migration/management/commands/create_user.py +++ b/lms/djangoapps/lms_migration/management/commands/create_user.py @@ -98,7 +98,6 @@ class Command(BaseCommand): name = raw_input('Full name: ') - user = User(username=uname, email=email, is_active=True) user.set_password(password) try: diff --git a/lms/djangoapps/lms_migration/migrate.py b/lms/djangoapps/lms_migration/migrate.py index daa30d8645..ee6f715fb6 100644 --- a/lms/djangoapps/lms_migration/migrate.py +++ b/lms/djangoapps/lms_migration/migrate.py @@ -147,7 +147,6 @@ def manage_modulestores(request, reload_dir=None, commit_id=None): else: html += '
    • %s
    ' % escape(data) - #---------------------------------------- html += '
    ' diff --git a/lms/djangoapps/notification_prefs/views.py b/lms/djangoapps/notification_prefs/views.py index 6bcb60a489..4df6465830 100644 --- a/lms/djangoapps/notification_prefs/views.py +++ b/lms/djangoapps/notification_prefs/views.py @@ -144,7 +144,6 @@ def ajax_status(request): if not request.user.is_authenticated(): raise PermissionDenied - qs = UserPreference.objects.filter( user=request.user, key=NOTIFICATION_PREF_KEY diff --git a/lms/djangoapps/shoppingcart/migrations/0018_auto__add_donation.py b/lms/djangoapps/shoppingcart/migrations/0018_auto__add_donation.py index fd89917904..012e7e2d4c 100644 --- a/lms/djangoapps/shoppingcart/migrations/0018_auto__add_donation.py +++ b/lms/djangoapps/shoppingcart/migrations/0018_auto__add_donation.py @@ -16,12 +16,10 @@ class Migration(SchemaMigration): )) db.send_create_signal('shoppingcart', ['Donation']) - def backwards(self, orm): # Deleting model 'Donation' db.delete_table('shoppingcart_donation') - models = { 'auth.group': { 'Meta': {'object_name': 'Group'}, diff --git a/lms/djangoapps/shoppingcart/migrations/0019_auto__add_donationconfiguration.py b/lms/djangoapps/shoppingcart/migrations/0019_auto__add_donationconfiguration.py index 78c0bdde14..d7364612bc 100644 --- a/lms/djangoapps/shoppingcart/migrations/0019_auto__add_donationconfiguration.py +++ b/lms/djangoapps/shoppingcart/migrations/0019_auto__add_donationconfiguration.py @@ -17,12 +17,10 @@ class Migration(SchemaMigration): )) db.send_create_signal('shoppingcart', ['DonationConfiguration']) - def backwards(self, orm): # Deleting model 'DonationConfiguration' db.delete_table('shoppingcart_donationconfiguration') - models = { 'auth.group': { 'Meta': {'object_name': 'Group'}, diff --git a/lms/djangoapps/shoppingcart/migrations/0020_auto__add_courseregcodeitem__add_courseregcodeitemannotation__add_fiel.py b/lms/djangoapps/shoppingcart/migrations/0020_auto__add_courseregcodeitem__add_courseregcodeitemannotation__add_fiel.py index 941fcd0100..af49f01c0a 100644 --- a/lms/djangoapps/shoppingcart/migrations/0020_auto__add_courseregcodeitem__add_courseregcodeitemannotation__add_fiel.py +++ b/lms/djangoapps/shoppingcart/migrations/0020_auto__add_courseregcodeitem__add_courseregcodeitemannotation__add_fiel.py @@ -59,7 +59,6 @@ class Migration(SchemaMigration): self.gf('django.db.models.fields.CharField')(default='personal', max_length=32), keep_default=False) - def backwards(self, orm): # Deleting model 'CourseRegCodeItem' db.delete_table('shoppingcart_courseregcodeitem') @@ -88,7 +87,6 @@ class Migration(SchemaMigration): # Deleting field 'Order.order_type' db.delete_column('shoppingcart_order', 'order_type') - models = { 'auth.group': { 'Meta': {'object_name': 'Group'}, diff --git a/lms/djangoapps/shoppingcart/migrations/0021_auto__add_field_orderitem_created__add_field_orderitem_modified.py b/lms/djangoapps/shoppingcart/migrations/0021_auto__add_field_orderitem_created__add_field_orderitem_modified.py index c4ca4bc4c8..226d2475e8 100644 --- a/lms/djangoapps/shoppingcart/migrations/0021_auto__add_field_orderitem_created__add_field_orderitem_modified.py +++ b/lms/djangoapps/shoppingcart/migrations/0021_auto__add_field_orderitem_created__add_field_orderitem_modified.py @@ -18,7 +18,6 @@ class Migration(SchemaMigration): self.gf('model_utils.fields.AutoLastModifiedField')(default=datetime.datetime.now), keep_default=False) - def backwards(self, orm): # Deleting field 'OrderItem.created' db.delete_column('shoppingcart_orderitem', 'created') @@ -26,7 +25,6 @@ class Migration(SchemaMigration): # Deleting field 'OrderItem.modified' db.delete_column('shoppingcart_orderitem', 'modified') - models = { 'auth.group': { 'Meta': {'object_name': 'Group'}, diff --git a/lms/djangoapps/shoppingcart/processors/tests/test_CyberSource.py b/lms/djangoapps/shoppingcart/processors/tests/test_CyberSource.py index f8120f911c..d719e06f89 100644 --- a/lms/djangoapps/shoppingcart/processors/tests/test_CyberSource.py +++ b/lms/djangoapps/shoppingcart/processors/tests/test_CyberSource.py @@ -263,7 +263,6 @@ class CyberSourceTests(TestCase): # finally, tests an accepted order self.assertTrue(payment_accepted(params)['accepted']) - @patch('shoppingcart.processors.CyberSource.render_to_string', autospec=True) def test_render_purchase_form_html(self, render): """ diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index d731483ad0..f7c2a2b17d 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -310,7 +310,6 @@ class PhotoVerification(StatusModel): return (status, error_msg) - def parsed_error_msg(self): """ Sometimes, the error message we've received needs to be parsed into @@ -777,7 +776,6 @@ class SoftwareSecurePhotoVerification(PhotoVerification): return header_txt + "\n\n" + body_txt - def send_request(self): """ Assembles a submission to Software Secure and sends it via HTTPS. diff --git a/pavelib/paver_tests/test_paver_get_quality_reports.py b/pavelib/paver_tests/test_paver_get_quality_reports.py index 75352bed0c..a1cde45300 100644 --- a/pavelib/paver_tests/test_paver_get_quality_reports.py +++ b/pavelib/paver_tests/test_paver_get_quality_reports.py @@ -9,7 +9,6 @@ import paver.easy from paver.easy import BuildFailure - class TestGetReportFiles(unittest.TestCase): """ Ensure only the report files we want are returned as part of run_quality. diff --git a/pavelib/utils/test/suites/acceptance_suite.py b/pavelib/utils/test/suites/acceptance_suite.py index 36bb0c083b..d33ee744fe 100644 --- a/pavelib/utils/test/suites/acceptance_suite.py +++ b/pavelib/utils/test/suites/acceptance_suite.py @@ -95,7 +95,6 @@ class AcceptanceTestSuite(TestSuite): if not self.fasttest: self._setup_acceptance_db() - def _setup_acceptance_db(self): """ TODO: Improve the following From 5677b143db4c41a6913954cf679c5e8672a7a110 Mon Sep 17 00:00:00 2001 From: stv Date: Sat, 8 Nov 2014 20:07:08 -0800 Subject: [PATCH 18/48] Fix PEP8: W391 blank line at end of file --- cms/djangoapps/contentstore/__init__.py | 1 - cms/djangoapps/contentstore/features/common.py | 1 - .../management/commands/restore_asset_from_trashcan.py | 1 - cms/djangoapps/contentstore/views/tabs.py | 1 - cms/wsgi.py | 1 - common/djangoapps/student/auth.py | 1 - .../student/management/commands/anonymized_id_mapping.py | 1 - common/djangoapps/util/models.py | 1 - common/djangoapps/util/tests/__init__.py | 1 - common/lib/xmodule/xmodule/mako_module.py | 1 - common/lib/xmodule/xmodule/tests/test_editing_module.py | 1 - common/lib/xmodule/xmodule/tests/test_word_cloud.py | 1 - pavelib/utils/test/utils.py | 1 - 13 files changed, 13 deletions(-) diff --git a/cms/djangoapps/contentstore/__init__.py b/cms/djangoapps/contentstore/__init__.py index 8b13789179..e69de29bb2 100644 --- a/cms/djangoapps/contentstore/__init__.py +++ b/cms/djangoapps/contentstore/__init__.py @@ -1 +0,0 @@ - diff --git a/cms/djangoapps/contentstore/features/common.py b/cms/djangoapps/contentstore/features/common.py index 491201bd15..e3929cf94d 100644 --- a/cms/djangoapps/contentstore/features/common.py +++ b/cms/djangoapps/contentstore/features/common.py @@ -387,4 +387,3 @@ def create_other_user(_step, name, has_extra_perms, role_name): @step('I log out') def log_out(_step): world.visit('logout') - diff --git a/cms/djangoapps/contentstore/management/commands/restore_asset_from_trashcan.py b/cms/djangoapps/contentstore/management/commands/restore_asset_from_trashcan.py index 6770bfaf44..7513b8b47c 100644 --- a/cms/djangoapps/contentstore/management/commands/restore_asset_from_trashcan.py +++ b/cms/djangoapps/contentstore/management/commands/restore_asset_from_trashcan.py @@ -10,4 +10,3 @@ class Command(BaseCommand): raise CommandError("restore_asset_from_trashcan requires one argument: ") restore_asset_from_trashcan(args[0]) - diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py index bc4636a030..d374e64343 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/cms/djangoapps/contentstore/views/tabs.py @@ -204,4 +204,3 @@ def primitive_insert(course, num, tab_type, name): tabs = course.tabs tabs.insert(num, new_tab) modulestore().update_item(course, ModuleStoreEnum.UserID.primitive_command) - diff --git a/cms/wsgi.py b/cms/wsgi.py index 199d3939ee..af11d8d532 100644 --- a/cms/wsgi.py +++ b/cms/wsgi.py @@ -17,4 +17,3 @@ startup.run() # as well as any WSGI server configured to use this file. from django.core.wsgi import get_wsgi_application application = get_wsgi_application() - diff --git a/common/djangoapps/student/auth.py b/common/djangoapps/student/auth.py index 49a06f2579..7fa22b9d58 100644 --- a/common/djangoapps/student/auth.py +++ b/common/djangoapps/student/auth.py @@ -83,4 +83,3 @@ def _check_caller_authority(caller, role): elif isinstance(role, CourseRole): # instructors can change the roles w/in their course if not has_access(caller, CourseInstructorRole(role.course_key)): raise PermissionDenied - diff --git a/common/djangoapps/student/management/commands/anonymized_id_mapping.py b/common/djangoapps/student/management/commands/anonymized_id_mapping.py index d7707cb738..4a1cf1b50a 100644 --- a/common/djangoapps/student/management/commands/anonymized_id_mapping.py +++ b/common/djangoapps/student/management/commands/anonymized_id_mapping.py @@ -65,4 +65,3 @@ class Command(BaseCommand): )) except IOError: raise CommandError("Error writing to file: %s" % output_filename) - diff --git a/common/djangoapps/util/models.py b/common/djangoapps/util/models.py index f2fb241a82..6b20219993 100644 --- a/common/djangoapps/util/models.py +++ b/common/djangoapps/util/models.py @@ -1,2 +1 @@ # Create your models here. - diff --git a/common/djangoapps/util/tests/__init__.py b/common/djangoapps/util/tests/__init__.py index 8b13789179..e69de29bb2 100644 --- a/common/djangoapps/util/tests/__init__.py +++ b/common/djangoapps/util/tests/__init__.py @@ -1 +0,0 @@ - diff --git a/common/lib/xmodule/xmodule/mako_module.py b/common/lib/xmodule/xmodule/mako_module.py index 3d37bd615e..ecbac28d86 100644 --- a/common/lib/xmodule/xmodule/mako_module.py +++ b/common/lib/xmodule/xmodule/mako_module.py @@ -39,4 +39,3 @@ class MakoModuleDescriptor(XModuleDescriptor): def get_html(self): return self.system.render_template( self.mako_template, self.get_context()) - diff --git a/common/lib/xmodule/xmodule/tests/test_editing_module.py b/common/lib/xmodule/xmodule/tests/test_editing_module.py index 01915830de..b492cf63b1 100644 --- a/common/lib/xmodule/xmodule/tests/test_editing_module.py +++ b/common/lib/xmodule/xmodule/tests/test_editing_module.py @@ -65,4 +65,3 @@ class TabsEditingDescriptorTestCase(unittest.TestCase): """"test get_context""" rendered_context = self.descriptor.get_context() self.assertListEqual(rendered_context['tabs'], self.tabs) - diff --git a/common/lib/xmodule/xmodule/tests/test_word_cloud.py b/common/lib/xmodule/xmodule/tests/test_word_cloud.py index bc7b542003..c690054cbb 100644 --- a/common/lib/xmodule/xmodule/tests/test_word_cloud.py +++ b/common/lib/xmodule/xmodule/tests/test_word_cloud.py @@ -46,4 +46,3 @@ class WordCloudModuleTest(LogicTest): self.assertEqual( 100.0, sum(i['percent'] for i in response['top_words'])) - diff --git a/pavelib/utils/test/utils.py b/pavelib/utils/test/utils.py index e92ae7f201..17c1c71101 100644 --- a/pavelib/utils/test/utils.py +++ b/pavelib/utils/test/utils.py @@ -73,4 +73,3 @@ def check_firefox_version(): '\t$ firefox --version\n' '\t{version}'.format(version=expected_firefox_ver) ) - From fe1c7d62dce840860382a849338bbc03d58ea732 Mon Sep 17 00:00:00 2001 From: stv Date: Mon, 10 Nov 2014 10:59:01 -0800 Subject: [PATCH 19/48] Lower PEP8 threshold to 200 violations --- scripts/all-tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/all-tests.sh b/scripts/all-tests.sh index 894d6bbc79..a666402078 100755 --- a/scripts/all-tests.sh +++ b/scripts/all-tests.sh @@ -57,7 +57,7 @@ set -e # Violations thresholds for failing the build PYLINT_THRESHOLD=4725 -PEP8_THRESHOLD=400 +PEP8_THRESHOLD=200 source $HOME/jenkins_env From bd87a9dd2955add49acba55e209a652f9081b671 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 7 Nov 2014 11:14:12 -0500 Subject: [PATCH 20/48] Record processor response on both successful and unsuccessful orders. --- .../shoppingcart/processors/CyberSource2.py | 24 ++++++++++++++++++- .../shoppingcart/processors/exceptions.py | 1 + .../processors/tests/test_CyberSource2.py | 14 +++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/shoppingcart/processors/CyberSource2.py b/lms/djangoapps/shoppingcart/processors/CyberSource2.py index 5606279829..1d66138a96 100644 --- a/lms/djangoapps/shoppingcart/processors/CyberSource2.py +++ b/lms/djangoapps/shoppingcart/processors/CyberSource2.py @@ -79,6 +79,7 @@ def process_postpay_callback(params): 'error_html': '' } else: + _record_payment_info(params, result['order']) return { 'success': False, 'order': result['order'], @@ -86,6 +87,11 @@ def process_postpay_callback(params): } except CCProcessorException as error: log.exception('error processing CyberSource postpay callback') + # if we have the order and the id, log it + if hasattr(error, 'order'): + _record_payment_info(params, error.order) + else: + log.info(json.dumps(params)) return { 'success': False, 'order': None, # due to exception we may not have the order @@ -350,7 +356,7 @@ def _payment_accepted(order_id, auth_amount, currency, decision): 'order': order } else: - raise CCProcessorWrongAmountException( + ex = CCProcessorWrongAmountException( _( u"The amount charged by the processor {charged_amount} {charged_amount_currency} is different " u"than the total cost of the order {total_cost} {total_cost_currency}." @@ -361,6 +367,8 @@ def _payment_accepted(order_id, auth_amount, currency, decision): total_cost_currency=order.currency ) ) + ex.order = order + raise ex else: return { 'accepted': False, @@ -408,6 +416,20 @@ def _record_purchase(params, order): ) +def _record_payment_info(params, order): + """ + Record the purchase and run purchased_callbacks + + Args: + params (dict): The parameters we received from CyberSource. + + Returns: + None + """ + order.processor_reply_dump = json.dumps(params) + order.save() + + def _get_processor_decline_html(params): """ Return HTML indicating that the user's payment was declined. diff --git a/lms/djangoapps/shoppingcart/processors/exceptions.py b/lms/djangoapps/shoppingcart/processors/exceptions.py index 2bcf35e50f..47a51a8b60 100644 --- a/lms/djangoapps/shoppingcart/processors/exceptions.py +++ b/lms/djangoapps/shoppingcart/processors/exceptions.py @@ -16,5 +16,6 @@ class CCProcessorDataException(CCProcessorException): class CCProcessorWrongAmountException(CCProcessorException): pass + class CCProcessorUserCancelled(CCProcessorException): pass diff --git a/lms/djangoapps/shoppingcart/processors/tests/test_CyberSource2.py b/lms/djangoapps/shoppingcart/processors/tests/test_CyberSource2.py index de3777222f..5a12fa5597 100644 --- a/lms/djangoapps/shoppingcart/processors/tests/test_CyberSource2.py +++ b/lms/djangoapps/shoppingcart/processors/tests/test_CyberSource2.py @@ -49,6 +49,13 @@ class CyberSource2Test(TestCase): line_cost=self.COST ) + def assert_dump_recorded(self, order): + """ + Verify that this order does have a dump of information from the + payment processor. + """ + self.assertNotEqual(order.processor_reply_dump, '') + def test_render_purchase_form_html(self): # Verify that the HTML form renders with the payment URL specified # in the test settings. @@ -131,6 +138,7 @@ class CyberSource2Test(TestCase): # Expect that the order has been marked as purchased self.assertEqual(result['order'].status, 'purchased') + self.assert_dump_recorded(result['order']) def test_process_payment_rejected(self): # Simulate a callback from CyberSource indicating that the payment was rejected @@ -140,6 +148,7 @@ class CyberSource2Test(TestCase): # Expect that we get an error message self.assertFalse(result['success']) self.assertIn(u"did not accept your payment", result['error_html']) + self.assert_dump_recorded(result['order']) def test_process_payment_invalid_signature(self): # Simulate a callback from CyberSource indicating that the payment was rejected @@ -167,6 +176,9 @@ class CyberSource2Test(TestCase): # Expect an error self.assertFalse(result['success']) self.assertIn(u"different amount than the order total", result['error_html']) + # refresh data for current order + order = Order.objects.get(id=self.order.id) + self.assert_dump_recorded(order) def test_process_amount_paid_not_decimal(self): # Change the payment amount to a non-decimal @@ -202,6 +214,7 @@ class CyberSource2Test(TestCase): msg="Payment was not successful: {error}".format(error=result.get('error_html')) ) self.assertEqual(result['error_html'], '') + self.assert_dump_recorded(result['order']) # Expect that the order has placeholders for the missing credit card digits self.assertEqual(result['order'].bill_to_ccnum, '####') @@ -233,6 +246,7 @@ class CyberSource2Test(TestCase): # Verify that this executes without a unicode error result = process_postpay_callback(params) self.assertTrue(result['success']) + self.assert_dump_recorded(result['order']) @ddt.data('string', u'üñîçø∂é') def test_get_processor_exception_html(self, error_string): From 58b4982d2da013039367418ab984c9bdadb2e6be Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 10 Nov 2014 12:08:28 -0500 Subject: [PATCH 21/48] harden down access to the shoppingcart if the ENABLE_PAID_COURSE_REGISTRATION flag is not set (either via the main Django configuration settings) or through microsite overrides --- common/djangoapps/student/tests/tests.py | 1 + lms/djangoapps/courseware/views.py | 5 ++-- lms/djangoapps/instructor/tests/test_api.py | 1 + .../shoppingcart/context_processor.py | 15 ++-------- lms/djangoapps/shoppingcart/decorators.py | 22 ++++++++++++++ .../shoppingcart/tests/test_views.py | 29 +++++++++++++++++++ lms/djangoapps/shoppingcart/urls.py | 26 +++++++---------- lms/djangoapps/shoppingcart/utils.py | 24 +++++++++++++++ lms/djangoapps/shoppingcart/views.py | 12 +++++++- 9 files changed, 105 insertions(+), 30 deletions(-) create mode 100644 lms/djangoapps/shoppingcart/decorators.py create mode 100644 lms/djangoapps/shoppingcart/utils.py diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index 1ef2defca4..3bf2fde715 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -265,6 +265,7 @@ class DashboardTest(ModuleStoreTestCase): @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') @patch('courseware.views.log.warning') + @patch.dict('django.conf.settings.FEATURES', {'ENABLE_PAID_COURSE_REGISTRATION': True}) def test_blocked_course_scenario(self, log_warning): self.client.login(username="jack", password="test") diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 6cb05bd0a9..cf44331ec7 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -49,6 +49,7 @@ from xmodule.tabs import CourseTabList, StaffGradingTab, PeerGradingTab, OpenEnd from xmodule.x_module import STUDENT_VIEW import shoppingcart from shoppingcart.models import CourseRegistrationCode +from shoppingcart.utils import is_shopping_cart_enabled from opaque_keys import InvalidKeyError from microsite_configuration import microsite @@ -731,8 +732,8 @@ def course_about(request, course_id): registration_price = 0 in_cart = False reg_then_add_to_cart_link = "" - if (settings.FEATURES.get('ENABLE_SHOPPING_CART') and - settings.FEATURES.get('ENABLE_PAID_COURSE_REGISTRATION')): + + if (is_shopping_cart_enabled()): registration_price = CourseMode.min_course_price_for_currency(course_key, settings.PAID_COURSE_REGISTRATION_CURRENCY[0]) if request.user.is_authenticated(): diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index feda24b885..b42c1dc3ad 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -1604,6 +1604,7 @@ class TestInstructorAPILevelsAccess(ModuleStoreTestCase, LoginEnrollmentTestCase @ddt.ddt @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) +@patch.dict('django.conf.settings.FEATURES', {'ENABLE_PAID_COURSE_REGISTRATION': True}) class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCase): """ Test endpoints that show data without side effects. diff --git a/lms/djangoapps/shoppingcart/context_processor.py b/lms/djangoapps/shoppingcart/context_processor.py index e884bef7e3..20e096155b 100644 --- a/lms/djangoapps/shoppingcart/context_processor.py +++ b/lms/djangoapps/shoppingcart/context_processor.py @@ -5,9 +5,8 @@ navigation. We want to do this in the context_processor to 1) keep database accesses out of templates (this led to a transaction bug with user email changes) 2) because navigation.html is "called" by being included in other templates, there's no "views.py" to put this. """ -from django.conf import settings + import shoppingcart -from microsite_configuration import microsite def user_has_cart_context_processor(request): @@ -19,16 +18,8 @@ def user_has_cart_context_processor(request): display_shopping_cart = ( # user is logged in and request.user.is_authenticated() and - # settings enable paid course reg - microsite.get_value( - 'ENABLE_PAID_COURSE_REGISTRATION', - settings.FEATURES.get('ENABLE_PAID_COURSE_REGISTRATION') - ) and - # settings enable shopping cart - microsite.get_value( - 'ENABLE_SHOPPING_CART', - settings.FEATURES.get('ENABLE_SHOPPING_CART') - ) and + # do we have the feature turned on + shoppingcart.utils.is_shopping_cart_enabled() and # user's cart has PaidCourseRegistrations shoppingcart.models.Order.user_cart_has_items( request.user, diff --git a/lms/djangoapps/shoppingcart/decorators.py b/lms/djangoapps/shoppingcart/decorators.py new file mode 100644 index 0000000000..9f4366f8bb --- /dev/null +++ b/lms/djangoapps/shoppingcart/decorators.py @@ -0,0 +1,22 @@ +""" +This file defines any decorators used by the shopping cart app +""" + +from django.http import Http404 +from .utils import is_shopping_cart_enabled + + +def enforce_shopping_cart_enabled(func): + """ + Is a decorator that forces a wrapped method to be run in a runtime + which has the ENABLE_SHOPPING_CART flag set + """ + def func_wrapper(*args, **kwargs): + """ + Wrapper function that does the enforcement that + the shopping cart feature is enabled + """ + if not is_shopping_cart_enabled(): + raise Http404 + return func(*args, **kwargs) + return func_wrapper diff --git a/lms/djangoapps/shoppingcart/tests/test_views.py b/lms/djangoapps/shoppingcart/tests/test_views.py index 873960f068..aed264158f 100644 --- a/lms/djangoapps/shoppingcart/tests/test_views.py +++ b/lms/djangoapps/shoppingcart/tests/test_views.py @@ -64,6 +64,7 @@ MODULESTORE_CONFIG = mixed_store_config(settings.COMMON_TEST_DATA_ROOT, {}, incl @override_settings(MODULESTORE=MODULESTORE_CONFIG) +@patch.dict('django.conf.settings.FEATURES', {'ENABLE_PAID_COURSE_REGISTRATION': True}) class ShoppingCartViewsTests(ModuleStoreTestCase): def setUp(self): patcher = patch('student.models.tracker') @@ -963,8 +964,36 @@ class ShoppingCartViewsTests(ModuleStoreTestCase): ((template, _context), _tmp) = render_mock.call_args self.assertEqual(template, cert_item.single_item_receipt_template) + def _assert_404(self, url, use_post=False): + """ + Helper method to assert that a given url will return a 404 status code + """ + if use_post: + response = self.client.post(url) + else: + response = self.client.get(url) + self.assertEquals(response.status_code, 404) + + @patch.dict('django.conf.settings.FEATURES', {'ENABLE_PAID_COURSE_REGISTRATION': False}) + def test_disabled_paid_courses(self): + """ + Assert that the pages that require ENABLE_PAID_COURSE_REGISTRATION=True return a + HTTP 404 status code when we have this flag turned off + """ + self.login_user() + self._assert_404(reverse('shoppingcart.views.show_cart', args=[])) + self._assert_404(reverse('shoppingcart.views.clear_cart', args=[])) + self._assert_404(reverse('shoppingcart.views.remove_item', args=[]), use_post=True) + self._assert_404(reverse('shoppingcart.views.register_code_redemption', args=["testing"])) + self._assert_404(reverse('shoppingcart.views.use_code', args=[]), use_post=True) + self._assert_404(reverse('shoppingcart.views.update_user_cart', args=[])) + self._assert_404(reverse('shoppingcart.views.reset_code_redemption', args=[]), use_post=True) + self._assert_404(reverse('shoppingcart.views.billing_details', args=[])) + self._assert_404(reverse('shoppingcart.views.register_courses', args=[])) + @override_settings(MODULESTORE=MODULESTORE_CONFIG) +@patch.dict('django.conf.settings.FEATURES', {'ENABLE_PAID_COURSE_REGISTRATION': True}) class RegistrationCodeRedemptionCourseEnrollment(ModuleStoreTestCase): """ Test suite for RegistrationCodeRedemption Course Enrollments diff --git a/lms/djangoapps/shoppingcart/urls.py b/lms/djangoapps/shoppingcart/urls.py index 02776da0a0..2ae1f408a4 100644 --- a/lms/djangoapps/shoppingcart/urls.py +++ b/lms/djangoapps/shoppingcart/urls.py @@ -6,23 +6,19 @@ urlpatterns = patterns('shoppingcart.views', # nopep8 url(r'^receipt/(?P[0-9]*)/$', 'show_receipt'), url(r'^donation/$', 'donate', name='donation'), url(r'^csv_report/$', 'csv_report', name='payment_csv_report'), + # These following URLs are only valid if the ENABLE_SHOPPING_CART feature flag is set + url(r'^$', 'show_cart'), + url(r'^clear/$', 'clear_cart'), + url(r'^remove_item/$', 'remove_item'), + url(r'^add/course/{}/$'.format(settings.COURSE_ID_PATTERN), 'add_course_to_cart', name='add_course_to_cart'), + url(r'^register/redeem/(?P[0-9A-Za-z]+)/$', 'register_code_redemption', name='register_code_redemption'), + url(r'^use_code/$', 'use_code'), + url(r'^update_user_cart/$', 'update_user_cart'), + url(r'^reset_code_redemption/$', 'reset_code_redemption'), + url(r'^billing_details/$', 'billing_details', name='billing_details'), + url(r'^register_courses/$', 'register_courses'), ) -if settings.FEATURES['ENABLE_SHOPPING_CART']: - urlpatterns += patterns( - 'shoppingcart.views', - url(r'^$', 'show_cart'), - url(r'^clear/$', 'clear_cart'), - url(r'^remove_item/$', 'remove_item'), - url(r'^add/course/{}/$'.format(settings.COURSE_ID_PATTERN), 'add_course_to_cart', name='add_course_to_cart'), - url(r'^register/redeem/(?P[0-9A-Za-z]+)/$', 'register_code_redemption', name='register_code_redemption'), - url(r'^use_code/$', 'use_code'), - url(r'^update_user_cart/$', 'update_user_cart'), - url(r'^reset_code_redemption/$', 'reset_code_redemption'), - url(r'^billing_details/$', 'billing_details', name='billing_details'), - url(r'^register_courses/$', 'register_courses'), - ) - if settings.FEATURES.get('ENABLE_PAYMENT_FAKE'): from shoppingcart.tests.payment_fake import PaymentFakeView urlpatterns += patterns( diff --git a/lms/djangoapps/shoppingcart/utils.py b/lms/djangoapps/shoppingcart/utils.py new file mode 100644 index 0000000000..de469f29be --- /dev/null +++ b/lms/djangoapps/shoppingcart/utils.py @@ -0,0 +1,24 @@ +""" +Utility methods for the Shopping Cart app +""" + +from django.conf import settings +from microsite_configuration import microsite + + +def is_shopping_cart_enabled(): + """ + Utility method to check the various configuration to verify that + all of the settings have been enabled + """ + enable_paid_course_registration = microsite.get_value( + 'ENABLE_PAID_COURSE_REGISTRATION', + settings.FEATURES.get('ENABLE_PAID_COURSE_REGISTRATION') + ) + + enable_shopping_cart = microsite.get_value( + 'ENABLE_SHOPPING_CART', + settings.FEATURES.get('ENABLE_SHOPPING_CART') + ) + + return (enable_paid_course_registration and enable_shopping_cart) diff --git a/lms/djangoapps/shoppingcart/views.py b/lms/djangoapps/shoppingcart/views.py index 53c72219ab..aa1985f95f 100644 --- a/lms/djangoapps/shoppingcart/views.py +++ b/lms/djangoapps/shoppingcart/views.py @@ -33,7 +33,7 @@ from .exceptions import ( ) from .models import ( Order, OrderTypes, - PaidCourseRegistration, OrderItem, Coupon, CourseRegCodeItem, + PaidCourseRegistration, OrderItem, Coupon, CouponRedemption, CourseRegistrationCode, RegistrationCodeRedemption, Donation, DonationConfiguration ) @@ -44,6 +44,7 @@ from .processors import ( import json from xmodule_django.models import CourseKeyField +from .decorators import enforce_shopping_cart_enabled log = logging.getLogger("shoppingcart") AUDIT_LOG = logging.getLogger("audit") @@ -94,6 +95,7 @@ def add_course_to_cart(request, course_id): @login_required +@enforce_shopping_cart_enabled def update_user_cart(request): """ when user change the number-of-students from the UI then @@ -127,6 +129,7 @@ def update_user_cart(request): @login_required +@enforce_shopping_cart_enabled def show_cart(request): """ This view shows cart items. @@ -158,6 +161,7 @@ def show_cart(request): @login_required +@enforce_shopping_cart_enabled def clear_cart(request): cart = Order.get_cart_for_user(request.user) cart.clear() @@ -175,6 +179,7 @@ def clear_cart(request): @login_required +@enforce_shopping_cart_enabled def remove_item(request): """ This will remove an item from the user cart and also delete the corresponding coupon codes redemption. @@ -227,6 +232,7 @@ def remove_code_redemption(order_item_course_id, item_id, item, user): @login_required +@enforce_shopping_cart_enabled def reset_code_redemption(request): """ This method reset the code redemption from user cart items. @@ -239,6 +245,7 @@ def reset_code_redemption(request): @login_required +@enforce_shopping_cart_enabled def use_code(request): """ This method may generate the discount against valid coupon code @@ -291,6 +298,7 @@ def get_reg_code_validity(registration_code, request, limiter): @require_http_methods(["GET", "POST"]) @login_required +@enforce_shopping_cart_enabled def register_code_redemption(request, registration_code): """ This view allows the student to redeem the registration code @@ -382,6 +390,7 @@ def use_coupon_code(coupons, user): @login_required +@enforce_shopping_cart_enabled def register_courses(request): """ This method enroll the user for available course(s) @@ -518,6 +527,7 @@ def postpay_callback(request): @require_http_methods(["GET", "POST"]) @login_required +@enforce_shopping_cart_enabled def billing_details(request): """ This is the view for capturing additional billing details From 9270c7c77d57631da64ece358a3f8610d97187cb Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 10 Nov 2014 15:20:35 -0500 Subject: [PATCH 22/48] Make OAuth token login endpoint CSRF exempt --- common/djangoapps/student/views.py | 1 + 1 file changed, 1 insertion(+) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index c51f995012..177c37dc84 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -1114,6 +1114,7 @@ def login_user(request, error=""): # pylint: disable-msg=too-many-statements,un }) # TODO: this should be status code 400 # pylint: disable=fixme +@csrf_exempt @require_POST @social_utils.strategy("social:complete") def login_oauth_token(request, backend): From 3816ac2ab0d3b9241e795e2b48f9ba8c354c0c19 Mon Sep 17 00:00:00 2001 From: Carson Gee Date: Mon, 10 Nov 2014 14:50:07 -0500 Subject: [PATCH 23/48] Correct unresolved ugettext_lazy error strings in git_export command --- .../management/commands/git_export.py | 4 +-- .../commands/tests/test_git_export.py | 28 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/git_export.py b/cms/djangoapps/contentstore/management/commands/git_export.py index 1bc7f18337..5182127dca 100644 --- a/cms/djangoapps/contentstore/management/commands/git_export.py +++ b/cms/djangoapps/contentstore/management/commands/git_export.py @@ -62,7 +62,7 @@ class Command(BaseCommand): try: course_key = SlashSeparatedCourseKey.from_deprecated_string(args[0]) except InvalidKeyError: - raise CommandError(GitExportError.BAD_COURSE) + raise CommandError(_(GitExportError.BAD_COURSE)) try: git_export_utils.export_to_git( @@ -72,4 +72,4 @@ class Command(BaseCommand): options.get('rdir', None) ) except git_export_utils.GitExportError as ex: - raise CommandError(str(ex)) + raise CommandError(_(ex.message)) diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_git_export.py b/cms/djangoapps/contentstore/management/commands/tests/test_git_export.py index 5166e1aaf6..7c6c4965de 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_git_export.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_git_export.py @@ -80,6 +80,34 @@ class TestGitExport(CourseTestCase): stderr=StringIO.StringIO()) self.assertEqual(ex.exception.code, 1) + def test_error_output(self): + """ + Verify that error output is actually resolved as the correct string + """ + output = StringIO.StringIO() + with self.assertRaises(SystemExit): + with self.assertRaisesRegexp(CommandError, GitExportError.BAD_COURSE): + call_command( + 'git_export', 'foo/bar:baz', 'silly', + stdout=output, stderr=output + ) + self.assertIn('Bad course location provided', output.getvalue()) + output.close() + + output = StringIO.StringIO() + with self.assertRaises(SystemExit): + with self.assertRaisesRegexp(CommandError, GitExportError.URL_BAD): + call_command( + 'git_export', 'foo/bar/baz', 'silly', + stdout=output, stderr=output + ) + self.assertIn( + 'Non writable git url provided. Expecting something like:' + ' git@github.com:mitocw/edx4edx_lite.git', + output.getvalue() + ) + output.close() + def test_bad_git_url(self): """ Test several bad URLs for validation From 23081f076b27b427720adb80dd7b0ff28ac4be7c Mon Sep 17 00:00:00 2001 From: AlasdairSwan Date: Fri, 31 Oct 2014 09:33:17 -0400 Subject: [PATCH 24/48] ECOM-574 Updated track selection page to be responsive --- common/djangoapps/course_modes/views.py | 1 + .../js/spec_helpers/rwd_header_footer.js | 99 ++++++++ lms/envs/common.py | 5 + lms/static/images/verified-ribbon.png | Bin 0 -> 1791 bytes lms/static/sass/base/_grid-settings.scss | 14 ++ lms/static/sass/base/_variables.scss | 2 + lms/static/sass/shared/_footer.scss | 38 ++- lms/static/sass/shared/_header.scss | 220 ++++++++++++++++-- lms/static/sass/views/_verification.scss | 152 ++++++++---- lms/templates/main.html | 3 + lms/templates/navigation-edx.html | 14 +- .../verify_student/_verification_header.html | 6 + 12 files changed, 483 insertions(+), 71 deletions(-) create mode 100644 common/static/js/spec_helpers/rwd_header_footer.js create mode 100644 lms/static/images/verified-ribbon.png create mode 100644 lms/static/sass/base/_grid-settings.scss diff --git a/common/djangoapps/course_modes/views.py b/common/djangoapps/course_modes/views.py index d4147b6fa3..043e4e32d8 100644 --- a/common/djangoapps/course_modes/views.py +++ b/common/djangoapps/course_modes/views.py @@ -94,6 +94,7 @@ class ChooseModeView(View): "error": error, "upgrade": upgrade, "can_audit": "audit" in modes, + "responsive": True } if "verified" in modes: context["suggested_prices"] = [ diff --git a/common/static/js/spec_helpers/rwd_header_footer.js b/common/static/js/spec_helpers/rwd_header_footer.js new file mode 100644 index 0000000000..f68b97c8c7 --- /dev/null +++ b/common/static/js/spec_helpers/rwd_header_footer.js @@ -0,0 +1,99 @@ +/** + * Adds rwd classes and click handlers. + */ + +(function($) { + 'use strict'; + + var rwd = (function() { + + var _fn = { + header: 'header.global-new', + + footer: '.edx-footer-new', + + resultsUrl: 'course-search', + + init: function() { + _fn.$header = $( _fn.header ); + _fn.$footer = $( _fn.footer ); + _fn.$nav = _fn.$header.find('nav'); + _fn.$globalNav = _fn.$nav.find('.nav-global'); + + _fn.add.elements(); + _fn.add.classes(); + _fn.eventHandlers.init(); + }, + + add: { + classes: function() { + // Add any RWD-specific classes + _fn.$header.addClass('rwd'); + _fn.$footer.addClass('rwd'); + }, + + elements: function() { + _fn.add.burger(); + _fn.add.registerLink(); + }, + + burger: function() { + _fn.$nav.prepend([ + '', + '', + '' + ].join('')); + }, + + registerLink: function() { + var $register = _fn.$nav.find('.cta-register'), + $li = {}, + $a = {}, + count = 0; + + // Add if register link is shown + if ( $register.length > 0 ) { + count = _fn.$globalNav.find('li').length + 1; + + // Create new li + $li = $('
  • '); + $li.addClass('desktop-hide nav-global-0' + count); + + // Clone register link and remove classes + $a = $register.clone(); + $a.removeClass(); + + // append to DOM + $a.appendTo( $li ); + _fn.$globalNav.append( $li ); + } + } + }, + + eventHandlers: { + init: function() { + _fn.eventHandlers.click(); + }, + + click: function() { + // Toggle menu + _fn.$nav.on( 'click', '.mobile-menu-button', _fn.toggleMenu ); + } + }, + + toggleMenu: function( event ) { + event.preventDefault(); + + _fn.$globalNav.toggleClass('show'); + } + }; + + return { + init: _fn.init + }; + })(); + + setTimeout( function() { + rwd.init(); + }, 100); +})(jQuery); diff --git a/lms/envs/common.py b/lms/envs/common.py index 97900cd92a..c238c3efcd 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1019,6 +1019,7 @@ main_vendor_js = base_vendor_js + [ dashboard_js = sorted(rooted_glob(PROJECT_ROOT / 'static', 'js/dashboard/**/*.js')) discussion_js = sorted(rooted_glob(COMMON_ROOT / 'static', 'coffee/src/discussion/**/*.js')) +rwd_header_footer_js = sorted(rooted_glob(PROJECT_ROOT / 'static', 'js/common_helpers/rwd_header_footer.js')) staff_grading_js = sorted(rooted_glob(PROJECT_ROOT / 'static', 'coffee/src/staff_grading/**/*.js')) open_ended_js = sorted(rooted_glob(PROJECT_ROOT / 'static', 'coffee/src/open_ended/**/*.js')) notes_js = sorted(rooted_glob(PROJECT_ROOT / 'static', 'coffee/src/notes/**/*.js')) @@ -1197,6 +1198,10 @@ PIPELINE_JS = { 'source_filenames': dashboard_js, 'output_filename': 'js/dashboard.js' }, + 'rwd_header_footer': { + 'source_filenames': rwd_header_footer_js, + 'output_filename': 'js/rwd_header_footer.js' + }, 'student_account': { 'source_filenames': student_account_js, 'output_filename': 'js/student_account.js' diff --git a/lms/static/images/verified-ribbon.png b/lms/static/images/verified-ribbon.png new file mode 100644 index 0000000000000000000000000000000000000000..56b4b04b5156026f3a4567ca5cdad209016c12f4 GIT binary patch literal 1791 zcmbVNc~BE)6yFd)l%TOVKs@3a$|0I#HwR=TKuicy491YQjGZEzWCIbB4V#t3Q~?d8 zSRD>KRVs>Y)sY#j0~Qr2qarwh%77!N)#5>|2Ud$XcvPz0aM=Fg^pEb$?)%>Nz2AH9 zd++yVH)~Rp0;bKL1^_@n@-n5ClCbCX^QFG*_|gVSu*k%8G7UG84wS$Exe?c6V6p|x z!n7D_T$k60#R335$E-^y)78rn18(7=9vhz1Vx!Oi5F78bp@tlc1oc>!*(&4w@zY5T zXg10?AB)ujwM~I#o0qu=EX|dwGq`dLQX?lm4vcjolmiP!qM*}~YqcXz8E4EdLaja9 zd=5BfLgvUg<4L8fHJ}0~Fi^}Ba}5F*0;8ooNGy^<&|(l4Ktetw;KNcb1RH0Z7F_!}D~UR}R(t5A z2PI}V5M~=`#;u^oBdW*Ok}?jJ>G%~aHnsY#W2=3lP*lnIPSnPScmlq~; zN=hh%0)cetBsSS=**J;YvvC`!P=JwY z)L^!H^o5@Oj9*uY5$1f%s3LF+I96c9{Eh@-P@^asMkQRagyM@RS|Z`1VgtrSg(hm> zU=YR##GDDd@&6Q#PbI|nFyvp1ncSji=vltyKc&2#Ak0dQ4MEMv6z@-&)YwHOE9E-p zHT~$DH}P#YU^%1Bd#lS>PYYVVZBsul^UnBwE z*ix!Z3TJnP?q#z^IyBEqBkMrL2_m+NeV9#?7L;MZie0LZs_-kq8S1K8} zGP{L3bsq0~za;c=4r}p?k$RVxcUjz$&ibca7h4}zihf(>E0$xvsX^0A*jrq_j+r?J zYdV(PR91&iJ96rW^5?g5E3&R#sMFk8IO5%}H;z1=$GF7nZi`9^SkWW?v3o;fP@AxK z*t2`VFxoBJ;eEJQ4IcFxDA4<|d>h#_O~J-& zNu)^-47KmS6Tu9oJ+Xorerx8=Xbx$3Pi#s@qkZ;7RJ7)Um))*xTf5()Kz~J_S(vqg=DAv0(INXHOyF)7GB0h*j|VrBBfH&)zT;4T-Z9hs=@mCH~iMl~+D; zltJkiQcU-eqalx{R^3}wej%%VRmHjbS2sGT=DeC;11S&Ip{1) zkN}==@A{>uH=rdcVL^W$In&)Ysw>&;SaGU29>|-@d`M5(GcTcq*4yDT5S1P{PkJ}O z#K@+2&_di4*>b_Y6aGa_zV~0;>sYgKy#~n%{ym@8>F~;9^w76HN(zP#HZ@m&*S~Pf zARBEzmI)xNFV(b2WLnYm@F+Ua)c|M%H(q5_HmUa@M8?2B-gChb*x?&TeRz;II- zGXlu-XQoWov1wsnXMi8M!y^bJ;oE(Fvv*%A-Z`@62K#4PS4)^~aCKqv&i!<8we_3+ suT)`oEo*jm#WW+?y%fP){Q)lq;6|Y7)o(sI;`t>dC#EV7EzR8YH+sdCZ~y=R literal 0 HcmV?d00001 diff --git a/lms/static/sass/base/_grid-settings.scss b/lms/static/sass/base/_grid-settings.scss new file mode 100644 index 0000000000..4e41d3ba82 --- /dev/null +++ b/lms/static/sass/base/_grid-settings.scss @@ -0,0 +1,14 @@ +@import "neat/neat-helpers"; // or "neat-helpers" when in Rails + +/* Change the grid settings */ +$max-width: 1200px; +/* Override the default global box-sizing */ +$border-box-sizing: false; + + + +/* Breakpoints */ +$mobile: new-breakpoint(max-width 320px 4); +$tablet: new-breakpoint(min-width 321px max-width 768px, 8); +$desktop: new-breakpoint(min-width 769px 12); +$xl-desktop: new-breakpoint(min-width 980px 12); diff --git a/lms/static/sass/base/_variables.scss b/lms/static/sass/base/_variables.scss index 4a149a134b..9f275e02a3 100644 --- a/lms/static/sass/base/_variables.scss +++ b/lms/static/sass/base/_variables.scss @@ -173,6 +173,7 @@ $m-blue-d1: #1790C7; $m-blue-d2: #1580B0; $m-blue-d3: #126F9A; $m-blue-d4: #0A4A67; +$m-blue-d5: #009EE7; $m-blue-t0: rgba($m-blue,0.125); $m-blue-t1: rgba($m-blue,0.25); $m-blue-t2: rgba($m-blue,0.50); @@ -423,6 +424,7 @@ $header-sans-serif: 'Open Sans', Arial, Helvetica, sans-serif; $msg-bg: $action-primary-bg; + // New Shopping Cart $dark-gray1: #4a4a4a; diff --git a/lms/static/sass/shared/_footer.scss b/lms/static/sass/shared/_footer.scss index 9a6e4c37b2..9afd513a8b 100644 --- a/lms/static/sass/shared/_footer.scss +++ b/lms/static/sass/shared/_footer.scss @@ -1,10 +1,14 @@ // Open edX: LMS footer // ==================== +@import '../base/grid-settings'; +@import 'neat/neat'; // lib - Neat + .wrapper-footer { box-shadow: 0 -1px 5px 0 rgba(0,0,0, 0.1); border-top: 1px solid tint($m-gray,50%); padding: 25px ($baseline/2) ($baseline*1.5) ($baseline/2); background: $footer-bg; + clear: both; footer { @include clearfix(); @@ -280,8 +284,6 @@ $edx-footer-bg-color: rgb(252,252,252); @extend %t-weight4; } } - - } .edx-footer-new { @@ -352,6 +354,7 @@ $edx-footer-bg-color: rgb(252,252,252); .footer-nav-title { @extend %edx-footer-title; + margin-top: $baseline; } .footer-nav-links { @@ -372,12 +375,14 @@ $edx-footer-bg-color: rgb(252,252,252); .footer-follow-title { @extend %edx-footer-title; + margin-top: $baseline; } .footer-follow-links { a { @extend %edx-footer-link; + margin-top: 20px; .icon, .copy { display: inline-block; @@ -397,4 +402,33 @@ $edx-footer-bg-color: rgb(252,252,252); } } } + + &.rwd { + @include box-sizing(border-box); + @include outer-container; + + &.wrapper-footer footer { + min-width: 0; + } + + .footer-about, + .footer-nav, + .footer-follow { + @include span-columns(12); + } + + @include media( $tablet ) { + } + + @include media( $desktop ) { + .footer-about { + @include span-columns(6); + } + + .footer-nav, + .footer-follow { + @include span-columns(3); + } + } + } } diff --git a/lms/static/sass/shared/_header.scss b/lms/static/sass/shared/_header.scss index 33f7b7771f..dd0bddeb3d 100644 --- a/lms/static/sass/shared/_header.scss +++ b/lms/static/sass/shared/_header.scss @@ -1,3 +1,6 @@ +@import '../base/grid-settings'; +@import 'neat/neat'; // lib - Neat + header.global { border-bottom: 1px solid $m-gray; box-shadow: 0 1px 5px 0 rgba(0,0,0, 0.1); @@ -317,7 +320,6 @@ header.global { .view-courses .nav-global-02, .view-schools .nav-global-03, .view-register .nav-global-04 { - a { text-decoration: none; color: $link-color !important; @@ -331,8 +333,10 @@ header.global { // CASE: marketing/course discovery header.global-new { @extend %ui-depth1; + /* Temp. fix until applied globally */ + @include box-sizing(border-box); + position: relative; - height: ($baseline*3.75); width: 100%; border-bottom: 4px solid $courseware-border-bottom-color; box-shadow: 0 1px 5px 0 rgba(0,0,0, 0.1); @@ -340,15 +344,16 @@ header.global-new { nav { @include clearfix(); + @include box-sizing(border-box); width: grid-width(12); - height: ($baseline*2); + height: 74px; margin: 0 auto; - padding: 18px ($baseline/2) 0; + padding: 17px 0; } h1.logo { float: left; - margin: -2px 39px 0px 0px; + margin: -2px 39px 0 10px; position: relative; a { @@ -560,7 +565,7 @@ header.global-new { } } - .nav-global { + %default-header-nav { margin-top: ($baseline/4); list-style: none; float: left; @@ -568,25 +573,21 @@ header.global-new { li, div { display: inline-block; - margin: 0 $baseline+1 0 0; + margin: 0; text-transform: uppercase; letter-spacing: 0 !important; - &:last-child { - margin-right: 0; - } - a { - border-bottom: 4px solid $header-bg; display:block; - padding: ($baseline/4); + padding: 3px 10px; font-size: 18px; - padding-bottom: ($baseline*1.25); - font-weight: 600; + line-height: 24px; + font-weight: 500; font-family: $header-sans-serif; color: $courseware-navigation-color; - &:hover, &:focus{ + &:hover, + &:focus { text-decoration: none; color: $courseware-hover-color; } @@ -594,25 +595,26 @@ header.global-new { } } + .nav-global { + @extend %default-header-nav; + } + .nav-courseware { - @extend .nav-global; + @extend %default-header-nav; float: right; div { display: inline-block; - margin: 0 21px 0 0; text-transform: uppercase; letter-spacing: 0!important; position: relative; - vertical-align: middle; &:last-child { - margin-right: 0; + margin-right: 10px; } a { &.nav-courseware-button { - padding: 5px 45px 5px 45px; border: 3px solid $courseware-button-border-color; border-radius: 5px; margin-top: -22px; @@ -628,6 +630,182 @@ header.global-new { } } } + + &.rwd { + nav { + max-width: 1180px; + width: 100%; + } + + .mobile-menu-button { + @extend %t-action1; + display: inline; + float: left; + text-decoration: none; + color: $m-gray; + margin-top: 9px; + + &:hover, + &:active, + &:focus { + text-decoration: none; + } + } + + .logo { + position: absolute; + width: 54px; + left: calc( 50% - 90px ); + top: 20px; + + img { + width: 54px; + } + } + + .nav-global, + .nav-courseware { + a { + @extend %t-action3; + + &.nav-courseware-button { + width: 86px; + text-align: center; + margin-top: -3px; + } + } + } + + .nav-global, + .nav-courseware-01 { + display: none; + } + + .nav-global { + position: absolute; + top: 73px; + left: calc( 50% - 160px ); + z-index: 1000; + width: 320px; + background: $m-blue-d3; + + &.show { + display: inline; + } + + a { + color: white; + padding: 10px; + font-weight: 300; + + &:hover, + &:focus { + background: $m-blue-d5; + color: white; + border-bottom: none; + } + } + + li { + display: block; + border-bottom: 1px solid $m-blue-d5; + } + } + + .nav-courseware { + display: inline; + + div:last-child { + margin-right: 0; + } + } + + @include media( 320px ) { + nav { + width: 320px; + } + } + + @include media( $desktop ) { + nav { + width: 100%; + } + + .mobile-menu-button { + display: none; + } + + .logo { + position: relative; + width: auto; + top: inherit; + left: inherit; + margin-left: 10px; + + img { + width: auto; + } + } + + .nav-global { + display: inline; + position: relative; + z-index: auto; + width: auto; + top: auto; + left: auto; + background: inherit; + + a { + color: $courseware-navigation-color; + padding: 3px 10px; + font-weight: 500; + + &:hover, + &:focus { + background: inherit; + color: $courseware-hover-color; + } + } + + li { + display: inline-block; + border-bottom: none; + } + } + + .nav-courseware { + div:last-child { + margin-right: 10px; + } + } + + .nav-courseware-01 { + display: inline-block; + } + + .desktop-hide { + display: none!important; + } + } + + @include media( $xl-desktop ) { + nav { + padding: 17px 10px; + } + + .nav-global, + .nav-courseware { + a { + font-size: 18px; + } + } + + .logo { + margin-left: 0; + } + } + } } .view-register header.global-new .cta-register { diff --git a/lms/static/sass/views/_verification.scss b/lms/static/sass/views/_verification.scss index 3945876888..b900784744 100644 --- a/lms/static/sass/views/_verification.scss +++ b/lms/static/sass/views/_verification.scss @@ -1,5 +1,7 @@ // lms - views - verification flow // ==================== +@import '../base/grid-settings'; +@import 'neat/neat'; // lib - Neat // MISC: extends - button %btn-verify-primary { @@ -12,7 +14,6 @@ .is-expandable { .title-expand { - } .expandable-icon { @@ -438,7 +439,6 @@ } } } - } } @@ -989,7 +989,7 @@ @extend %t-weight4; position: absolute; top: -($baseline*1.25); - left: 45%; + left: calc( 50% - 46px ); padding: ($baseline/2) ($baseline*1.5); background: white; text-align: center; @@ -1129,22 +1129,35 @@ } .content-supplementary { - width: flex-grid(12,12); + @include box-sizing(border-box); + @include outer-container; + @include span-columns(12); .list-help { @include clearfix(); .help-item { - width: flex-grid(4,12); + @include fill-parent; + float: left; margin-right: flex-gutter(); + margin-bottom: 25px; &:last-child { - margin-right: 0; + margin: 0; } + } + } - &.help-item-technical { - width: flex-grid(8,12); + @include media( 550px ) { + .list-help { + .help-item { + @include span-columns(4); + margin-bottom: 0; + + &.help-item-technical { + @include span-columns(8); + } } } } @@ -1154,6 +1167,10 @@ // VIEW: select a track &.step-select-track { + .container { + min-width: 0; + max-width: 1200px; + } .sts-track { @extend %text-sr; @@ -1161,11 +1178,10 @@ .form-register-choose { @include clearfix(); - width: flex-grid(12,12); margin: ($baseline*2) 0; .deco-divider { - width: flex-grid(12,12); + @include fill-parent; float: left; } } @@ -1175,7 +1191,7 @@ } .register-choice { - width: flex-grid(12,12); + @include fill-parent; margin: 0 flex-gutter() $baseline 0; border-top: ($baseline/4) solid $m-gray-d4; padding: $baseline ($baseline*1.5); @@ -1190,28 +1206,35 @@ vertical-align: middle; } - .wrapper-copy { - width: flex-grid(8,8); - } - .list-actions { - width: flex-grid(8,8); + @include fill-parent; text-align: right; + float: right; + margin: ($baseline/4) 0; + border-top: none; + clear: both; } .title { @extend %t-title5; @extend %t-weight5; margin-bottom: ($baseline/2); + width: calc( 100% - 30px ); } .copy { @extend %t-copy-base; } - .action-select input { - @extend %t-weight4; - padding: ($baseline/2) ($baseline*0.75); + .action-select { + @include fill-parent; + + input { + @extend %t-weight4; + padding: ($baseline/2) ($baseline*0.75); + width: 100%; + white-space: normal; + } } } @@ -1226,15 +1249,9 @@ display: block; width: ($baseline*2.9); height: ($baseline*4.2); - background: transparent url('../images/honor-ribbon.png') no-repeat 0 0; - } - - .wrapper-copy { - width: flex-grid(8,8); } .list-actions { - width: flex-grid(8,8); margin: ($baseline) 0; } @@ -1249,19 +1266,12 @@ .deco-ribbon { position: absolute; - top: -($baseline*1.5); + top: -10px; right: $baseline; display: block; - width: ($baseline*3); - height: ($baseline*4); - background: transparent url('../images/vcert-ribbon-s.png') no-repeat 0 0; - } - - .list-actions { - margin: ($baseline/4) 0; - border-top: none; - width: flex-grid(4,12); - float: right; + width: 45px; + height: 45px; + background: transparent url('../images/verified-ribbon.png') no-repeat 0 0; } .action-intro, .action-select { @@ -1270,15 +1280,11 @@ } .action-intro { + @include fill-parent; @extend %copy-detail; - width: flex-grid(3,8); text-align: left; } - .action-select { - width: initial; - } - .action-select input { @extend %btn-verify-primary; } @@ -1301,7 +1307,7 @@ } .help-register { - width: flex-grid(4,12); + @include span-columns(4); .title { @extend %hd-lv4; @@ -1333,8 +1339,8 @@ .contribution-options { @include clearfix(); + @include fill-parent; margin: 0; - width: flex-grid(8,12); &:after{ clear: none; @@ -1342,6 +1348,7 @@ } .field { + @include fill-parent; float: left; margin: 0 ($baseline/2) ($baseline/2) 0; padding: ($baseline/2) ($baseline*0.75); @@ -1380,6 +1387,65 @@ } } } + + @include media(min-width 550px max-width 768px) { + .contribution-options { + .field { + @include span-columns(6); + + &:nth-of-type(even) { + margin-right: 0; + } + } + } + + .register-choice { + .list-actions { + float: left; + width: auto; + } + + .action-select { + width: initial; + + input { + width: initial; + } + } + } + } + + @include media( $desktop ) { + .contribution-options { + .field { + width: auto; + } + } + + .register-choice { + .list-actions { + @include span-columns(4); + width: auto; + } + + .action-select { + width: initial; + + input { + width: initial; + } + } + } + } + + @include media( $xl-desktop ) { + .register-choice { + .list-actions { + float: right; + clear: none; + } + } + } } // VIEW: requirements diff --git a/lms/templates/main.html b/lms/templates/main.html index 51649afed1..0026085f3c 100644 --- a/lms/templates/main.html +++ b/lms/templates/main.html @@ -11,6 +11,9 @@ + % if responsive: + + % endif <%! from django.utils.translation import ugettext as _ %> <%! from microsite_configuration import microsite %> <%! from microsite_configuration import page_title_breadcrumbs %> diff --git a/lms/templates/navigation-edx.html b/lms/templates/navigation-edx.html index d5807a93a8..54cdfb7ba5 100644 --- a/lms/templates/navigation-edx.html +++ b/lms/templates/navigation-edx.html @@ -53,11 +53,15 @@ site_status_msg = get_site_status_msg(course_id) % if user.is_authenticated():
    <%block name="navigation_global_links_authenticated"> - % if settings.FEATURES.get('COURSES_ARE_BROWSABLE'): -
    - ${_('Find Courses')} -
    - % endif +
  • + ${_("How it Works")} +
  • +
  • + ${_("Find Courses")} +
  • +
  • + ${_("Schools & Partners")} +
  • diff --git a/lms/templates/verify_student/_verification_header.html b/lms/templates/verify_student/_verification_header.html index 8a1cdadac2..780fd6b00b 100644 --- a/lms/templates/verify_student/_verification_header.html +++ b/lms/templates/verify_student/_verification_header.html @@ -1,5 +1,7 @@ <%! from django.utils.translation import ugettext as _ %> +<%namespace name='static' file='../static_content.html'/> +

    @@ -38,3 +40,7 @@ % endif

    + +<%block name="js_extra"> + <%static:js group='rwd_header_footer'/> + From 1d85659b1332408b1e7d48b178b5a8edca3b5026 Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Mon, 10 Nov 2014 16:03:25 -0500 Subject: [PATCH 25/48] Prevent instructors from accidentally downgrading enrollment modes through instructor dash TNL-804 --- lms/djangoapps/instructor/enrollment.py | 8 +++-- lms/djangoapps/instructor/tests/test_api.py | 32 +++++++++++++++++++ .../courseware/instructor_dashboard.html | 23 +++---------- 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index f0198e92a5..1e9735ee11 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -26,9 +26,12 @@ class EmailEnrollmentState(object): exists_user = User.objects.filter(email=email).exists() if exists_user: user = User.objects.get(email=email) - exists_ce = CourseEnrollment.is_enrolled(user, course_id) + mode, is_active = CourseEnrollment.enrollment_mode_for_user(user, course_id) + # is_active is `None` if the user is not enrolled in the course + exists_ce = is_active is not None and is_active full_name = user.profile.name else: + mode = None exists_ce = False full_name = None ceas = CourseEnrollmentAllowed.objects.filter(course_id=course_id, email=email).all() @@ -40,6 +43,7 @@ class EmailEnrollmentState(object): self.allowed = exists_allowed self.auto_enroll = bool(state_auto_enroll) self.full_name = full_name + self.mode = mode def __repr__(self): return "{}(user={}, enrollment={}, allowed={}, auto_enroll={})".format( @@ -84,7 +88,7 @@ def enroll_email(course_id, student_email, auto_enroll=False, email_students=Fal previous_state = EmailEnrollmentState(course_id, student_email) if previous_state.user: - CourseEnrollment.enroll_by_email(student_email, course_id) + CourseEnrollment.enroll_by_email(student_email, course_id, previous_state.mode) if email_students: email_params['message'] = 'enrolled_enroll' email_params['email_address'] = student_email diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index feda24b885..50d2bf2545 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -1050,6 +1050,38 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): ) ) + def test_enroll_already_enrolled_student(self): + """ + Ensure that already enrolled "verified" students cannot be downgraded + to "honor" + """ + course_enrollment = CourseEnrollment.objects.get( + user=self.enrolled_student, course_id=self.course.id + ) + # make this enrollment "verified" + course_enrollment.mode = u'verified' + course_enrollment.save() + self.assertEqual(course_enrollment.mode, u'verified') + + # now re-enroll the student through the instructor dash + url = reverse( + 'students_update_enrollment', + kwargs={'course_id': self.course.id.to_deprecated_string()}, + ) + params = { + 'identifiers': self.enrolled_student.email, + 'action': 'enroll', + 'email_students': True, + } + response = self.client.post(url, params) + self.assertEqual(response.status_code, 200) + + # affirm that the student is still in "verified" mode + course_enrollment = CourseEnrollment.objects.get( + user=self.enrolled_student, course_id=self.course.id + ) + self.assertEqual(course_enrollment.mode, u"verified") + @ddt.ddt @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) diff --git a/lms/templates/courseware/instructor_dashboard.html b/lms/templates/courseware/instructor_dashboard.html index 606b5fbc01..4439fc6ba7 100644 --- a/lms/templates/courseware/instructor_dashboard.html +++ b/lms/templates/courseware/instructor_dashboard.html @@ -192,10 +192,8 @@ function goto( mode)

    ${_("Note: some of these buttons are known to time out for larger " - "courses. We have temporarily disabled those features for courses " - "with more than {max_enrollment} students. We are urgently working on " - "fixing this issue. Thank you for your patience as we continue " - "working to improve the platform!").format( + "courses. We have disabled those features for courses " + "with more than {max_enrollment} students.").format( max_enrollment=settings.FEATURES['MAX_ENROLLMENT_INSTR_BUTTONS'] )}

    @@ -428,10 +426,8 @@ function goto( mode)

    ${_("Note: some of these buttons are known to time out for larger " - "courses. We have temporarily disabled those features for courses " - "with more than {max_enrollment} students. We are urgently working on " - "fixing this issue. Thank you for your patience as we continue " - "working to improve the platform!").format( + "courses. We have disabled those features for courses " + "with more than {max_enrollment} students.").format( max_enrollment=settings.FEATURES['MAX_ENROLLMENT_INSTR_BUTTONS'] )}

    @@ -461,17 +457,6 @@ function goto( mode)
    %endif -

    ${_("Batch Enrollment")}

    -

    ${_("Enroll or un-enroll one or many students: enter emails, separated by new lines or commas;")}

    - -

    - ${_("Notify students by email")} -

    - ${_("Auto-enroll students when they activate")} - -

    - - %endif ##----------------------------------------------------------------------------- From db8dd8de64842429c19c06a64e48d50aa2e6bc4b Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 10 Nov 2014 18:05:41 -0500 Subject: [PATCH 26/48] Fix OAuth token login endpoint to set session user --- common/djangoapps/student/tests/test_login.py | 1 + common/djangoapps/student/views.py | 1 + 2 files changed, 2 insertions(+) diff --git a/common/djangoapps/student/tests/test_login.py b/common/djangoapps/student/tests/test_login.py index b35be740bf..eb3183a4a6 100644 --- a/common/djangoapps/student/tests/test_login.py +++ b/common/djangoapps/student/tests/test_login.py @@ -485,6 +485,7 @@ class LoginOAuthTokenMixin(object): self._setup_user_response(success=True) response = self.client.post(self.url, {"access_token": "dummy"}) self.assertEqual(response.status_code, 204) + self.assertEqual(self.client.session['_auth_user_id'], self.user.id) def test_invalid_token(self): self._setup_user_response(success=False) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 177c37dc84..0ebf4971dd 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -1135,6 +1135,7 @@ def login_oauth_token(request, backend): pass # do_auth can return a non-User object if it fails if user and isinstance(user, User): + login(request, user) return JsonResponse(status=204) else: # Ensure user does not re-enter the pipeline From 0fdf8fb2228993f409f84ff19f3030193d1f8e45 Mon Sep 17 00:00:00 2001 From: stv Date: Sat, 8 Nov 2014 20:35:42 -0800 Subject: [PATCH 27/48] Fix PEP8: E201 whitespace after '{' --- common/djangoapps/terrain/browser.py | 4 +++- common/djangoapps/terrain/stubs/tests/test_http.py | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/common/djangoapps/terrain/browser.py b/common/djangoapps/terrain/browser.py index 5fa2ba4951..6464b6e9a3 100644 --- a/common/djangoapps/terrain/browser.py +++ b/common/djangoapps/terrain/browser.py @@ -95,7 +95,9 @@ def initial_setup(server): if browser_driver == 'chrome': desired_capabilities = DesiredCapabilities.CHROME - desired_capabilities['loggingPrefs'] = { 'browser':'ALL' } + desired_capabilities['loggingPrefs'] = { + 'browser': 'ALL', + } elif browser_driver == 'firefox': desired_capabilities = DesiredCapabilities.FIREFOX else: diff --git a/common/djangoapps/terrain/stubs/tests/test_http.py b/common/djangoapps/terrain/stubs/tests/test_http.py index ba0769ac45..fb768deabc 100644 --- a/common/djangoapps/terrain/stubs/tests/test_http.py +++ b/common/djangoapps/terrain/stubs/tests/test_http.py @@ -25,7 +25,9 @@ class StubHttpServiceTest(unittest.TestCase): 'test_empty': '', 'test_int': 12345, 'test_float': 123.45, - 'test_dict': { 'test_key': 'test_val' }, + 'test_dict': { + 'test_key': 'test_val', + }, 'test_empty_dict': {}, 'test_unicode': u'\u2603 the snowman', 'test_none': None, From 01960483590b6fd33d80c4865221e340be207a11 Mon Sep 17 00:00:00 2001 From: stv Date: Sat, 8 Nov 2014 20:37:01 -0800 Subject: [PATCH 28/48] Fix PEP8: E202 whitespace before '}' --- common/djangoapps/student/tests/test_course_listing.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/student/tests/test_course_listing.py b/common/djangoapps/student/tests/test_course_listing.py index 13d838715e..b73067cbd0 100644 --- a/common/djangoapps/student/tests/test_course_listing.py +++ b/common/djangoapps/student/tests/test_course_listing.py @@ -105,7 +105,10 @@ class TestCourseListing(ModuleStoreTestCase): course_location = SlashSeparatedCourseKey('testOrg', 'erroredCourse', 'RunBabyRun') course = self._create_course_with_access_groups(course_location) course_db_record = mongo_store._find_one(course.location) - course_db_record.setdefault('metadata', {}).get('tabs', []).append({"type": "wiko", "name": "Wiki" }) + course_db_record.setdefault('metadata', {}).get('tabs', []).append({ + "type": "wiko", + "name": "Wiki", + }) mongo_store.collection.update( {'_id': course.location.to_deprecated_son()}, {'$set': { From 02e224df1f9a894bd6e26bb5f1b1997ba0f2bd0c Mon Sep 17 00:00:00 2001 From: stv Date: Sat, 8 Nov 2014 20:24:36 -0800 Subject: [PATCH 29/48] Fix PEP8: E203 whitespace before ':' --- .../open_ended_grading_classes/peer_grading_service.py | 9 ++++++++- common/lib/xmodule/xmodule/tests/__init__.py | 8 +++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py index 0b72ad7b6f..bd6cf0ef2c 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py @@ -160,4 +160,11 @@ class MockPeerGradingService(object): ]} def get_data_for_location(self, problem_location, student_id): - return {"version": 1, "count_graded": 3, "count_required": 3, "success": True, "student_sub_count": 1, 'submissions_available' : 0} + return { + "version": 1, + "count_graded": 3, + "count_required": 3, + "success": True, + "student_sub_count": 1, + 'submissions_available': 0, + } diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 960f57c16f..4c8ff94f76 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -89,7 +89,13 @@ def get_test_system(course_id=SlashSeparatedCourseKey('org', 'course', 'run')): filestore=Mock(), debug=True, hostname="edx.org", - xqueue={'interface': None, 'callback_url': '/', 'default_queuename': 'testqueue', 'waittime': 10, 'construct_callback' : Mock(side_effect="/")}, + xqueue={ + 'interface': None, + 'callback_url': '/', + 'default_queuename': 'testqueue', + 'waittime': 10, + 'construct_callback': Mock(side_effect="/"), + }, node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules"), anonymous_student_id='student', open_ended_grading_interface=open_ended_grading_interface, From 96ce219d80d63509d5daade26fd8a30053952961 Mon Sep 17 00:00:00 2001 From: stv Date: Sat, 8 Nov 2014 20:39:01 -0800 Subject: [PATCH 30/48] Fix PEP8: E211 whitespace before '(' --- common/djangoapps/terrain/browser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/djangoapps/terrain/browser.py b/common/djangoapps/terrain/browser.py index 6464b6e9a3..75ad8aa984 100644 --- a/common/djangoapps/terrain/browser.py +++ b/common/djangoapps/terrain/browser.py @@ -241,7 +241,7 @@ def capture_console_log(scenario): output_dir = '{}/log'.format(settings.TEST_ROOT) file_name = '{}/{}.log'.format(output_dir, scenario.name.replace(' ', '_')) - with open (file_name, 'w') as output_file: + with open(file_name, 'w') as output_file: for line in log: output_file.write("{}{}".format(dumps(line), '\n')) From 1a9f8b61f9393aa9a52d9dc4e8eec90f0a0871a9 Mon Sep 17 00:00:00 2001 From: stv Date: Sat, 8 Nov 2014 21:04:26 -0800 Subject: [PATCH 31/48] Fix PEP8: E221 multiple spaces before operator --- lms/djangoapps/instructor/tests/test_legacy_xss.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/instructor/tests/test_legacy_xss.py b/lms/djangoapps/instructor/tests/test_legacy_xss.py index bd0e75f7f7..310e9c3eb9 100644 --- a/lms/djangoapps/instructor/tests/test_legacy_xss.py +++ b/lms/djangoapps/instructor/tests/test_legacy_xss.py @@ -45,7 +45,7 @@ class TestXss(ModuleStoreTestCase): Build a request with the given action, call the instructor dashboard view, and check that HTML code in a user's name is properly escaped. """ - req = self._request_factory.post( + req = self._request_factory.post( "dummy_url", data={"action": action} ) From 41390f0239514aa553ba1a32c20be9cad183914f Mon Sep 17 00:00:00 2001 From: stv Date: Sat, 8 Nov 2014 20:26:09 -0800 Subject: [PATCH 32/48] Fix PEP8: E222 multiple spaces after operator --- common/lib/xmodule/xmodule/split_test_module.py | 2 +- lms/djangoapps/shoppingcart/processors/CyberSource2.py | 2 +- pavelib/utils/test/suites/acceptance_suite.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/split_test_module.py b/common/lib/xmodule/xmodule/split_test_module.py index fdd4629a9a..3dd59bfd68 100644 --- a/common/lib/xmodule/xmodule/split_test_module.py +++ b/common/lib/xmodule/xmodule/split_test_module.py @@ -26,7 +26,7 @@ log = logging.getLogger('edx.' + __name__) # Make '_' a no-op so we can scrape strings _ = lambda text: text -DEFAULT_GROUP_NAME = _(u'Group ID {group_id}') +DEFAULT_GROUP_NAME = _(u'Group ID {group_id}') class SplitTestFields(object): diff --git a/lms/djangoapps/shoppingcart/processors/CyberSource2.py b/lms/djangoapps/shoppingcart/processors/CyberSource2.py index 1d66138a96..2794e03448 100644 --- a/lms/djangoapps/shoppingcart/processors/CyberSource2.py +++ b/lms/djangoapps/shoppingcart/processors/CyberSource2.py @@ -288,7 +288,7 @@ def get_purchase_params(cart, callback_url=None, extra_data=None): params['transaction_type'] = 'sale' params['locale'] = 'en' - params['signed_date_time'] = datetime.utcnow().strftime('%Y-%m-%dT%H:%M:%SZ') + params['signed_date_time'] = datetime.utcnow().strftime('%Y-%m-%dT%H:%M:%SZ') params['signed_field_names'] = 'access_key,profile_id,amount,currency,transaction_type,reference_number,signed_date_time,locale,transaction_uuid,signed_field_names,unsigned_field_names,orderNumber' params['unsigned_field_names'] = '' params['transaction_uuid'] = uuid.uuid4().hex diff --git a/pavelib/utils/test/suites/acceptance_suite.py b/pavelib/utils/test/suites/acceptance_suite.py index d33ee744fe..377b753836 100644 --- a/pavelib/utils/test/suites/acceptance_suite.py +++ b/pavelib/utils/test/suites/acceptance_suite.py @@ -33,7 +33,7 @@ class AcceptanceTest(TestSuite): @property def cmd(self): - report_file = self.report_dir / "{}.xml".format(self.system) + report_file = self.report_dir / "{}.xml".format(self.system) report_args = "--with-xunit --xunit-file {}".format(report_file) cmd = ( From 00ae488960e4044c21a8867b0786ecdec0b4ec3f Mon Sep 17 00:00:00 2001 From: stv Date: Sat, 8 Nov 2014 20:23:00 -0800 Subject: [PATCH 33/48] Fix PEP8: E225 missing whitespace around operator --- cms/envs/test.py | 4 ++-- common/djangoapps/terrain/stubs/ora.py | 2 +- common/lib/capa/capa/safe_exec/safe_exec.py | 2 +- .../open_ended_grading_classes/combined_open_ended_rubric.py | 4 ++-- common/lib/xmodule/xmodule/tests/test_video.py | 4 ++-- lms/djangoapps/courseware/masquerade.py | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cms/envs/test.py b/cms/envs/test.py index f06140c5f8..36b9e54e29 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -69,9 +69,9 @@ STATICFILES_DIRS += [ # If we don't add these settings, then Django templates that can't # find pipelined assets will raise a ValueError. # http://stackoverflow.com/questions/12816941/unit-testing-with-django-pipeline -STATICFILES_STORAGE='pipeline.storage.NonPackagingPipelineStorage' +STATICFILES_STORAGE = 'pipeline.storage.NonPackagingPipelineStorage' STATIC_URL = "/static/" -PIPELINE_ENABLED=False +PIPELINE_ENABLED = False # Update module store settings per defaults for tests update_module_store_settings( diff --git a/common/djangoapps/terrain/stubs/ora.py b/common/djangoapps/terrain/stubs/ora.py index 1edf93c484..8efc5be4aa 100644 --- a/common/djangoapps/terrain/stubs/ora.py +++ b/common/djangoapps/terrain/stubs/ora.py @@ -45,7 +45,7 @@ class StudentState(object): @property def num_pending(self): - return max(self.INITIAL_ESSAYS_AVAILABLE- self.num_graded, 0) + return max(self.INITIAL_ESSAYS_AVAILABLE - self.num_graded, 0) @property def num_required(self): diff --git a/common/lib/capa/capa/safe_exec/safe_exec.py b/common/lib/capa/capa/safe_exec/safe_exec.py index b25f7b47a2..b57afbcd0d 100644 --- a/common/lib/capa/capa/safe_exec/safe_exec.py +++ b/common/lib/capa/capa/safe_exec/safe_exec.py @@ -21,7 +21,7 @@ random.Random = random_module.Random sys.modules['random'] = random """ -ASSUMED_IMPORTS=[ +ASSUMED_IMPORTS = [ ("numpy", "numpy"), ("math", "math"), ("scipy", "scipy"), diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_rubric.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_rubric.py index 0a4641f66d..de90378f6c 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_rubric.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_rubric.py @@ -236,7 +236,7 @@ class CombinedOpenEndedRubric(object): rubric_categories[i]['options'][j]['grader_types'].append(grader_type) #Grab the score and add it to the actual scores. J will be the score for the selected #grader type - if len(actual_scores)<=i: + if len(actual_scores) <= i: #Initialize a new list in the list of lists actual_scores.append([j]) else: @@ -249,7 +249,7 @@ class CombinedOpenEndedRubric(object): for (i, a) in enumerate(actual_scores): if int(a) == max_scores[i]: correct.append(1) - elif int(a)==0: + elif int(a) == 0: correct.append(0) else: correct.append(.5) diff --git a/common/lib/xmodule/xmodule/tests/test_video.py b/common/lib/xmodule/xmodule/tests/test_video.py index 8763c15225..f27cb6c2dc 100644 --- a/common/lib/xmodule/xmodule/tests/test_video.py +++ b/common/lib/xmodule/xmodule/tests/test_video.py @@ -566,7 +566,7 @@ class VideoCdnTest(unittest.TestCase): original_video_url = "http://www.original_video.com/original_video.mp4" cdn_response_video_url = "http://www.cdn_video.com/cdn_video.mp4" cdn_response_content = '{{"sources":["{cdn_url}"]}}'.format(cdn_url=cdn_response_video_url) - cdn_response.return_value=Mock(status_code=200, content=cdn_response_content) + cdn_response.return_value = Mock(status_code=200, content=cdn_response_content) fake_cdn_url = 'http://fake_cdn.com/' self.assertEqual( get_video_from_cdn(fake_cdn_url, original_video_url), @@ -579,6 +579,6 @@ class VideoCdnTest(unittest.TestCase): Test if no alternative video in CDN exists. """ original_video_url = "http://www.original_video.com/original_video.mp4" - cdn_response.return_value=Mock(status_code=404) + cdn_response.return_value = Mock(status_code=404) fake_cdn_url = 'http://fake_cdn.com/' self.assertIsNone(get_video_from_cdn(fake_cdn_url, original_video_url)) diff --git a/lms/djangoapps/courseware/masquerade.py b/lms/djangoapps/courseware/masquerade.py index a044e89d91..41a868cc9b 100644 --- a/lms/djangoapps/courseware/masquerade.py +++ b/lms/djangoapps/courseware/masquerade.py @@ -62,4 +62,4 @@ def is_masquerading_as_student(user): Return True if user is masquerading as a student, False otherwise ''' masq = getattr(user, 'masquerade_as_student', False) - return masq==True + return masq is True From b388ab9db066df4d5c3009bdc3ad34e3246f7962 Mon Sep 17 00:00:00 2001 From: stv Date: Sat, 8 Nov 2014 17:24:01 -0800 Subject: [PATCH 34/48] Fix PEP8: E226 missing whitespace around arithmetic operator --- common/lib/xmodule/xmodule/lti_module.py | 2 +- common/test/acceptance/pages/lms/annotation_component.py | 2 +- common/test/data/uploads/python_lib_zip/number_helpers.py | 2 +- lms/djangoapps/courseware/tests/test_navigation.py | 2 +- scripts/cov_merge.py | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py index 2cf31195e6..85d4fb9300 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/common/lib/xmodule/xmodule/lti_module.py @@ -773,7 +773,7 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'} imsx_messageIdentifier = root.xpath("//def:imsx_messageIdentifier", namespaces=namespaces)[0].text or '' sourcedId = root.xpath("//def:sourcedId", namespaces=namespaces)[0].text score = root.xpath("//def:textString", namespaces=namespaces)[0].text - action = root.xpath("//def:imsx_POXBody", namespaces=namespaces)[0].getchildren()[0].tag.replace('{'+lti_spec_namespace+'}', '') + action = root.xpath("//def:imsx_POXBody", namespaces=namespaces)[0].getchildren()[0].tag.replace('{' + lti_spec_namespace + '}', '') # Raise exception if score is not float or not in range 0.0-1.0 regarding spec. score = float(score) if not 0 <= score <= 1: diff --git a/common/test/acceptance/pages/lms/annotation_component.py b/common/test/acceptance/pages/lms/annotation_component.py index ad8ac3fd20..a1e31fc520 100644 --- a/common/test/acceptance/pages/lms/annotation_component.py +++ b/common/test/acceptance/pages/lms/annotation_component.py @@ -41,7 +41,7 @@ class AnnotationComponentPage(PageObject): Return css selector for current active problem with sub_selector. """ return 'div[data-problem-id="{}"] {}'.format( - self.q(css='.vert-{}'.format(self.active_problem+1)).map( + self.q(css='.vert-{}'.format(self.active_problem + 1)).map( lambda el: el.get_attribute('data-id')).results[0], sub_selector, ) diff --git a/common/test/data/uploads/python_lib_zip/number_helpers.py b/common/test/data/uploads/python_lib_zip/number_helpers.py index 04db8e9bfe..8cc3c93804 100644 --- a/common/test/data/uploads/python_lib_zip/number_helpers.py +++ b/common/test/data/uploads/python_lib_zip/number_helpers.py @@ -3,4 +3,4 @@ def seventeen(): def fortytwo(x): - return 42+x + return 42 + x diff --git a/lms/djangoapps/courseware/tests/test_navigation.py b/lms/djangoapps/courseware/tests/test_navigation.py index 8d97567c5d..6b5dbe41a3 100644 --- a/lms/djangoapps/courseware/tests/test_navigation.py +++ b/lms/djangoapps/courseware/tests/test_navigation.py @@ -79,7 +79,7 @@ class TestNavigation(ModuleStoreTestCase, LoginEnrollmentTestCase): ''' Check if the progress tab is active in the tab set ''' for line in response.content.split('\n'): if tabname in line and 'active' in line: - raise AssertionError("assertTabInactive failed: "+tabname+" active") + raise AssertionError("assertTabInactive failed: " + tabname + " active") return def test_chrome_settings(self): diff --git a/scripts/cov_merge.py b/scripts/cov_merge.py index 8b4cd1c6d5..02632d14be 100644 --- a/scripts/cov_merge.py +++ b/scripts/cov_merge.py @@ -119,7 +119,7 @@ class ReportMerge(object): report_path = os.path.join(self.DESTINATION, output_file) else: report_filename = path.split('reports/')[1].split('/cover')[0].replace('/', '_') - report_path = os.path.join(self.DESTINATION, report_filename+'_coverage.html') + report_path = os.path.join(self.DESTINATION, report_filename + '_coverage.html') # Write everything to single report file with open(report_path, 'w') as report_file: From 500499b6783ea7e870b773a175250bdb596c02f7 Mon Sep 17 00:00:00 2001 From: stv Date: Sat, 8 Nov 2014 20:18:29 -0800 Subject: [PATCH 35/48] Fix PEP8: E231 missing whitespace after '[:,]' --- cms/djangoapps/contentstore/views/preview.py | 2 +- .../templatetags/microsite.py | 2 +- common/djangoapps/terrain/stubs/http.py | 4 ++-- .../terrain/stubs/tests/test_lti_stub.py | 2 +- common/djangoapps/terrain/stubs/xqueue.py | 3 ++- .../sandbox-packages/verifiers/draganddrop.py | 16 ++++++++-------- .../peer_grading_service.py | 2 +- .../self_assessment_module.py | 2 +- .../courseware/features/high-level-tabs.py | 2 +- lms/djangoapps/notification_prefs/tests.py | 4 ++-- lms/djangoapps/notification_prefs/views.py | 2 +- .../open_ended_grading/staff_grading_service.py | 2 +- lms/djangoapps/staticbook/views.py | 2 +- 13 files changed, 23 insertions(+), 22 deletions(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index a16fad00a9..3e292827b1 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -151,7 +151,7 @@ def _preview_module_system(request, descriptor): replace_urls=partial(static_replace.replace_static_urls, data_directory=None, course_id=course_id), user=request.user, can_execute_unsafe_code=(lambda: can_execute_unsafe_code(course_id)), - get_python_lib_zip=(lambda :get_python_lib_zip(contentstore, course_id)), + get_python_lib_zip=(lambda: get_python_lib_zip(contentstore, course_id)), mixins=settings.XBLOCK_MIXINS, course_id=course_id, anonymous_student_id='student', diff --git a/common/djangoapps/microsite_configuration/templatetags/microsite.py b/common/djangoapps/microsite_configuration/templatetags/microsite.py index 1aea3c4944..b349f66a87 100644 --- a/common/djangoapps/microsite_configuration/templatetags/microsite.py +++ b/common/djangoapps/microsite_configuration/templatetags/microsite.py @@ -43,7 +43,7 @@ def platform_name(): @register.simple_tag(name="favicon_path") -def favicon_path(default=getattr(settings,'FAVICON_PATH', 'images/favicon.ico')): +def favicon_path(default=getattr(settings, 'FAVICON_PATH', 'images/favicon.ico')): """ Django template tag that outputs the configured favicon: {% favicon_path %} diff --git a/common/djangoapps/terrain/stubs/http.py b/common/djangoapps/terrain/stubs/http.py index 8013616431..80d2a0e70b 100644 --- a/common/djangoapps/terrain/stubs/http.py +++ b/common/djangoapps/terrain/stubs/http.py @@ -123,8 +123,8 @@ class StubHttpRequestHandler(BaseHTTPRequestHandler, object): # By default, `parse_qs` returns a list of values for each param # For convenience, we replace lists of 1 element with just the element return { - k:v[0] if len(v) == 1 else v - for k,v in urlparse.parse_qs(query).items() + key: value[0] if len(value) == 1 else value + for key, value in urlparse.parse_qs(query).items() } @lazy diff --git a/common/djangoapps/terrain/stubs/tests/test_lti_stub.py b/common/djangoapps/terrain/stubs/tests/test_lti_stub.py index 34f2445ac2..9614e4b9b0 100644 --- a/common/djangoapps/terrain/stubs/tests/test_lti_stub.py +++ b/common/djangoapps/terrain/stubs/tests/test_lti_stub.py @@ -35,7 +35,7 @@ class StubLtiServiceTest(unittest.TestCase): 'launch_presentation_return_url': '', 'lis_outcome_service_url': 'http://localhost:8001/test_callback', 'lis_result_sourcedid': '', - 'resource_link_id':'', + 'resource_link_id': '', } def test_invalid_request_url(self): diff --git a/common/djangoapps/terrain/stubs/xqueue.py b/common/djangoapps/terrain/stubs/xqueue.py index 76ba787fb5..ad66ce2ba3 100644 --- a/common/djangoapps/terrain/stubs/xqueue.py +++ b/common/djangoapps/terrain/stubs/xqueue.py @@ -214,6 +214,7 @@ class StubXQueueService(StubHttpService): except for 'default' and 'register_submission_url' which have special meaning """ return { - key:val for key, val in self.config.iteritems() + key: value + for key, value in self.config.iteritems() if key not in self.NON_QUEUE_CONFIG_KEYS }.items() diff --git a/common/lib/sandbox-packages/verifiers/draganddrop.py b/common/lib/sandbox-packages/verifiers/draganddrop.py index cdfa163f33..28ac146d4f 100644 --- a/common/lib/sandbox-packages/verifiers/draganddrop.py +++ b/common/lib/sandbox-packages/verifiers/draganddrop.py @@ -21,7 +21,7 @@ or: { "molecule": "[100, 200]" }, ] } -values are (x,y) coordinates of centers of dragged images. +values are (x, y) coordinates of centers of dragged images. """ import json @@ -77,7 +77,7 @@ class PositionsCompare(list): list or string:: "abc" - target [10, 20] - list of integers - [[10,20], 200] list of list and integer + [[10, 20], 200] list of list and integer """ def __eq__(self, other): @@ -223,10 +223,10 @@ class DragAndDrop(object): Examples: - many draggables per position: - user ['1','2','2','2'] is 'anyof' equal to ['1', '2', '3'] + user ['1', '2', '2', '2'] is 'anyof' equal to ['1', '2', '3'] - draggables can be placed in any order: - user ['1','2','3','4'] is 'anyof' equal to ['4', '2', '1', 3'] + user ['1', '2', '3', '4'] is 'anyof' equal to ['4', '2', '1', 3'] 'unordered_equal' is same as 'exact' but disregards on order @@ -235,7 +235,7 @@ class DragAndDrop(object): Equality functon depends on type of element. They declared in PositionsCompare class. For position like targets ids ("t1", "t2", etc..) it is string equality function. For coordinate - positions ([1,2] or [[1,2], 15]) it is coordinate_positions_compare + positions ([1, 2] or [[1, 2], 15]) it is coordinate_positions_compare function (see docstrings in PositionsCompare class) Args: @@ -352,7 +352,7 @@ class DragAndDrop(object): # correct_answer entries. If the draggable is mentioned in at least one # correct_answer entry, the value is False. # default to consider every user answer excess until proven otherwise. - self.excess_draggables = dict((users_draggable.keys()[0],True) + self.excess_draggables = dict((users_draggable.keys()[0], True) for users_draggable in user_answer) # Convert nested `user_answer` to flat format. @@ -414,8 +414,8 @@ def grade(user_input, correct_answer): 'rule': 'anyof' }, { - 'draggables': ['l1_c','l8_c'], - 'targets': ['t5_c','t6_c'], + 'draggables': ['l1_c', 'l8_c'], + 'targets': ['t5_c', 't6_c'], 'rule': 'anyof' } ] diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py index bd6cf0ef2c..036ed74b22 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py @@ -103,7 +103,7 @@ class PeerGradingService(GradingService): self._record_result('get_problem_list', result) dog_stats_api.histogram( self._metric_name('get_problem_list.result.length'), - len(result.get('problem_list',[])) + len(result.get('problem_list', [])), ) return result diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py index 30c5c10c49..efc4f38d3b 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py @@ -198,7 +198,7 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild): 'success': success, 'rubric_html': self.get_rubric_html(system), 'error': error_message, - 'student_response': data['student_answer'].replace("\n","
    ") + 'student_response': data['student_answer'].replace("\n", "
    "), } def save_assessment(self, data, _system): diff --git a/lms/djangoapps/courseware/features/high-level-tabs.py b/lms/djangoapps/courseware/features/high-level-tabs.py index 774ac841c5..e2c1f7129e 100644 --- a/lms/djangoapps/courseware/features/high-level-tabs.py +++ b/lms/djangoapps/courseware/features/high-level-tabs.py @@ -7,5 +7,5 @@ def i_click_on_the_tab_and_check(step): tab_text = tab_title['TabName'] title = tab_title['PageTitle'] world.click_link(tab_text) - world.wait_for(lambda _driver:title in world.browser.title) + world.wait_for(lambda _driver: title in world.browser.title) assert(title in world.browser.title) diff --git a/lms/djangoapps/notification_prefs/tests.py b/lms/djangoapps/notification_prefs/tests.py index 2a12f70047..3206dfaabc 100644 --- a/lms/djangoapps/notification_prefs/tests.py +++ b/lms/djangoapps/notification_prefs/tests.py @@ -67,7 +67,7 @@ class NotificationPrefViewTest(UrlResetMixin, TestCase): request.user = self.user response = ajax_status(request) self.assertEqual(response.status_code, 200) - self.assertEqual(json.loads(response.content), {"status":0}) + self.assertEqual(json.loads(response.content), {"status": 0}) def test_ajax_status_get_1(self): self.create_prefs() @@ -75,7 +75,7 @@ class NotificationPrefViewTest(UrlResetMixin, TestCase): request.user = self.user response = ajax_status(request) self.assertEqual(response.status_code, 200) - self.assertEqual(json.loads(response.content), {"status":1}) + self.assertEqual(json.loads(response.content), {"status": 1}) def test_ajax_status_post(self): request = self.request_factory.post("dummy") diff --git a/lms/djangoapps/notification_prefs/views.py b/lms/djangoapps/notification_prefs/views.py index 4df6465830..eef24c5ee4 100644 --- a/lms/djangoapps/notification_prefs/views.py +++ b/lms/djangoapps/notification_prefs/views.py @@ -149,7 +149,7 @@ def ajax_status(request): key=NOTIFICATION_PREF_KEY ) - return HttpResponse(json.dumps({"status":len(qs)}), content_type="application/json") + return HttpResponse(json.dumps({"status": len(qs)}), content_type="application/json") @require_GET diff --git a/lms/djangoapps/open_ended_grading/staff_grading_service.py b/lms/djangoapps/open_ended_grading/staff_grading_service.py index d67376591b..81baf721c2 100644 --- a/lms/djangoapps/open_ended_grading/staff_grading_service.py +++ b/lms/djangoapps/open_ended_grading/staff_grading_service.py @@ -321,7 +321,7 @@ def get_problem_list(request, course_id): u'If not, please do so and return to this page.' ) valid_problem_list = [] - for i in xrange(0,len(problem_list)): + for i in xrange(0, len(problem_list)): # Needed to ensure that the 'location' key can be accessed. try: problem_list[i] = json.loads(problem_list[i]) diff --git a/lms/djangoapps/staticbook/views.py b/lms/djangoapps/staticbook/views.py index 4d454f90f8..dadeb56568 100644 --- a/lms/djangoapps/staticbook/views.py +++ b/lms/djangoapps/staticbook/views.py @@ -110,7 +110,7 @@ def pdf_index(request, course_id, book_index, chapter=None, page=None): if page is not None: viewer_params += '&page={}'.format(page) - if request.GET.get('viewer','') == 'true': + if request.GET.get('viewer', '') == 'true': template = 'pdf_viewer.html' else: template = 'static_pdfbook.html' From 16afc89b9b67624a7de634fa4fa0c2e6b251e4a6 Mon Sep 17 00:00:00 2001 From: stv Date: Sat, 8 Nov 2014 20:43:35 -0800 Subject: [PATCH 36/48] Fix PEP8: E241 multiple spaces after ',' --- .../contentstore/management/commands/prompt.py | 9 +++++++-- cms/djangoapps/contentstore/views/transcripts_ajax.py | 10 ++++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/prompt.py b/cms/djangoapps/contentstore/management/commands/prompt.py index 44f981b5ac..260ab44079 100644 --- a/cms/djangoapps/contentstore/management/commands/prompt.py +++ b/cms/djangoapps/contentstore/management/commands/prompt.py @@ -11,8 +11,13 @@ def query_yes_no(question, default="yes"): The "answer" return value is one of "yes" or "no". """ - valid = {"yes": True, "y": True, "ye": True, - "no": False, "n": False} + valid = { + "yes": True, + "y": True, + "ye": True, + "no": False, + "n": False, + } if default is None: prompt = " [y/n] " elif default == "yes": diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index 57e8e2614e..d404a6d9dc 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -377,7 +377,10 @@ def choose_transcripts(request): if item.sub != html5_id: # update sub value item.sub = html5_id item.save_with_metadata(request.user) - response = {'status': 'Success', 'subs': item.sub} + response = { + 'status': 'Success', + 'subs': item.sub, + } return JsonResponse(response) @@ -408,7 +411,10 @@ def replace_transcripts(request): item.sub = youtube_id item.save_with_metadata(request.user) - response = {'status': 'Success', 'subs': item.sub} + response = { + 'status': 'Success', + 'subs': item.sub, + } return JsonResponse(response) From 180adfed52adf3926ac9ed902592743b9711c4ee Mon Sep 17 00:00:00 2001 From: stv Date: Sat, 8 Nov 2014 20:50:52 -0800 Subject: [PATCH 37/48] Fix PEP8: E251 unexpected spaces around keyword / parameter equals --- common/lib/xmodule/xmodule/combined_open_ended_module.py | 2 +- .../lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index 3a184ffec0..a6e8d5bf36 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -208,7 +208,7 @@ class CombinedOpenEndedFields(object): "This field is only populated if the instructor changes tasks after " "the module is created and students have attempted it (for example, if a self assessed problem is " "changed to self and peer assessed)."), - scope = Scope.user_state + scope=Scope.user_state, ) task_states = List( help=_("List of state dictionaries of each task within this module."), diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py b/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py index 4882c42734..e98095b596 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py @@ -88,7 +88,7 @@ def modulestore(): ModuleStoreNoSettings.modulestore = class_( None, # contentstore ModuleStoreNoSettings.MODULESTORE['DOC_STORE_CONFIG'], - branch_setting_func = lambda: ModuleStoreEnum.Branch.draft_preferred, + branch_setting_func=lambda: ModuleStoreEnum.Branch.draft_preferred, **options ) From cb894cf35e7d118470ba564c85dcba9ce83be325 Mon Sep 17 00:00:00 2001 From: stv Date: Sat, 8 Nov 2014 20:41:18 -0800 Subject: [PATCH 38/48] Fix PEP8: E261 at least two spaces before inline comment --- cms/djangoapps/contentstore/features/video_handout.py | 2 +- cms/envs/common.py | 2 +- cms/envs/devstack.py | 2 +- common/lib/capa/capa/xqueue_interface.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cms/djangoapps/contentstore/features/video_handout.py b/cms/djangoapps/contentstore/features/video_handout.py index c847a2a5fb..63a9e42c9e 100644 --- a/cms/djangoapps/contentstore/features/video_handout.py +++ b/cms/djangoapps/contentstore/features/video_handout.py @@ -3,7 +3,7 @@ # pylint: disable=C0111 from lettuce import world, step -from nose.tools import assert_true # pylint: disable=E0611 +from nose.tools import assert_true # pylint: disable=E0611 from video_editor import RequestHandlerWithSessionId, success_upload_file diff --git a/cms/envs/common.py b/cms/envs/common.py index 2e987ed5b0..c542b373b7 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -736,7 +736,7 @@ ADVANCED_COMPONENT_TYPES = [ 'done', # Lets students mark things as done. See https://github.com/pmitros/DoneXBlock 'audio', # Embed an audio file. See https://github.com/pmitros/AudioXBlock 'recommender', # Crowdsourced recommender. Prototype by dli&pmitros. Intended for roll-out in one place in one course. - 'profile', # Prototype user profile XBlock. Used to test XBlock parameter passing. See https://github.com/pmitros/ProfileXBlock + 'profile', # Prototype user profile XBlock. Used to test XBlock parameter passing. See https://github.com/pmitros/ProfileXBlock 'split_test', 'combinedopenended', 'peergrading', diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index 05986e8b3d..f45e601e6e 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -2,7 +2,7 @@ Specific overrides to the base prod settings to make development easier. """ -from .aws import * # pylint: disable=wildcard-import, unused-wildcard-import +from .aws import * # pylint: disable=wildcard-import, unused-wildcard-import # Don't use S3 in devstack, fall back to filesystem del DEFAULT_FILE_STORAGE diff --git a/common/lib/capa/capa/xqueue_interface.py b/common/lib/capa/capa/xqueue_interface.py index 5b877e998b..aa327dc285 100644 --- a/common/lib/capa/capa/xqueue_interface.py +++ b/common/lib/capa/capa/xqueue_interface.py @@ -14,7 +14,7 @@ dateformat = '%Y%m%d%H%M%S' XQUEUE_METRIC_NAME = 'edxapp.xqueue' # Wait time for response from Xqueue. -XQUEUE_TIMEOUT = 35 # seconds +XQUEUE_TIMEOUT = 35 # seconds def make_hashkey(seed): From 1164624fbc4779eaa079121448b59c6166dbade0 Mon Sep 17 00:00:00 2001 From: stv Date: Sat, 8 Nov 2014 18:34:53 -0800 Subject: [PATCH 39/48] Fix PEP8: W291 trailing whitespace --- common/djangoapps/track/middleware.py | 2 +- common/lib/xmodule/xmodule/video_module/video_module.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/common/djangoapps/track/middleware.py b/common/djangoapps/track/middleware.py index 9dc2a03692..242e819a5f 100644 --- a/common/djangoapps/track/middleware.py +++ b/common/djangoapps/track/middleware.py @@ -106,7 +106,7 @@ class TrackMiddleware(object): for header_name, context_key in META_KEY_TO_CONTEXT_KEY.iteritems(): context[context_key] = request.META.get(header_name, '') - # Google Analytics uses the clientId to keep track of unique visitors. A GA cookie looks like + # Google Analytics uses the clientId to keep track of unique visitors. A GA cookie looks like # this: _ga=GA1.2.1033501218.1368477899. The clientId is this part: 1033501218.1368477899. google_analytics_cookie = request.COOKIES.get('_ga') if google_analytics_cookie is None: diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py index d10bd65790..0de4ed2f87 100644 --- a/common/lib/xmodule/xmodule/video_module/video_module.py +++ b/common/lib/xmodule/xmodule/video_module/video_module.py @@ -132,7 +132,7 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers, def get_transcripts_for_student(self): """Return transcript information necessary for rendering the XModule student view. - This is more or less a direct extraction from `get_html`. + This is more or less a direct extraction from `get_html`. Returns: Tuple of (track_url, transcript_language, sorted_languages) From bd4658e6a020e2f50981c2efb6b3fe6db26e07b4 Mon Sep 17 00:00:00 2001 From: stv Date: Mon, 10 Nov 2014 13:30:47 -0800 Subject: [PATCH 40/48] Lower PEP8 threshold to 150 violations --- scripts/all-tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/all-tests.sh b/scripts/all-tests.sh index a666402078..52cab28a02 100755 --- a/scripts/all-tests.sh +++ b/scripts/all-tests.sh @@ -57,7 +57,7 @@ set -e # Violations thresholds for failing the build PYLINT_THRESHOLD=4725 -PEP8_THRESHOLD=200 +PEP8_THRESHOLD=150 source $HOME/jenkins_env From b7d526b9073aadcf19d71fd321cdb85f54c7c88b Mon Sep 17 00:00:00 2001 From: zubair-arbi Date: Tue, 11 Nov 2014 14:22:26 +0500 Subject: [PATCH 41/48] avoid annonymous user from redirecting to survey form WL-150 --- .../courseware/tests/test_course_survey.py | 14 ++++++++++++++ lms/djangoapps/courseware/views.py | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/tests/test_course_survey.py b/lms/djangoapps/courseware/tests/test_course_survey.py index ba79989f76..7a92a8e5f7 100644 --- a/lms/djangoapps/courseware/tests/test_course_survey.py +++ b/lms/djangoapps/courseware/tests/test_course_survey.py @@ -106,6 +106,20 @@ class SurveyViewsTests(LoginEnrollmentTestCase): """ self._assert_survey_redirect(self.course) + def test_anonymous_user_visiting_course_with_survey(self): + """ + Verifies that anonymous user going to the courseware info with an unanswered survey is not + redirected to survery and info page renders without server error. + """ + self.logout() + resp = self.client.get( + reverse( + 'info', + kwargs={'course_id': unicode(self.course.id)} + ) + ) + self.assertEquals(resp.status_code, 200) + def test_visiting_course_with_existing_answers(self): """ Verifies that going to the courseware with an answered survey, there is no redirect diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 5887aa6489..002dd5f51e 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -586,7 +586,7 @@ def course_info(request, course_id): # check to see if there is a required survey that must be taken before # the user can access the course. - if survey.utils.must_answer_survey(course, request.user): + if request.user.is_authenticated() and survey.utils.must_answer_survey(course, request.user): return redirect(reverse('course_survey', args=[unicode(course.id)])) staff_access = has_access(request.user, 'staff', course) From a93faad78bac5764fb19ba5347c0faa823cfc684 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 10 Nov 2014 18:57:23 -0500 Subject: [PATCH 42/48] try importing the exact references we need rather than deferencing it from the djangoapp module --- .../courseware/tests/test_microsites.py | 33 ++++++++++++++++++- lms/djangoapps/courseware/views.py | 5 ++- .../shoppingcart/context_processor.py | 9 ++--- lms/envs/test.py | 2 ++ lms/templates/courseware/course_about.html | 9 ++--- 5 files changed, 45 insertions(+), 13 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_microsites.py b/lms/djangoapps/courseware/tests/test_microsites.py index 2f34de8e14..b9a916dd8c 100644 --- a/lms/djangoapps/courseware/tests/test_microsites.py +++ b/lms/djangoapps/courseware/tests/test_microsites.py @@ -10,7 +10,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from helpers import LoginEnrollmentTestCase from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE - +from course_modes.models import CourseMode from xmodule.course_module import ( CATALOG_VISIBILITY_CATALOG_AND_ABOUT, CATALOG_VISIBILITY_NONE) @@ -55,6 +55,7 @@ class TestMicrosites(ModuleStoreTestCase, LoginEnrollmentTestCase): self.course_with_visibility = CourseFactory.create( display_name='visible_course', org='TestMicrositeX', + course="foo", catalog_visibility=CATALOG_VISIBILITY_CATALOG_AND_ABOUT, ) @@ -189,3 +190,33 @@ class TestMicrosites(ModuleStoreTestCase, LoginEnrollmentTestCase): url = reverse('about_course', args=[self.course_hidden_visibility.id.to_deprecated_string()]) resp = self.client.get(url, HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) self.assertEqual(resp.status_code, 404) + + @override_settings(SITE_NAME=settings.MICROSITE_TEST_HOSTNAME) + def test_paid_course_registration(self): + """ + Make sure that Microsite overrides on the ENABLE_SHOPPING_CART and + ENABLE_PAID_COURSE_ENROLLMENTS are honored + """ + course_mode = CourseMode( + course_id=self.course_with_visibility.id, + mode_slug="honor", + mode_display_name="honor cert", + min_price=10, + ) + course_mode.save() + + # first try on the non microsite, which + # should pick up the global configuration (where ENABLE_PAID_COURSE_REGISTRATIONS = False) + url = reverse('about_course', args=[self.course_with_visibility.id.to_deprecated_string()]) + resp = self.client.get(url) + self.assertEqual(resp.status_code, 200) + self.assertIn("Register for {}".format(self.course_with_visibility.id.course), resp.content) + self.assertNotIn("Add {} to Cart ($10)".format(self.course_with_visibility.id.course), resp.content) + + # now try on the microsite + url = reverse('about_course', args=[self.course_with_visibility.id.to_deprecated_string()]) + resp = self.client.get(url, HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) + self.assertEqual(resp.status_code, 200) + self.assertNotIn("Register for {}".format(self.course_with_visibility.id.course), resp.content) + self.assertIn("Add {} to Cart ($10)".format(self.course_with_visibility.id.course), resp.content) + self.assertIn('$("#add_to_cart_post").click', resp.content) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index cf44331ec7..2ecaab3d37 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -733,7 +733,8 @@ def course_about(request, course_id): in_cart = False reg_then_add_to_cart_link = "" - if (is_shopping_cart_enabled()): + _is_shopping_cart_enabled = is_shopping_cart_enabled() + if (_is_shopping_cart_enabled): registration_price = CourseMode.min_course_price_for_currency(course_key, settings.PAID_COURSE_REGISTRATION_CURRENCY[0]) if request.user.is_authenticated(): @@ -775,6 +776,8 @@ def course_about(request, course_id): # We do not want to display the internal courseware header, which is used when the course is found in the # context. This value is therefor explicitly set to render the appropriate header. 'disable_courseware_header': True, + 'is_shopping_cart_enabled': _is_shopping_cart_enabled, + 'cart_link': reverse('shoppingcart.views.show_cart'), }) diff --git a/lms/djangoapps/shoppingcart/context_processor.py b/lms/djangoapps/shoppingcart/context_processor.py index 20e096155b..5fc9439025 100644 --- a/lms/djangoapps/shoppingcart/context_processor.py +++ b/lms/djangoapps/shoppingcart/context_processor.py @@ -6,7 +6,8 @@ navigation. We want to do this in the context_processor to 2) because navigation.html is "called" by being included in other templates, there's no "views.py" to put this. """ -import shoppingcart +from .models import Order, PaidCourseRegistration, CourseRegCodeItem +from .utils import is_shopping_cart_enabled def user_has_cart_context_processor(request): @@ -19,11 +20,11 @@ def user_has_cart_context_processor(request): # user is logged in and request.user.is_authenticated() and # do we have the feature turned on - shoppingcart.utils.is_shopping_cart_enabled() and + is_shopping_cart_enabled() and # user's cart has PaidCourseRegistrations - shoppingcart.models.Order.user_cart_has_items( + Order.user_cart_has_items( request.user, - [shoppingcart.models.PaidCourseRegistration, shoppingcart.models.CourseRegCodeItem] + [PaidCourseRegistration, CourseRegCodeItem] ) ) diff --git a/lms/envs/test.py b/lms/envs/test.py index 049fdf7b89..2810337e63 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -352,6 +352,8 @@ MICROSITE_CONFIGURATION = { "ALWAYS_REDIRECT_HOMEPAGE_TO_DASHBOARD_FOR_AUTHENTICATED_USER": False, "COURSE_CATALOG_VISIBILITY_PERMISSION": "see_in_catalog", "COURSE_ABOUT_VISIBILITY_PERMISSION": "see_about_page", + "ENABLE_SHOPPING_CART": True, + "ENABLE_PAID_COURSE_REGISTRATION": True, }, "default": { "university": "default_university", diff --git a/lms/templates/courseware/course_about.html b/lms/templates/courseware/course_about.html index 77063c099c..0859f2247b 100644 --- a/lms/templates/courseware/course_about.html +++ b/lms/templates/courseware/course_about.html @@ -4,11 +4,6 @@ from courseware.courses import course_image_url, get_course_about_section from django.conf import settings from edxmako.shortcuts import marketing_link - - if settings.FEATURES.get('ENABLE_SHOPPING_CART'): - cart_link = reverse('shoppingcart.views.show_cart') - else: - cart_link = "" %> <%namespace name='static' file='../static_content.html'/> <%! from microsite_configuration import microsite %> @@ -42,7 +37,7 @@ event.preventDefault(); }); - % if settings.FEATURES.get('ENABLE_SHOPPING_CART') and settings.FEATURES.get('ENABLE_PAID_COURSE_REGISTRATION'): + % if is_shopping_cart_enabled: add_course_complete_handler = function(jqXHR, textStatus) { if (jqXHR.status == 200) { location.href = "${cart_link}"; @@ -162,7 +157,7 @@ ## so that they can register and become a real user that can enroll. % elif not is_shib_course and not can_enroll: ${_("Enrollment is Closed")} - %elif settings.FEATURES.get('ENABLE_PAID_COURSE_REGISTRATION') and registration_price: + %elif is_shopping_cart_enabled and registration_price: <% if user.is_authenticated(): reg_href = "#" From da5bfd45aaff72ecd074d060586942b7d4563c0d Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Wed, 12 Nov 2014 09:17:56 -0500 Subject: [PATCH 43/48] Fix pylint violation. --- lms/djangoapps/shoppingcart/processors/CyberSource2.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lms/djangoapps/shoppingcart/processors/CyberSource2.py b/lms/djangoapps/shoppingcart/processors/CyberSource2.py index 1d66138a96..68e35247c0 100644 --- a/lms/djangoapps/shoppingcart/processors/CyberSource2.py +++ b/lms/djangoapps/shoppingcart/processors/CyberSource2.py @@ -367,6 +367,7 @@ def _payment_accepted(order_id, auth_amount, currency, decision): total_cost_currency=order.currency ) ) + #pylint: disable=attribute-defined-outside-init ex.order = order raise ex else: From 6e85ec0739838d70508c3f1d9e765d28ed13960b Mon Sep 17 00:00:00 2001 From: stephensanchez Date: Fri, 24 Oct 2014 15:47:01 +0000 Subject: [PATCH 44/48] Changing event suppression to signal suppression. Code review changes. Fix test and pylint. pylint error. Updating the test to verify the signal is fired. Test for all analytics events Import pylint error. Pylint error --- .../management/commands/transfer_students.py | 4 +- .../tests/test_transfer_students.py | 95 +++++++++++++++++-- common/djangoapps/student/models.py | 14 +-- lms/djangoapps/shoppingcart/models.py | 4 +- .../shoppingcart/tests/test_models.py | 18 ++++ 5 files changed, 118 insertions(+), 17 deletions(-) diff --git a/common/djangoapps/student/management/commands/transfer_students.py b/common/djangoapps/student/management/commands/transfer_students.py index 041402e19e..8e8557ff88 100644 --- a/common/djangoapps/student/management/commands/transfer_students.py +++ b/common/djangoapps/student/management/commands/transfer_students.py @@ -83,7 +83,7 @@ class Command(TrackedCommand): # Move the Student between the classes. mode = enrollment.mode old_is_active = enrollment.is_active - CourseEnrollment.unenroll(user, source_key, emit_unenrollment_event=False) + CourseEnrollment.unenroll(user, source_key, skip_refund=True) print(u"Unenrolled {} from {}".format(user.username, unicode(source_key))) for dest_key in dest_keys: @@ -98,7 +98,7 @@ class Command(TrackedCommand): # Un-enroll from the new course if the user had un-enrolled # form the old course. if not old_is_active: - new_enrollment.update_enrollment(is_active=False, emit_unenrollment_event=False) + new_enrollment.update_enrollment(is_active=False, skip_refund=True) if transfer_certificates: self._transfer_certificate_item(source_key, enrollment, user, dest_keys, new_enrollment) diff --git a/common/djangoapps/student/management/tests/test_transfer_students.py b/common/djangoapps/student/management/tests/test_transfer_students.py index caebeeace2..45376eb148 100644 --- a/common/djangoapps/student/management/tests/test_transfer_students.py +++ b/common/djangoapps/student/management/tests/test_transfer_students.py @@ -2,11 +2,16 @@ Tests the transfer student management command """ from django.conf import settings +from mock import patch, call from opaque_keys.edx import locator import unittest import ddt + +from shoppingcart.models import Order, CertificateItem # pylint: disable=F0401 +from course_modes.models import CourseMode from student.management.commands import transfer_students -from student.models import CourseEnrollment +from student.models import CourseEnrollment, UNENROLL_DONE, EVENT_NAME_ENROLLMENT_DEACTIVATED, \ + EVENT_NAME_ENROLLMENT_ACTIVATED, EVENT_NAME_ENROLLMENT_MODE_CHANGED from student.tests.factories import UserFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -18,18 +23,40 @@ class TestTransferStudents(ModuleStoreTestCase): """Tests for transferring students between courses.""" PASSWORD = 'test' + signal_fired = False + + def setUp(self, **kwargs): + """Connect a stub receiver, and analytics event tracking.""" + UNENROLL_DONE.connect(self.assert_unenroll_signal) + patcher = patch('student.models.tracker') + self.mock_tracker = patcher.start() + self.addCleanup(patcher.stop) + + def tearDown(self): + """Disconnects the UNENROLL stub receiver.""" + UNENROLL_DONE.disconnect(self.assert_unenroll_signal) + + def assert_unenroll_signal(self, skip_refund=False, **kwargs): # pylint: disable=W0613 + """ Signal Receiver stub for testing that the unenroll signal was fired. """ + self.assertFalse(self.signal_fired) + self.assertTrue(skip_refund) + self.signal_fired = True def test_transfer_students(self): - student = UserFactory() + """ Verify the transfer student command works as intended. """ + student = UserFactory.create() student.set_password(self.PASSWORD) # pylint: disable=E1101 student.save() # pylint: disable=E1101 - + mode = 'verified' # Original Course original_course_location = locator.CourseLocator('Org0', 'Course0', 'Run0') course = self._create_course(original_course_location) # Enroll the student in 'verified' CourseEnrollment.enroll(student, course.id, mode="verified") + # Create and purchase a verified cert for the original course. + self._create_and_purchase_verified(student, course.id) + # New Course 1 course_location_one = locator.CourseLocator('Org1', 'Course1', 'Run1') new_course_one = self._create_course(course_location_one) @@ -45,11 +72,55 @@ class TestTransferStudents(ModuleStoreTestCase): transfer_students.Command().handle( source_course=original_key, dest_course_list=new_key_one + "," + new_key_two ) + self.assertTrue(self.signal_fired) + + # Confirm the analytics event was emitted. + self.mock_tracker.emit.assert_has_calls( # pylint: disable=E1103 + [ + call( + EVENT_NAME_ENROLLMENT_ACTIVATED, + {'course_id': original_key, 'user_id': student.id, 'mode': mode} + ), + call( + EVENT_NAME_ENROLLMENT_MODE_CHANGED, + {'course_id': original_key, 'user_id': student.id, 'mode': mode} + ), + call( + EVENT_NAME_ENROLLMENT_DEACTIVATED, + {'course_id': original_key, 'user_id': student.id, 'mode': mode} + ), + call( + EVENT_NAME_ENROLLMENT_ACTIVATED, + {'course_id': new_key_one, 'user_id': student.id, 'mode': mode} + ), + call( + EVENT_NAME_ENROLLMENT_MODE_CHANGED, + {'course_id': new_key_one, 'user_id': student.id, 'mode': mode} + ), + call( + EVENT_NAME_ENROLLMENT_ACTIVATED, + {'course_id': new_key_two, 'user_id': student.id, 'mode': mode} + ), + call( + EVENT_NAME_ENROLLMENT_MODE_CHANGED, + {'course_id': new_key_two, 'user_id': student.id, 'mode': mode} + ) + ] + ) + self.mock_tracker.reset_mock() # Confirm the enrollment mode is verified on the new courses, and enrollment is enabled as appropriate. - self.assertEquals(('verified', False), CourseEnrollment.enrollment_mode_for_user(student, course.id)) - self.assertEquals(('verified', True), CourseEnrollment.enrollment_mode_for_user(student, new_course_one.id)) - self.assertEquals(('verified', True), CourseEnrollment.enrollment_mode_for_user(student, new_course_two.id)) + self.assertEquals((mode, False), CourseEnrollment.enrollment_mode_for_user(student, course.id)) + self.assertEquals((mode, True), CourseEnrollment.enrollment_mode_for_user(student, new_course_one.id)) + self.assertEquals((mode, True), CourseEnrollment.enrollment_mode_for_user(student, new_course_two.id)) + + # Confirm the student has not be refunded. + target_certs = CertificateItem.objects.filter( + course_id=course.id, user_id=student, status='purchased', mode=mode + ) + self.assertTrue(target_certs[0]) + self.assertFalse(target_certs[0].refund_requested_time) + self.assertEquals(target_certs[0].order.status, 'purchased') def _create_course(self, course_location): """ Creates a course """ @@ -58,3 +129,15 @@ class TestTransferStudents(ModuleStoreTestCase): number=course_location.course, run=course_location.run ) + + def _create_and_purchase_verified(self, student, course_id): + """ Creates a verified mode for the course and purchases it for the student. """ + course_mode = CourseMode(course_id=course_id, + mode_slug="verified", + mode_display_name="verified cert", + min_price=50) + course_mode.save() + # When there is no expiration date on a verified mode, the user can always get a refund + cart = Order.get_cart_for_user(user=student) + CertificateItem.add_to_order(cart, course_id, 50, 'verified') + cart.purchase() diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index dda87a80e5..311becddbc 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -56,7 +56,7 @@ from ratelimitbackend import admin import analytics -UNENROLL_DONE = Signal(providing_args=["course_enrollment"]) +UNENROLL_DONE = Signal(providing_args=["course_enrollment", "skip_refund"]) log = logging.getLogger(__name__) AUDIT_LOG = logging.getLogger("audit") SessionStore = import_module(settings.SESSION_ENGINE).SessionStore # pylint: disable=invalid-name @@ -776,7 +776,7 @@ class CourseEnrollment(models.Model): is_course_full = cls.num_enrolled_in(course.id) >= course.max_student_enrollments_allowed return is_course_full - def update_enrollment(self, mode=None, is_active=None, emit_unenrollment_event=True): + def update_enrollment(self, mode=None, is_active=None, skip_refund=False): """ Updates an enrollment for a user in a class. This includes options like changing the mode, toggling is_active True/False, etc. @@ -814,8 +814,8 @@ class CourseEnrollment(models.Model): u"mode:{}".format(self.mode)] ) - elif emit_unenrollment_event: - UNENROLL_DONE.send(sender=None, course_enrollment=self) + else: + UNENROLL_DONE.send(sender=None, course_enrollment=self, skip_refund=skip_refund) self.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED) @@ -988,7 +988,7 @@ class CourseEnrollment(models.Model): raise @classmethod - def unenroll(cls, user, course_id, emit_unenrollment_event=True): + def unenroll(cls, user, course_id, skip_refund=False): """ Remove the user from a given course. If the relevant `CourseEnrollment` object doesn't exist, we log an error but don't throw an exception. @@ -999,11 +999,11 @@ class CourseEnrollment(models.Model): `course_id` is our usual course_id string (e.g. "edX/Test101/2013_Fall) - `emit_unenrollment_events` can be set to False to suppress events firing. + `skip_refund` can be set to True to avoid the refund process. """ try: record = CourseEnrollment.objects.get(user=user, course_id=course_id) - record.update_enrollment(is_active=False, emit_unenrollment_event=emit_unenrollment_event) + record.update_enrollment(is_active=False, skip_refund=skip_refund) except cls.DoesNotExist: err_msg = u"Tried to unenroll student {} from {} but they were not enrolled" diff --git a/lms/djangoapps/shoppingcart/models.py b/lms/djangoapps/shoppingcart/models.py index 37a32dfe0c..d06624b2c1 100644 --- a/lms/djangoapps/shoppingcart/models.py +++ b/lms/djangoapps/shoppingcart/models.py @@ -1038,7 +1038,7 @@ class CertificateItem(OrderItem): mode = models.SlugField() @receiver(UNENROLL_DONE) - def refund_cert_callback(sender, course_enrollment=None, **kwargs): + def refund_cert_callback(sender, course_enrollment=None, skip_refund=False, **kwargs): # pylint: disable=E0213,W0613 """ When a CourseEnrollment object calls its unenroll method, this function checks to see if that unenrollment occurred in a verified certificate that was within the refund deadline. If so, it actually performs the @@ -1048,7 +1048,7 @@ class CertificateItem(OrderItem): """ # Only refund verified cert unenrollments that are within bounds of the expiration date - if not course_enrollment.refundable(): + if (not course_enrollment.refundable()) or skip_refund: return target_certs = CertificateItem.objects.filter(course_id=course_enrollment.course_id, user_id=course_enrollment.user, status='purchased', mode='verified') diff --git a/lms/djangoapps/shoppingcart/tests/test_models.py b/lms/djangoapps/shoppingcart/tests/test_models.py index d2d992b89f..15385062be 100644 --- a/lms/djangoapps/shoppingcart/tests/test_models.py +++ b/lms/djangoapps/shoppingcart/tests/test_models.py @@ -495,6 +495,24 @@ class CertificateItemTest(ModuleStoreTestCase): self.assertTrue(target_certs[0].refund_requested_time) self.assertEquals(target_certs[0].order.status, 'refunded') + def test_no_refund_on_cert_callback(self): + # If we explicitly skip refunds, the unenroll action should not modify the purchase. + CourseEnrollment.enroll(self.user, self.course_key, 'verified') + cart = Order.get_cart_for_user(user=self.user) + CertificateItem.add_to_order(cart, self.course_key, self.cost, 'verified') + cart.purchase() + + CourseEnrollment.unenroll(self.user, self.course_key, skip_refund=True) + target_certs = CertificateItem.objects.filter( + course_id=self.course_key, + user_id=self.user, + status='purchased', + mode='verified' + ) + self.assertTrue(target_certs[0]) + self.assertFalse(target_certs[0].refund_requested_time) + self.assertEquals(target_certs[0].order.status, 'purchased') + def test_refund_cert_callback_before_expiration(self): # If the expiration date has not yet passed on a verified mode, the user can be refunded many_days = datetime.timedelta(days=60) From 326a1221b978f42b781ffe8c8c4eb9aee6b2173d Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Wed, 12 Nov 2014 11:47:41 -0500 Subject: [PATCH 45/48] Remove over optimization which didn't work correctly Added story PLAT-293 to do it correctly. This fixes TNL-764 --- .../xmodule/xmodule/modulestore/mongo/base.py | 6 +-- .../tests/test_mixed_modulestore.py | 42 ++++++++++--------- .../xmodule/modulestore/tests/test_publish.py | 14 ++++--- 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 2eb3505c9d..3bb2480eb7 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -20,7 +20,6 @@ import re from uuid import uuid4 from bson.son import SON -from contracts import contract, new_contract from datetime import datetime from fs.osfs import OSFS from mongodb_proxy import MongoProxy, autoretry_read @@ -1237,10 +1236,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo # update subtree edited info for ancestors # don't update the subtree info for descendants of the publish root for efficiency - if ( - (not isPublish or (isPublish and is_publish_root)) and - not self._is_in_bulk_operation(xblock.location.course_key) - ): + if not isPublish or (isPublish and is_publish_root): ancestor_payload = { 'edit_info.subtree_edited_on': now, 'edit_info.subtree_edited_by': user_id diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 152d7cf26a..16fbfb15a8 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -1432,25 +1432,26 @@ class TestMixedModuleStore(CourseComparisonTest): self.assertLess(node.subtree_edited_on, subtree_before) self.assertEqual(node.subtree_edited_by, subtree_by) - # Create a dummy vertical & html to test against - component = self.store.create_child( - self.user_id, - test_course.location, - 'vertical', - block_id='test_vertical' - ) - child = self.store.create_child( - self.user_id, - component.location, - 'html', - block_id='test_html' - ) - sibling = self.store.create_child( - self.user_id, - component.location, - 'html', - block_id='test_html_no_change' - ) + with self.store.bulk_operations(test_course.id): + # Create a dummy vertical & html to test against + component = self.store.create_child( + self.user_id, + test_course.location, + 'vertical', + block_id='test_vertical' + ) + child = self.store.create_child( + self.user_id, + component.location, + 'html', + block_id='test_html' + ) + sibling = self.store.create_child( + self.user_id, + component.location, + 'html', + block_id='test_html_no_change' + ) after_create = datetime.datetime.now(UTC) # Verify that all nodes were last edited in the past by create_user @@ -1461,7 +1462,8 @@ class TestMixedModuleStore(CourseComparisonTest): component.display_name = 'Changed Display Name' editing_user = self.user_id - 2 - component = self.store.update_item(component, editing_user) + with self.store.bulk_operations(test_course.id): # TNL-764 bulk ops disabled ancestor updates + component = self.store.update_item(component, editing_user) after_edit = datetime.datetime.now(UTC) check_node(component.location, after_create, after_edit, editing_user, after_create, after_edit, editing_user) # but child didn't change diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py index 1bccf755f8..ff0ddc2e04 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py @@ -24,20 +24,21 @@ class TestPublish(SplitWMongoCourseBoostrapper): # with bulk will delay all inheritance computations which won't be added into the mongo_calls with self.draft_mongo.bulk_operations(self.old_course_key): - # finds: 1 for parent to add child + # finds: 1 for parent to add child and 2 to get ancestors # sends: 1 for insert, 1 for parent (add child) - with check_mongo_calls(1, 2): + with check_mongo_calls(3, 2): self._create_item('chapter', 'Chapter1', {}, {'display_name': 'Chapter 1'}, 'course', 'runid', split=False) - with check_mongo_calls(2, 2): + with check_mongo_calls(4, 2): self._create_item('chapter', 'Chapter2', {}, {'display_name': 'Chapter 2'}, 'course', 'runid', split=False) # For each vertical (2) created: # - load draft # - load non-draft # - get last error # - load parent + # - get ancestors # - load inheritable data - with check_mongo_calls(7, 4): + with check_mongo_calls(15, 6): self._create_item('vertical', 'Vert1', {}, {'display_name': 'Vertical 1'}, 'chapter', 'Chapter1', split=False) self._create_item('vertical', 'Vert2', {}, {'display_name': 'Vertical 2'}, 'chapter', 'Chapter1', split=False) # For each (4) item created @@ -48,8 +49,9 @@ class TestPublish(SplitWMongoCourseBoostrapper): # - load parent # - load inheritable data # - load parent + # - load ancestors # count for updates increased to 16 b/c of edit_info updating - with check_mongo_calls(16, 8): + with check_mongo_calls(40, 16): self._create_item('html', 'Html1', "

    Goodbye

    ", {'display_name': 'Parented Html'}, 'vertical', 'Vert1', split=False) self._create_item( 'discussion', 'Discussion1', @@ -77,7 +79,7 @@ class TestPublish(SplitWMongoCourseBoostrapper): split=False ) - with check_mongo_calls(0, 2): + with check_mongo_calls(2, 2): # 2 finds b/c looking for non-existent parents self._create_item('static_tab', 'staticuno', "

    tab

    ", {'display_name': 'Tab uno'}, None, None, split=False) self._create_item('course_info', 'updates', "
    1. Sep 22

      test

    ", {}, None, None, split=False) From 3676b4f72fb1bb743a1d59f045bd698db3d60b55 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Wed, 12 Nov 2014 14:17:27 -0500 Subject: [PATCH 46/48] Upgrade diff-cover --- requirements/edx/github.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 45fcf555f9..0064263d21 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -23,7 +23,7 @@ # Our libraries: -e git+https://github.com/edx/XBlock.git@2029af2a4b524310847decfb34ef39da8a30dc4e#egg=XBlock -e git+https://github.com/edx/codejail.git@66dd5a45e5072666ff9a70c768576e9ffd1daa4b#egg=codejail --e git+https://github.com/edx/diff-cover.git@v0.7.1#egg=diff_cover +-e git+https://github.com/edx/diff-cover.git@v0.7.2#egg=diff_cover -e git+https://github.com/edx/js-test-tool.git@v0.1.5#egg=js_test_tool -e git+https://github.com/edx/event-tracking.git@0.1.0#egg=event-tracking -e git+https://github.com/edx/bok-choy.git@4a259e3548a19e41cc39433caf68ea58d10a27ba#egg=bok_choy From 482113883a70e517cbddb05d3ebb3c8b4b118ff6 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Wed, 12 Nov 2014 16:05:59 -0500 Subject: [PATCH 47/48] Paging and sorting for assets PLAT-75 --- common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py index 686b8e7687..be5d2b931f 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py @@ -330,8 +330,6 @@ class TestMongoAssetMetadataStorage(unittest.TestCase): Save multiple metadata in each store and retrieve it singularly, as all assets, and after deleting all. """ # Temporarily only perform this test for Old Mongo - not Split. - if not isinstance(storebuilder, MongoModulestoreBuilder): - raise unittest.SkipTest with MongoContentstoreBuilder().build() as contentstore: with storebuilder.build(contentstore) as store: course1 = CourseFactory.create(modulestore=store) From 8e2d7942641d88dc9c21ccee61c41a16a27b9f89 Mon Sep 17 00:00:00 2001 From: Ben Patterson Date: Wed, 12 Nov 2014 22:11:56 -0500 Subject: [PATCH 48/48] Improve compatibility with additional mysql installations. Some mysql debian packages do not grant mysql user as many permissions, and we need to add them. --- scripts/reset-test-db.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/reset-test-db.sh b/scripts/reset-test-db.sh index 42d8d69e8d..b224059028 100755 --- a/scripts/reset-test-db.sh +++ b/scripts/reset-test-db.sh @@ -26,6 +26,7 @@ DB_CACHE_DIR="common/test/db_cache" # Ensure the test database exists. echo "CREATE DATABASE IF NOT EXISTS test;" | mysql -u root +echo "GRANT ALL ON test.* TO mysql@localhost" | mysql -u root # Clear out the test database ./manage.py lms --settings bok_choy reset_db --traceback --noinput