From 5ecb90ec35b8f8d9cc75a4eb55c6cdb8611c77b2 Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Tue, 25 Nov 2014 16:13:14 +0000 Subject: [PATCH 01/20] Made empty course url the 'home' url instead. --- cms/djangoapps/contentstore/views/course.py | 6 +++-- cms/templates/index.html | 29 +-------------------- cms/urls.py | 1 + 3 files changed, 6 insertions(+), 30 deletions(-) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 5bad2efa2207..f40e86f08204 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -1,6 +1,7 @@ """ Views related to operations on course objects """ +from django.shortcuts import redirect import json import random import string # pylint: disable=deprecated-module @@ -71,7 +72,8 @@ from xmodule.course_module import CourseFields -__all__ = ['course_info_handler', 'course_handler', 'course_info_update_handler', +__all__ = ['course_info_handler', 'course_handler', 'course_listing', + 'course_info_update_handler', 'course_rerun_handler', 'settings_handler', 'grading_handler', @@ -230,7 +232,7 @@ def course_handler(request, course_key_string=None): return HttpResponseBadRequest() elif request.method == 'GET': # assume html if course_key_string is None: - return course_listing(request) + return redirect(reverse("home")) else: return course_index(request, CourseKey.from_string(course_key_string)) else: diff --git a/cms/templates/index.html b/cms/templates/index.html index cd2f8978aecb..8b5318f2abde 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -14,7 +14,7 @@ <%block name="content">
-

${_("My Courses")}

+

${_("Studio Home")}

% if user.is_active:
From a9bb0a598d7ffba54c749e9451839c0db6c33be7 Mon Sep 17 00:00:00 2001 From: Matjaz Gregoric Date: Fri, 14 Nov 2014 09:08:14 +0100 Subject: [PATCH 04/20] Update text on new content library form. --- cms/templates/index.html | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/cms/templates/index.html b/cms/templates/index.html index 8b5318f2abde..24652770200d 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -129,15 +129,14 @@

${_("Create a New Library")}

  • - ${_("The name of the organization sponsoring the library.")} ${_("Note: This is part of your library URL, so no spaces or special characters are allowed.")} ${_("This cannot be changed.")} + ${_("The public organization name for your library.")} ${_("This cannot be changed.")}
  • - - ## Translators: This is an example for the "number" used to identify a library, seen when filling out the form to create a new library. This example is short for "Computer Science Problems". The example number may contain letters but must not contain spaces. - - ${_("The unique code that identifies this library.")} ${_("Note: This is part of your library URL, so no spaces or special characters are allowed.")} ${_("This cannot be changed.")} + + + ${_("The {em_start}major version number{em_end} of your library. Minor revisions are tracked as edits happen within a library.").format(em_start='', em_end='')}
    • From 1f047b99270eaf4b5f67a873d75dac009053e696 Mon Sep 17 00:00:00 2001 From: Matjaz Gregoric Date: Fri, 14 Nov 2014 08:55:21 +0100 Subject: [PATCH 05/20] Libraries UI: remove underline on hover. This changes the style of the inactive libraries/courses tab when hovering. Only the active tab on the dashboard page gets the blue underline. When hovering over an inactive tab, the color of the text changes, but the underline does not appear. --- cms/static/sass/views/_dashboard.scss | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/cms/static/sass/views/_dashboard.scss b/cms/static/sass/views/_dashboard.scss index 63f55a9c097e..fc4bf00dcd20 100644 --- a/cms/static/sass/views/_dashboard.scss +++ b/cms/static/sass/views/_dashboard.scss @@ -299,19 +299,21 @@ line-height: $baseline*2; margin: 0 10px; - &.active, &:hover { + &.active { border-bottom: 4px solid $blue; } + &.active, &:hover { + a { + color: $gray-d2; + } + } + a { color: $blue; cursor: pointer; display: inline-block; } - - &.active a { - color: $gray-d2; - } } } From 8fccdb44293b247327f69453c22eb25da64009f7 Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Fri, 28 Nov 2014 16:52:00 +0000 Subject: [PATCH 06/20] Added test for /home/ redirect. --- .../test/acceptance/tests/studio/test_studio_general.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/common/test/acceptance/tests/studio/test_studio_general.py b/common/test/acceptance/tests/studio/test_studio_general.py index 88dc31467389..c0b409b66bb5 100644 --- a/common/test/acceptance/tests/studio/test_studio_general.py +++ b/common/test/acceptance/tests/studio/test_studio_general.py @@ -88,6 +88,15 @@ def setUp(self): ] ] + def test_page_redirect(self): + """ + /course/ is the base URL for all courses, but by itself, it should + redirect to /home/. + """ + self.dashboard_page = DashboardPage(self.browser) + self.dashboard_page.visit() + self.assertEqual(self.browser.current_url.strip('/').rsplit('/')[-1], 'home') + @skip('Intermittently failing with Page not found error for Assets. TE-418') def test_page_existence(self): """ From 1cbba1322ade88fa8ea1c3d509c00e1533bc564a Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Fri, 28 Nov 2014 17:23:45 +0000 Subject: [PATCH 07/20] Quality check and test fixes. --- .../contentstore/tests/test_contentstore.py | 8 ++--- .../contentstore/tests/test_i18n.py | 10 +++--- cms/djangoapps/contentstore/tests/tests.py | 10 +++--- cms/djangoapps/contentstore/views/public.py | 2 +- .../views/tests/test_course_index.py | 4 +-- cms/templates/index.html | 2 +- common/djangoapps/student/tests/test_login.py | 2 +- .../pages/lms/login_and_register.py | 2 ++ common/test/acceptance/tests/lms/test_lms.py | 5 ++- docs/shared/conf.py | 1 - pavelib/paver_tests/test_prereqs.py | 31 +++++++++++++++++-- pavelib/prereqs.py | 2 +- 12 files changed, 55 insertions(+), 24 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 27714230dc62..e0122f4cd92d 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1167,7 +1167,7 @@ def assert_course_permission_denied(self): def test_course_index_view_with_no_courses(self): """Test viewing the index page with no courses""" # Create a course so there is something to view - resp = self.client.get_html('/course/') + resp = self.client.get_html('/home/') self.assertContains( resp, '

    My Courses

    ', @@ -1189,7 +1189,7 @@ def test_item_factory(self): def test_course_index_view_with_course(self): """Test viewing the index page with an existing course""" CourseFactory.create(display_name='Robot Super Educational Course') - resp = self.client.get_html('/course/') + resp = self.client.get_html('/home/') self.assertContains( resp, '

    Robot Super Educational Course

    ', @@ -1604,7 +1604,7 @@ def assertInCourseListing(self, course_key): Asserts that the given course key is in the accessible course listing section of the html and NOT in the unsucceeded course action section of the html. """ - course_listing = lxml.html.fromstring(self.client.get_html('/course/').content) + course_listing = lxml.html.fromstring(self.client.get_html('/home/').content) self.assertEqual(len(self.get_course_listing_elements(course_listing, course_key)), 1) self.assertEqual(len(self.get_unsucceeded_course_action_elements(course_listing, course_key)), 0) @@ -1613,7 +1613,7 @@ def assertInUnsucceededCourseActions(self, course_key): Asserts that the given course key is in the unsucceeded course action section of the html and NOT in the accessible course listing section of the html. """ - course_listing = lxml.html.fromstring(self.client.get_html('/course/').content) + course_listing = lxml.html.fromstring(self.client.get_html('/home/').content) self.assertEqual(len(self.get_course_listing_elements(course_listing, course_key)), 0) self.assertEqual(len(self.get_unsucceeded_course_action_elements(course_listing, course_key)), 1) diff --git a/cms/djangoapps/contentstore/tests/test_i18n.py b/cms/djangoapps/contentstore/tests/test_i18n.py index e9e17394883f..da7911722035 100644 --- a/cms/djangoapps/contentstore/tests/test_i18n.py +++ b/cms/djangoapps/contentstore/tests/test_i18n.py @@ -44,9 +44,9 @@ def test_course_plain_english(self): self.client = AjaxEnabledTestClient() self.client.login(username=self.uname, password=self.password) - resp = self.client.get_html('/course/') + resp = self.client.get_html('/home/') self.assertContains(resp, - '

    My Courses

    ', + '

    Studio Home

    ', status_code=200, html=True) @@ -56,13 +56,13 @@ def test_course_explicit_english(self): self.client.login(username=self.uname, password=self.password) resp = self.client.get_html( - '/course/', + '/home/', {}, HTTP_ACCEPT_LANGUAGE='en', ) self.assertContains(resp, - '

    My Courses

    ', + '

    Studio Home

    ', status_code=200, html=True) @@ -81,7 +81,7 @@ def test_course_with_accents(self): self.client.login(username=self.uname, password=self.password) resp = self.client.get_html( - '/course/', + '/home/', {}, HTTP_ACCEPT_LANGUAGE='eo' ) diff --git a/cms/djangoapps/contentstore/tests/tests.py b/cms/djangoapps/contentstore/tests/tests.py index 9975764345e1..76429befa6cf 100644 --- a/cms/djangoapps/contentstore/tests/tests.py +++ b/cms/djangoapps/contentstore/tests/tests.py @@ -234,13 +234,13 @@ def test_login_link_on_activation_age(self): def test_private_pages_auth(self): """Make sure pages that do require login work.""" auth_pages = ( - '/course/', + '/home/', ) # These are pages that should just load when the user is logged in # (no data needed) simple_auth_pages = ( - '/course/', + '/home/', ) # need an activated user @@ -266,7 +266,7 @@ def test_private_pages_auth(self): def test_index_auth(self): # not logged in. Should return a redirect. - resp = self.client.get_html('/course/') + resp = self.client.get_html('/home/') self.assertEqual(resp.status_code, 302) # Logged in should work. @@ -283,7 +283,7 @@ def test_inactive_session_timeout(self): self.login(self.email, self.pw) # make sure we can access courseware immediately - course_url = '/course/' + course_url = '/home/' resp = self.client.get_html(course_url) self.assertEquals(resp.status_code, 200) @@ -293,7 +293,7 @@ def test_inactive_session_timeout(self): resp = self.client.get_html(course_url) # re-request, and we should get a redirect to login page - self.assertRedirects(resp, settings.LOGIN_REDIRECT_URL + '?next=/course/') + self.assertRedirects(resp, settings.LOGIN_REDIRECT_URL + '?next=/home/') class ForumTestCase(CourseTestCase): diff --git a/cms/djangoapps/contentstore/views/public.py b/cms/djangoapps/contentstore/views/public.py index 597bb5e18781..7bc6545868c9 100644 --- a/cms/djangoapps/contentstore/views/public.py +++ b/cms/djangoapps/contentstore/views/public.py @@ -66,6 +66,6 @@ def login_page(request): def howitworks(request): "Proxy view" if request.user.is_authenticated(): - return redirect('/course/') + return redirect('/home/') else: return render_to_response('howitworks.html', {}) diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py index 965879e15428..32b5ccbf2cf2 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_index.py @@ -42,7 +42,7 @@ def check_index_and_outline(self, authed_client): """ Test getting the list of courses and then pulling up their outlines """ - index_url = '/course/' + index_url = '/home/' index_response = authed_client.get(index_url, {}, HTTP_ACCEPT='text/html') parsed_html = lxml.html.fromstring(index_response.content) course_link_eles = parsed_html.find_class('course-link') @@ -68,7 +68,7 @@ def test_libraries_on_course_index(self): # Add a library: lib1 = LibraryFactory.create() - index_url = '/course/' + index_url = '/home/' index_response = self.client.get(index_url, {}, HTTP_ACCEPT='text/html') parsed_html = lxml.html.fromstring(index_response.content) library_link_elements = parsed_html.find_class('library-link') diff --git a/cms/templates/index.html b/cms/templates/index.html index 24652770200d..943d288a28e4 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -2,7 +2,7 @@ <%inherit file="base.html" /> <%def name="online_help_token()"><% return "home" %> -<%block name="title">${_("My Courses")} +<%block name="title">${_("Studio Home")} <%block name="bodyclass">is-signedin index view-dashboard <%block name="requirejs"> diff --git a/common/djangoapps/student/tests/test_login.py b/common/djangoapps/student/tests/test_login.py index 828e951cbbf6..8cf07e3a80b7 100644 --- a/common/djangoapps/student/tests/test_login.py +++ b/common/djangoapps/student/tests/test_login.py @@ -482,7 +482,7 @@ def test_success(self): 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) + self.assertEqual(self.client.session['_auth_user_id'], self.user.id) # pylint: disable=no-member def test_invalid_token(self): self._setup_user_response(success=False) diff --git a/common/test/acceptance/pages/lms/login_and_register.py b/common/test/acceptance/pages/lms/login_and_register.py index 1bda4fa2d817..f775066fb65c 100644 --- a/common/test/acceptance/pages/lms/login_and_register.py +++ b/common/test/acceptance/pages/lms/login_and_register.py @@ -244,6 +244,7 @@ def errors(self): def wait_for_errors(self): """Wait for errors to be visible, then return them. """ def _check_func(): + """Return success status and any errors that occurred.""" errors = self.errors return (bool(errors), errors) return Promise(_check_func, "Errors are visible").fulfill() @@ -257,6 +258,7 @@ def success(self): def wait_for_success(self): """Wait for a success message to be visible, then return it.""" def _check_func(): + """Return success status and any errors that occurred.""" success = self.success return (bool(success), success) return Promise(_check_func, "Success message is visible").fulfill() diff --git a/common/test/acceptance/tests/lms/test_lms.py b/common/test/acceptance/tests/lms/test_lms.py index 09853250bfcf..d15b0d77582f 100644 --- a/common/test/acceptance/tests/lms/test_lms.py +++ b/common/test/acceptance/tests/lms/test_lms.py @@ -119,7 +119,7 @@ def test_toggle_to_register_form(self): def test_password_reset_success(self): # Create a user account - email, password = self._create_unique_user() + email, password = self._create_unique_user() # pylint: disable=unused-variable # Navigate to the password reset form and try to submit it self.login_page.visit().password_reset(email=email) @@ -141,6 +141,9 @@ def test_password_reset_failure(self): ) def _create_unique_user(self): + """ + Create a new user with a unique name and email. + """ username = "test_{uuid}".format(uuid=self.unique_id[0:6]) email = "{user}@example.com".format(user=username) password = "password" diff --git a/docs/shared/conf.py b/docs/shared/conf.py index dfedf01ca81b..992f0e54f38c 100644 --- a/docs/shared/conf.py +++ b/docs/shared/conf.py @@ -22,7 +22,6 @@ # ----------------------------------------------------------------------------- import os -import sys BASEDIR = os.path.dirname(os.path.abspath(__file__)) diff --git a/pavelib/paver_tests/test_prereqs.py b/pavelib/paver_tests/test_prereqs.py index e4586d18434d..47437ee90768 100644 --- a/pavelib/paver_tests/test_prereqs.py +++ b/pavelib/paver_tests/test_prereqs.py @@ -1,12 +1,21 @@ - import os import unittest from pavelib.prereqs import no_prereq_install class TestPaverPrereqInstall(unittest.TestCase): - + """ + Test the status of the NO_PREREQ_INSTALL variable, its presence and how + paver handles it. + """ def check_val(self, set_val, expected_val): + """ + Verify that setting the variable to a certain value returns + the expected boolean for it. + + As environment variables are only stored as strings, we have to cast + whatever it's set at to a boolean that does not violate expectations. + """ _orig_environ = dict(os.environ) os.environ['NO_PREREQ_INSTALL'] = set_val self.assertEqual( @@ -21,19 +30,37 @@ def check_val(self, set_val, expected_val): os.environ.update(_orig_environ) def test_no_prereq_install_true(self): + """ + Ensure that 'true' will be True. + """ self.check_val('true', True) def test_no_prereq_install_false(self): + """ + Ensure that 'false' will be False. + """ self.check_val('false', False) def test_no_prereq_install_True(self): + """ + Ensure that 'True' will be True. + """ self.check_val('True', True) def test_no_prereq_install_False(self): + """ + Ensure that 'False' will be False. + """ self.check_val('False', False) def test_no_prereq_install_0(self): + """ + Ensure that '0' will be False. + """ self.check_val('0', False) def test_no_prereq_install_1(self): + """ + Ensure that '1' will be True. + """ self.check_val('1', True) diff --git a/pavelib/prereqs.py b/pavelib/prereqs.py index 0a334a815204..220a5387c4ec 100644 --- a/pavelib/prereqs.py +++ b/pavelib/prereqs.py @@ -41,7 +41,7 @@ def no_prereq_install(): try: return vals[val] - except: + except KeyError: return False From 6e67b7157f86b749c92e01c7b3c924b6692272e3 Mon Sep 17 00:00:00 2001 From: Matjaz Gregoric Date: Wed, 17 Dec 2014 12:28:34 +0100 Subject: [PATCH 08/20] Update tests. Most of the updates are related to the My Courses -> Studio Home change. --- cms/djangoapps/contentstore/features/common.py | 2 +- cms/djangoapps/contentstore/features/courses.py | 8 ++++---- .../contentstore/features/help.feature | 2 +- .../contentstore/features/signup.feature | 2 +- cms/djangoapps/contentstore/features/signup.py | 2 +- .../contentstore/tests/test_contentstore.py | 3 +-- .../js/mock/mock-index-page.underscore | 17 +++++------------ cms/templates/widgets/header.html | 2 +- common/djangoapps/terrain/ui_helpers.py | 2 +- common/test/acceptance/pages/studio/index.py | 11 ++--------- .../acceptance/tests/studio/test_studio_home.py | 10 ---------- 11 files changed, 18 insertions(+), 43 deletions(-) diff --git a/cms/djangoapps/contentstore/features/common.py b/cms/djangoapps/contentstore/features/common.py index b0681433b09d..deefb84ff199 100644 --- a/cms/djangoapps/contentstore/features/common.py +++ b/cms/djangoapps/contentstore/features/common.py @@ -171,7 +171,7 @@ def log_into_studio( world.log_in(username=uname, password=password, email=email, name=name) # Navigate to the studio dashboard world.visit('/') - assert_in(uname, world.css_text('h2.title', timeout=10)) + assert_in(uname, world.css_text('span.account-username', timeout=10)) def add_course_author(user, course): diff --git a/cms/djangoapps/contentstore/features/courses.py b/cms/djangoapps/contentstore/features/courses.py index 7b3ccbcbd2af..075d8525b56e 100644 --- a/cms/djangoapps/contentstore/features/courses.py +++ b/cms/djangoapps/contentstore/features/courses.py @@ -33,8 +33,8 @@ def i_create_a_course(step): create_a_course() -@step('I click the course link in My Courses$') -def i_click_the_course_link_in_my_courses(step): +@step('I click the course link in Studio Home$') +def i_click_the_course_link_in_studio_home(step): course_css = 'a.course-link' world.css_click(course_css) @@ -52,8 +52,8 @@ def courseware_page_has_loaded_in_studio(step): assert world.is_css_present(course_title_css) -@step('I see the course listed in My Courses$') -def i_see_the_course_in_my_courses(step): +@step('I see the course listed in Studio Home$') +def i_see_the_course_in_studio_home(step): course_css = 'h3.class-title' assert world.css_has_text(course_css, world.scenario_dict['COURSE'].display_name) diff --git a/cms/djangoapps/contentstore/features/help.feature b/cms/djangoapps/contentstore/features/help.feature index eb0f872247b3..567a2f252664 100644 --- a/cms/djangoapps/contentstore/features/help.feature +++ b/cms/djangoapps/contentstore/features/help.feature @@ -11,7 +11,7 @@ Feature: CMS.Help Scenario: Users can access online help within a course Given I have opened a new course in Studio - And I click the course link in My Courses + And I click the course link in Studio Home Then I should see online help for "outline" And I go to the course updates page diff --git a/cms/djangoapps/contentstore/features/signup.feature b/cms/djangoapps/contentstore/features/signup.feature index 92ff0d393d7d..3f29c4fda68c 100644 --- a/cms/djangoapps/contentstore/features/signup.feature +++ b/cms/djangoapps/contentstore/features/signup.feature @@ -26,4 +26,4 @@ Feature: CMS.Sign in And I visit the url "/signin?next=http://www.google.com/" When I fill in and submit the signin form And I wait for "2" seconds - Then I should see that the path is "/course/" + Then I should see that the path is "/home/" diff --git a/cms/djangoapps/contentstore/features/signup.py b/cms/djangoapps/contentstore/features/signup.py index 26374fe963f7..a661b2473c2e 100644 --- a/cms/djangoapps/contentstore/features/signup.py +++ b/cms/djangoapps/contentstore/features/signup.py @@ -24,7 +24,7 @@ def i_press_the_button_on_the_registration_form(step): @step('I should see an email verification prompt') def i_should_see_an_email_verification_prompt(step): - world.css_has_text('h1.page-header', u'My Courses') + world.css_has_text('h1.page-header', u'Studio Home') world.css_has_text('div.msg h3.title', u'We need to verify your email address') diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index e0122f4cd92d..a107d654494b 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1166,11 +1166,10 @@ def assert_course_permission_denied(self): def test_course_index_view_with_no_courses(self): """Test viewing the index page with no courses""" - # Create a course so there is something to view resp = self.client.get_html('/home/') self.assertContains( resp, - '

    My Courses

    ', + '

    Studio Home

    ', status_code=200, html=True ) diff --git a/cms/templates/js/mock/mock-index-page.underscore b/cms/templates/js/mock/mock-index-page.underscore index 83f1bc509e26..8a149ece9d23 100644 --- a/cms/templates/js/mock/mock-index-page.underscore +++ b/cms/templates/js/mock/mock-index-page.underscore @@ -1,6 +1,6 @@
    -

    My Courses

    +

    Studio Home

    diff --git a/cms/templates/library.html b/cms/templates/library.html index c58b1d4e5110..c66c7bfa4450 100644 --- a/cms/templates/library.html +++ b/cms/templates/library.html @@ -43,7 +43,19 @@

    ${context_library.display_name_with_default | h}

    +
    + + From 8aa3c4804ef2b0f44d7f9e0ec19583484a7ee78f Mon Sep 17 00:00:00 2001 From: Matjaz Gregoric Date: Sat, 3 Jan 2015 10:18:29 +0100 Subject: [PATCH 14/20] Mark Library 'mode' field non-editable. The only supported mode is currently 'random', so it doesn't make sense to expose the 'mode' in the edit form. --- common/lib/xmodule/xmodule/library_content_module.py | 9 +++++++++ .../lib/xmodule/xmodule/tests/test_library_content.py | 11 ++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index bf1e3dc8c1db..e424292857c9 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -322,6 +322,15 @@ class LibraryContentDescriptor(LibraryContentFields, MakoModuleDescriptor, XmlDe js = {'coffee': [resource_string(__name__, 'js/src/vertical/edit.coffee')]} js_module_name = "VerticalDescriptor" + @property + def non_editable_metadata_fields(self): + non_editable_fields = super(LibraryContentDescriptor, self).non_editable_metadata_fields + # The only supported mode is currently 'random'. + # Add the mode field to non_editable_metadata_fields so that it doesn't + # render in the edit form. + non_editable_fields.append(LibraryContentFields.mode) + return non_editable_fields + @XBlock.handler def refresh_children(self, request=None, suffix=None, update_db=True): # pylint: disable=unused-argument """ diff --git a/common/lib/xmodule/xmodule/tests/test_library_content.py b/common/lib/xmodule/xmodule/tests/test_library_content.py index fec957d0cda5..63dfb327b9c2 100644 --- a/common/lib/xmodule/xmodule/tests/test_library_content.py +++ b/common/lib/xmodule/xmodule/tests/test_library_content.py @@ -5,7 +5,7 @@ Higher-level tests are in `cms/djangoapps/contentstore/tests/test_libraries.py`. """ import ddt -from xmodule.library_content_module import LibraryVersionReference, ANY_CAPA_TYPE_VALUE +from xmodule.library_content_module import LibraryVersionReference, ANY_CAPA_TYPE_VALUE, LibraryContentDescriptor from xmodule.modulestore.tests.factories import LibraryFactory, CourseFactory, ItemFactory from xmodule.modulestore.tests.utils import MixedSplitTestCase from xmodule.tests import get_test_system @@ -241,3 +241,12 @@ def test_capa_type_filtering(self): self.lc_block.capa_type = ANY_CAPA_TYPE_VALUE self.lc_block.refresh_children() self.assertEqual(len(self.lc_block.children), len(self.lib_blocks) + 4) + + def test_non_editable_settings(self): + """ + Test the settings that are marked as "non-editable". + """ + non_editable_metadata_fields = self.lc_block.non_editable_metadata_fields + self.assertIn(LibraryContentDescriptor.mode, non_editable_metadata_fields) + self.assertNotIn(LibraryContentDescriptor.display_name, non_editable_metadata_fields) + From 42fac5ab0cba25e9bb95db5cb3fe9670d0200ec2 Mon Sep 17 00:00:00 2001 From: Matjaz Gregoric Date: Sat, 3 Jan 2015 10:27:57 +0100 Subject: [PATCH 15/20] Modify error message when library with same org/code exists. Users are unlikely to change their organization name. --- cms/djangoapps/contentstore/views/library.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index c599cc1559f9..9aeb9c9b13a2 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -142,7 +142,7 @@ def _create_library(request): 'ErrMsg': _( 'There is already a library defined with the same ' 'organization and library code. Please ' - 'change either organization or library code to be unique.' + 'change your library code so that it is unique within your organization.' ) }) From 8efa46806a5c1de54a7a547c430868607920d33f Mon Sep 17 00:00:00 2001 From: Matjaz Gregoric Date: Tue, 6 Jan 2015 09:56:49 +0100 Subject: [PATCH 16/20] Slower scroll to 'Add New Component'. --- cms/static/js/views/pages/container.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 179a2a052932..593821788857 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -326,7 +326,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views scrollToNewComponentButtons: function(event) { event.preventDefault(); - $.scrollTo(this.$('.add-xblock-component')); + $.scrollTo(this.$('.add-xblock-component'), {duration: 250}); } }); From 66f354d278b072c5bede25bc6dc89931dc1ec8d1 Mon Sep 17 00:00:00 2001 From: Matjaz Gregoric Date: Tue, 6 Jan 2015 10:53:49 +0100 Subject: [PATCH 17/20] Update text on new library form. --- cms/templates/index.html | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cms/templates/index.html b/cms/templates/index.html index 4ac556959179..12c60a0eb6f3 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -134,9 +134,10 @@

    ${_("Create a New Library")}

  • - - - ${_("The {em_start}major version number{em_end} of your library. Minor revisions are tracked as edits happen within a library.").format(em_start='', em_end='')} + + ## Translators: This is an example for the "code" used to identify a library, seen when filling out the form to create a new library. This example is short for "Computer Science Problems". The example number may contain letters but must not contain spaces. + + ${_("The unique code that identifies this library.")} ${_("Note: This is part of your library URL, so no spaces or special characters are allowed.")} ${_("This cannot be changed.")}
    • From 39fbac2870a745c6f4746c9f1a8e97d6911ae341 Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Tue, 6 Jan 2015 18:34:54 +0300 Subject: [PATCH 18/20] Refactoring create_course_utils and create_library_utils to reduce the amount of differences (mostly renames --- cms/static/js/index.js | 4 +- .../js/spec/views/pages/course_rerun_spec.js | 2 +- cms/static/js/views/course_rerun.js | 2 +- .../js/views/utils/create_course_utils.js | 74 ++++++++++--------- .../js/views/utils/create_library_utils.js | 31 ++++---- 5 files changed, 60 insertions(+), 53 deletions(-) diff --git a/cms/static/js/index.js b/cms/static/js/index.js index a78e0d5e90e7..74239e3a2677 100644 --- a/cms/static/js/index.js +++ b/cms/static/js/index.js @@ -60,7 +60,7 @@ define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape", "js/vie }; analytics.track('Created a Course', course_info); - CreateCourseUtils.createCourse(course_info, function (errorMessage) { + CreateCourseUtils.create(course_info, function (errorMessage) { $('.create-course .wrap-error').addClass('is-shown'); $('#course_creation_error').html('

    ' + errorMessage + '

    '); $('.new-course-save').addClass('is-disabled').attr('aria-disabled', true); @@ -114,7 +114,7 @@ define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape", "js/vie }; analytics.track('Created a Library', lib_info); - CreateLibraryUtils.createLibrary(lib_info, function (errorMessage) { + CreateLibraryUtils.create(lib_info, function (errorMessage) { $('.create-library .wrap-error').addClass('is-shown'); $('#library_creation_error').html('

    ' + errorMessage + '

    '); $('.new-library-save').addClass('is-disabled').attr('aria-disabled', true); diff --git a/cms/static/js/spec/views/pages/course_rerun_spec.js b/cms/static/js/spec/views/pages/course_rerun_spec.js index 320f8d6557ec..ba6cd799aaa0 100644 --- a/cms/static/js/spec/views/pages/course_rerun_spec.js +++ b/cms/static/js/spec/views/pages/course_rerun_spec.js @@ -62,7 +62,7 @@ define(["jquery", "js/common_helpers/ajax_helpers", "js/spec_helpers/view_helper describe("Error messages", function () { var setErrorMessage = function(selector, message) { var element = $(selector).parent(); - CreateCourseUtils.setNewCourseFieldInErr(element, message); + CreateCourseUtils.setFieldInErr(element, message); return element; }; diff --git a/cms/static/js/views/course_rerun.js b/cms/static/js/views/course_rerun.js index bfa6370d1b5f..724008c862e4 100644 --- a/cms/static/js/views/course_rerun.js +++ b/cms/static/js/views/course_rerun.js @@ -41,7 +41,7 @@ define(["domReady", "jquery", "underscore", "js/views/utils/create_course_utils" }; analytics.track('Reran a Course', course_info); - CreateCourseUtils.createCourse(course_info, function (errorMessage) { + CreateCourseUtils.create(course_info, function (errorMessage) { $('.wrapper-error').addClass('is-shown').removeClass('is-hidden'); $('#course_rerun_error').html('

    ' + errorMessage + '

    '); $('.rerun-course-save').addClass('is-disabled').removeClass('is-processing').html(gettext('Create Re-run')); diff --git a/cms/static/js/views/utils/create_course_utils.js b/cms/static/js/views/utils/create_course_utils.js index 735d22b708d8..6f0182154795 100644 --- a/cms/static/js/views/utils/create_course_utils.js +++ b/cms/static/js/views/utils/create_course_utils.js @@ -3,38 +3,42 @@ */ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], function ($, _, gettext, ViewUtils) { + "use strict"; return function (selectors, classes) { - var toggleSaveButton, validateTotalCourseItemsLength, setNewCourseFieldInErr, - hasInvalidRequiredFields, createCourse, validateFilledFields, configureHandlers; + var toggleSaveButton, validateTotalKeyLength, setFieldInErr, + hasInvalidRequiredFields, create, validateFilledFields, configureHandlers; var validateRequiredField = ViewUtils.validateRequiredField; var validateURLItemEncoding = ViewUtils.validateURLItemEncoding; - var keyLengthViolationMessage = gettext('The combined length of the organization, course number, and course run fields cannot be more than <%=limit%> characters.'); + var keyLengthViolationMessage = gettext("The combined length of the organization, course number, and course run fields cannot be more than <%=limit%> characters."); + + var keyFieldSelectors = [selectors.org, selectors.number, selectors.run]; + var nonEmptyCheckFieldSelectors = [selectors.name, selectors.org, selectors.number, selectors.run]; toggleSaveButton = function (is_enabled) { var is_disabled = !is_enabled; $(selectors.save).toggleClass(classes.disabled, is_disabled).attr('aria-disabled', is_disabled); }; - // Ensure that org, course_num and run passes checkTotalKeyLengthViolations - validateTotalCourseItemsLength = function () { + // Ensure that key fields passes checkTotalKeyLengthViolations check + validateTotalKeyLength = function () { ViewUtils.checkTotalKeyLengthViolations( selectors, classes, - [selectors.org, selectors.number, selectors.run], + keyFieldSelectors, keyLengthViolationMessage ); }; - setNewCourseFieldInErr = function (el, msg) { - if (msg) { - el.addClass(classes.error); - el.children(selectors.tipError).addClass(classes.showing).removeClass(classes.hiding).text(msg); + setFieldInErr = function (element, message) { + if (message) { + element.addClass(classes.error); + element.children(selectors.tipError).addClass(classes.showing).removeClass(classes.hiding).text(message); toggleSaveButton(false); } else { - el.removeClass(classes.error); - el.children(selectors.tipError).addClass(classes.hiding).removeClass(classes.showing); + element.removeClass(classes.error); + element.children(selectors.tipError).addClass(classes.hiding).removeClass(classes.showing); // One "error" div is always present, but hidden or shown if ($(selectors.error).length === 1) { toggleSaveButton(true); @@ -45,18 +49,18 @@ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], // One final check for empty values hasInvalidRequiredFields = function () { return _.reduce( - [selectors.name, selectors.org, selectors.number, selectors.run], - function (acc, ele) { - var $ele = $(ele); - var error = validateRequiredField($ele.val()); - setNewCourseFieldInErr($ele.parent(), error); + nonEmptyCheckFieldSelectors, + function (acc, element) { + var $element = $(element); + var error = validateRequiredField($element.val()); + setFieldInErr($element.parent(), error); return error ? true : acc; }, false ); }; - createCourse = function (courseInfo, errorHandler) { + create = function (courseInfo, errorHandler) { $.postJSON( '/course/', courseInfo, @@ -73,10 +77,10 @@ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], // Ensure that all fields are not empty validateFilledFields = function () { return _.reduce( - [selectors.org, selectors.number, selectors.run, selectors.name], - function (acc, ele) { - var $ele = $(ele); - return $ele.val().length !== 0 ? acc : false; + nonEmptyCheckFieldSelectors, + function (acc, element) { + var $element = $(element); + return $element.val().length !== 0 ? acc : false; }, true ); @@ -85,19 +89,19 @@ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], // Handle validation asynchronously configureHandlers = function () { _.each( - [selectors.org, selectors.number, selectors.run], - function (ele) { - var $ele = $(ele); - $ele.on('keyup', function (event) { + keyFieldSelectors, + function (element) { + var $element = $(element); + $element.on('keyup', function (event) { // Don't bother showing "required field" error when // the user tabs into a new field; this is distracting // and unnecessary - if (event.keyCode === 9) { + if (event.keyCode === $.ui.keyCode.TAB) { return; } - var error = validateURLItemEncoding($ele.val(), $(selectors.allowUnicode).val() === 'True'); - setNewCourseFieldInErr($ele.parent(), error); - validateTotalCourseItemsLength(); + var error = validateURLItemEncoding($element.val(), $(selectors.allowUnicode).val() === 'True'); + setFieldInErr($element.parent(), error); + validateTotalKeyLength(); if (!validateFilledFields()) { toggleSaveButton(false); } @@ -107,8 +111,8 @@ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], var $name = $(selectors.name); $name.on('keyup', function () { var error = validateRequiredField($name.val()); - setNewCourseFieldInErr($name.parent(), error); - validateTotalCourseItemsLength(); + setFieldInErr($name.parent(), error); + validateTotalKeyLength(); if (!validateFilledFields()) { toggleSaveButton(false); } @@ -116,10 +120,10 @@ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], }; return { - validateTotalCourseItemsLength: validateTotalCourseItemsLength, - setNewCourseFieldInErr: setNewCourseFieldInErr, + validateTotalKeyLength: validateTotalKeyLength, + setFieldInErr: setFieldInErr, hasInvalidRequiredFields: hasInvalidRequiredFields, - createCourse: createCourse, + create: create, validateFilledFields: validateFilledFields, configureHandlers: configureHandlers }; diff --git a/cms/static/js/views/utils/create_library_utils.js b/cms/static/js/views/utils/create_library_utils.js index 7c7b6cffa13c..d417e07d9343 100644 --- a/cms/static/js/views/utils/create_library_utils.js +++ b/cms/static/js/views/utils/create_library_utils.js @@ -5,29 +5,32 @@ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], function ($, _, gettext, ViewUtils) { "use strict"; return function (selectors, classes) { - var toggleSaveButton, validateTotalKeyLength, setNewLibraryFieldInErr, - hasInvalidRequiredFields, createLibrary, validateFilledFields, configureHandlers; + var toggleSaveButton, validateTotalKeyLength, setFieldInErr, + hasInvalidRequiredFields, create, validateFilledFields, configureHandlers; var validateRequiredField = ViewUtils.validateRequiredField; var validateURLItemEncoding = ViewUtils.validateURLItemEncoding; var keyLengthViolationMessage = gettext("The combined length of the organization and library code fields cannot be more than <%=limit%> characters."); + var keyFieldSelectors = [selectors.org, selectors.number]; + var nonEmptyCheckFieldSelectors = [selectors.name, selectors.org, selectors.number]; + toggleSaveButton = function (is_enabled) { var is_disabled = !is_enabled; $(selectors.save).toggleClass(classes.disabled, is_disabled).attr('aria-disabled', is_disabled); }; - // Ensure that org/librarycode passes validateTotalKeyLength check + // Ensure that key fields passes checkTotalKeyLengthViolations check validateTotalKeyLength = function () { ViewUtils.checkTotalKeyLengthViolations( selectors, classes, - [selectors.org, selectors.number], + keyFieldSelectors, keyLengthViolationMessage ); }; - setNewLibraryFieldInErr = function (element, message) { + setFieldInErr = function (element, message) { if (message) { element.addClass(classes.error); element.children(selectors.tipError).addClass(classes.showing).removeClass(classes.hiding).text(message); @@ -46,18 +49,18 @@ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], // One final check for empty values hasInvalidRequiredFields = function () { return _.reduce( - [selectors.name, selectors.org, selectors.number], + nonEmptyCheckFieldSelectors, function (acc, element) { var $element = $(element); var error = validateRequiredField($element.val()); - setNewLibraryFieldInErr($element.parent(), error); + setFieldInErr($element.parent(), error); return error ? true : acc; }, false ); }; - createLibrary = function (libraryInfo, errorHandler) { + create = function (libraryInfo, errorHandler) { $.postJSON( '/library/', libraryInfo @@ -80,7 +83,7 @@ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], // Ensure that all fields are not empty validateFilledFields = function () { return _.reduce( - [selectors.org, selectors.number, selectors.name], + nonEmptyCheckFieldSelectors, function (acc, element) { var $element = $(element); return $element.val().length !== 0 ? acc : false; @@ -92,7 +95,7 @@ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], // Handle validation asynchronously configureHandlers = function () { _.each( - [selectors.org, selectors.number], + keyFieldSelectors, function (element) { var $element = $(element); $element.on('keyup', function (event) { @@ -103,7 +106,7 @@ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], return; } var error = validateURLItemEncoding($element.val(), $(selectors.allowUnicode).val() === 'True'); - setNewLibraryFieldInErr($element.parent(), error); + setFieldInErr($element.parent(), error); validateTotalKeyLength(); if (!validateFilledFields()) { toggleSaveButton(false); @@ -114,7 +117,7 @@ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], var $name = $(selectors.name); $name.on('keyup', function () { var error = validateRequiredField($name.val()); - setNewLibraryFieldInErr($name.parent(), error); + setFieldInErr($name.parent(), error); validateTotalKeyLength(); if (!validateFilledFields()) { toggleSaveButton(false); @@ -124,9 +127,9 @@ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], return { validateTotalKeyLength: validateTotalKeyLength, - setNewLibraryFieldInErr: setNewLibraryFieldInErr, + setFieldInErr: setFieldInErr, hasInvalidRequiredFields: hasInvalidRequiredFields, - createLibrary: createLibrary, + create: create, validateFilledFields: validateFilledFields, configureHandlers: configureHandlers }; From f1632306209125ddce2dc08c65c07fa810c33819 Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Tue, 6 Jan 2015 20:36:18 +0000 Subject: [PATCH 19/20] Refactored shared elements of create factories into base class. --- cms/static/js/index.js | 4 +- .../js/spec/views/pages/course_rerun_spec.js | 2 +- cms/static/js/views/course_rerun.js | 2 +- .../js/views/utils/create_course_utils.js | 111 +--------------- .../js/views/utils/create_library_utils.js | 115 +--------------- .../js/views/utils/create_utils_base.js | 123 ++++++++++++++++++ 6 files changed, 137 insertions(+), 220 deletions(-) create mode 100644 cms/static/js/views/utils/create_utils_base.js diff --git a/cms/static/js/index.js b/cms/static/js/index.js index 74239e3a2677..e1c91d03a189 100644 --- a/cms/static/js/index.js +++ b/cms/static/js/index.js @@ -2,7 +2,7 @@ define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape", "js/vie "js/views/utils/create_library_utils", "js/views/utils/view_utils"], function (domReady, $, _, CancelOnEscape, CreateCourseUtilsFactory, CreateLibraryUtilsFactory, ViewUtils) { "use strict"; - var CreateCourseUtils = CreateCourseUtilsFactory({ + var CreateCourseUtils = new CreateCourseUtilsFactory({ name: '.new-course-name', org: '.new-course-org', number: '.new-course-number', @@ -21,7 +21,7 @@ define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape", "js/vie error: 'error' }); - var CreateLibraryUtils = CreateLibraryUtilsFactory({ + var CreateLibraryUtils = new CreateLibraryUtilsFactory({ name: '.new-library-name', org: '.new-library-org', number: '.new-library-number', diff --git a/cms/static/js/spec/views/pages/course_rerun_spec.js b/cms/static/js/spec/views/pages/course_rerun_spec.js index ba6cd799aaa0..01a5b9c858a5 100644 --- a/cms/static/js/spec/views/pages/course_rerun_spec.js +++ b/cms/static/js/spec/views/pages/course_rerun_spec.js @@ -26,7 +26,7 @@ define(["jquery", "js/common_helpers/ajax_helpers", "js/spec_helpers/view_helper }, mockCreateCourseRerunHTML = readFixtures('mock/mock-create-course-rerun.underscore'); - var CreateCourseUtils = CreateCourseUtilsFactory(selectors, classes); + var CreateCourseUtils = new CreateCourseUtilsFactory(selectors, classes); var fillInFields = function (org, number, run, name) { $(selectors.org).val(org); diff --git a/cms/static/js/views/course_rerun.js b/cms/static/js/views/course_rerun.js index 724008c862e4..ee99ee2c8102 100644 --- a/cms/static/js/views/course_rerun.js +++ b/cms/static/js/views/course_rerun.js @@ -1,6 +1,6 @@ define(["domReady", "jquery", "underscore", "js/views/utils/create_course_utils", "js/views/utils/view_utils"], function (domReady, $, _, CreateCourseUtilsFactory, ViewUtils) { - var CreateCourseUtils = CreateCourseUtilsFactory({ + var CreateCourseUtils = new CreateCourseUtilsFactory({ name: '.rerun-course-name', org: '.rerun-course-org', number: '.rerun-course-number', diff --git a/cms/static/js/views/utils/create_course_utils.js b/cms/static/js/views/utils/create_course_utils.js index 6f0182154795..68ee1c7a70bb 100644 --- a/cms/static/js/views/utils/create_course_utils.js +++ b/cms/static/js/views/utils/create_course_utils.js @@ -1,66 +1,17 @@ /** * Provides utilities for validating courses during creation, for both new courses and reruns. */ -define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], - function ($, _, gettext, ViewUtils) { +define(["jquery", "gettext", "js/views/utils/view_utils", "js/views/utils/create_utils_base"], + function ($, gettext, ViewUtils, CreateUtilsFactory) { "use strict"; return function (selectors, classes) { - var toggleSaveButton, validateTotalKeyLength, setFieldInErr, - hasInvalidRequiredFields, create, validateFilledFields, configureHandlers; - - var validateRequiredField = ViewUtils.validateRequiredField; - var validateURLItemEncoding = ViewUtils.validateURLItemEncoding; - var keyLengthViolationMessage = gettext("The combined length of the organization, course number, and course run fields cannot be more than <%=limit%> characters."); - var keyFieldSelectors = [selectors.org, selectors.number, selectors.run]; var nonEmptyCheckFieldSelectors = [selectors.name, selectors.org, selectors.number, selectors.run]; - toggleSaveButton = function (is_enabled) { - var is_disabled = !is_enabled; - $(selectors.save).toggleClass(classes.disabled, is_disabled).attr('aria-disabled', is_disabled); - }; - - // Ensure that key fields passes checkTotalKeyLengthViolations check - validateTotalKeyLength = function () { - ViewUtils.checkTotalKeyLengthViolations( - selectors, classes, - keyFieldSelectors, - keyLengthViolationMessage - ); - }; - - setFieldInErr = function (element, message) { - if (message) { - element.addClass(classes.error); - element.children(selectors.tipError).addClass(classes.showing).removeClass(classes.hiding).text(message); - toggleSaveButton(false); - } - else { - element.removeClass(classes.error); - element.children(selectors.tipError).addClass(classes.hiding).removeClass(classes.showing); - // One "error" div is always present, but hidden or shown - if ($(selectors.error).length === 1) { - toggleSaveButton(true); - } - } - }; + CreateUtilsFactory.call(this, selectors, classes, keyLengthViolationMessage, keyFieldSelectors, nonEmptyCheckFieldSelectors); - // One final check for empty values - hasInvalidRequiredFields = function () { - return _.reduce( - nonEmptyCheckFieldSelectors, - function (acc, element) { - var $element = $(element); - var error = validateRequiredField($element.val()); - setFieldInErr($element.parent(), error); - return error ? true : acc; - }, - false - ); - }; - - create = function (courseInfo, errorHandler) { + this.create = function (courseInfo, errorHandler) { $.postJSON( '/course/', courseInfo, @@ -73,59 +24,5 @@ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], } ); }; - - // Ensure that all fields are not empty - validateFilledFields = function () { - return _.reduce( - nonEmptyCheckFieldSelectors, - function (acc, element) { - var $element = $(element); - return $element.val().length !== 0 ? acc : false; - }, - true - ); - }; - - // Handle validation asynchronously - configureHandlers = function () { - _.each( - keyFieldSelectors, - function (element) { - var $element = $(element); - $element.on('keyup', function (event) { - // Don't bother showing "required field" error when - // the user tabs into a new field; this is distracting - // and unnecessary - if (event.keyCode === $.ui.keyCode.TAB) { - return; - } - var error = validateURLItemEncoding($element.val(), $(selectors.allowUnicode).val() === 'True'); - setFieldInErr($element.parent(), error); - validateTotalKeyLength(); - if (!validateFilledFields()) { - toggleSaveButton(false); - } - }); - } - ); - var $name = $(selectors.name); - $name.on('keyup', function () { - var error = validateRequiredField($name.val()); - setFieldInErr($name.parent(), error); - validateTotalKeyLength(); - if (!validateFilledFields()) { - toggleSaveButton(false); - } - }); - }; - - return { - validateTotalKeyLength: validateTotalKeyLength, - setFieldInErr: setFieldInErr, - hasInvalidRequiredFields: hasInvalidRequiredFields, - create: create, - validateFilledFields: validateFilledFields, - configureHandlers: configureHandlers - }; }; }); diff --git a/cms/static/js/views/utils/create_library_utils.js b/cms/static/js/views/utils/create_library_utils.js index d417e07d9343..9cda9580dae6 100644 --- a/cms/static/js/views/utils/create_library_utils.js +++ b/cms/static/js/views/utils/create_library_utils.js @@ -1,66 +1,17 @@ /** * Provides utilities for validating libraries during creation. */ -define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], - function ($, _, gettext, ViewUtils) { +define(["jquery", "gettext", "js/views/utils/view_utils", "js/views/utils/create_utils_base"], + function ($, gettext, ViewUtils, CreateUtilsFactory) { "use strict"; return function (selectors, classes) { - var toggleSaveButton, validateTotalKeyLength, setFieldInErr, - hasInvalidRequiredFields, create, validateFilledFields, configureHandlers; - - var validateRequiredField = ViewUtils.validateRequiredField; - var validateURLItemEncoding = ViewUtils.validateURLItemEncoding; - - var keyLengthViolationMessage = gettext("The combined length of the organization and library code fields cannot be more than <%=limit%> characters."); - + var keyLengthViolationMessage = gettext("The combined length of the organization and library code fields cannot be more than <%=limit%> characters.") var keyFieldSelectors = [selectors.org, selectors.number]; var nonEmptyCheckFieldSelectors = [selectors.name, selectors.org, selectors.number]; - toggleSaveButton = function (is_enabled) { - var is_disabled = !is_enabled; - $(selectors.save).toggleClass(classes.disabled, is_disabled).attr('aria-disabled', is_disabled); - }; - - // Ensure that key fields passes checkTotalKeyLengthViolations check - validateTotalKeyLength = function () { - ViewUtils.checkTotalKeyLengthViolations( - selectors, classes, - keyFieldSelectors, - keyLengthViolationMessage - ); - }; + CreateUtilsFactory.call(this, selectors, classes, keyLengthViolationMessage, keyFieldSelectors, nonEmptyCheckFieldSelectors); - setFieldInErr = function (element, message) { - if (message) { - element.addClass(classes.error); - element.children(selectors.tipError).addClass(classes.showing).removeClass(classes.hiding).text(message); - toggleSaveButton(false); - } - else { - element.removeClass(classes.error); - element.children(selectors.tipError).addClass(classes.hiding).removeClass(classes.showing); - // One "error" div is always present, but hidden or shown - if ($(selectors.error).length === 1) { - toggleSaveButton(true); - } - } - }; - - // One final check for empty values - hasInvalidRequiredFields = function () { - return _.reduce( - nonEmptyCheckFieldSelectors, - function (acc, element) { - var $element = $(element); - var error = validateRequiredField($element.val()); - setFieldInErr($element.parent(), error); - return error ? true : acc; - }, - false - ); - }; - - create = function (libraryInfo, errorHandler) { + this.create = function (libraryInfo, errorHandler) { $.postJSON( '/library/', libraryInfo @@ -78,60 +29,6 @@ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], } errorHandler(reason); }); - }; - - // Ensure that all fields are not empty - validateFilledFields = function () { - return _.reduce( - nonEmptyCheckFieldSelectors, - function (acc, element) { - var $element = $(element); - return $element.val().length !== 0 ? acc : false; - }, - true - ); - }; - - // Handle validation asynchronously - configureHandlers = function () { - _.each( - keyFieldSelectors, - function (element) { - var $element = $(element); - $element.on('keyup', function (event) { - // Don't bother showing "required field" error when - // the user tabs into a new field; this is distracting - // and unnecessary - if (event.keyCode === $.ui.keyCode.TAB) { - return; - } - var error = validateURLItemEncoding($element.val(), $(selectors.allowUnicode).val() === 'True'); - setFieldInErr($element.parent(), error); - validateTotalKeyLength(); - if (!validateFilledFields()) { - toggleSaveButton(false); - } - }); - } - ); - var $name = $(selectors.name); - $name.on('keyup', function () { - var error = validateRequiredField($name.val()); - setFieldInErr($name.parent(), error); - validateTotalKeyLength(); - if (!validateFilledFields()) { - toggleSaveButton(false); - } - }); - }; - - return { - validateTotalKeyLength: validateTotalKeyLength, - setFieldInErr: setFieldInErr, - hasInvalidRequiredFields: hasInvalidRequiredFields, - create: create, - validateFilledFields: validateFilledFields, - configureHandlers: configureHandlers - }; + } }; }); diff --git a/cms/static/js/views/utils/create_utils_base.js b/cms/static/js/views/utils/create_utils_base.js new file mode 100644 index 000000000000..480587223b81 --- /dev/null +++ b/cms/static/js/views/utils/create_utils_base.js @@ -0,0 +1,123 @@ +/** + * Mixin class for creation of things like courses and libraries. + */ +define(["jquery", "underscore", "gettext", "js/views/utils/view_utils"], + function ($, _, gettext, ViewUtils) { + return function (selectors, classes, keyLengthViolationMessage, keyFieldSelectors, nonEmptyCheckFieldSelectors) { + var self = this; + + this.selectors = selectors; + this.classes = classes; + this.validateRequiredField = ViewUtils.validateRequiredField; + this.validateURLItemEncoding = ViewUtils.validateURLItemEncoding; + this.keyLengthViolationMessage = keyLengthViolationMessage; + // Key fields for your model, like [selectors.org, selectors.number] + this.keyFieldSelectors = keyFieldSelectors; + // Fields that must not be empty on your model. + this.nonEmptyCheckFieldSelectors = nonEmptyCheckFieldSelectors; + + this.create = function (courseInfo, errorHandler) { + // Replace this with a function that will make a request to create the object. + }; + + // Ensure that key fields passes checkTotalKeyLengthViolations check + this.validateTotalKeyLength = function () { + ViewUtils.checkTotalKeyLengthViolations( + self.selectors, self.classes, + self.keyFieldSelectors, + self.keyLengthViolationMessage + ); + }; + + this.toggleSaveButton = function (is_enabled) { + var is_disabled = !is_enabled; + $(self.selectors.save).toggleClass(self.classes.disabled, is_disabled).attr('aria-disabled', is_disabled); + }; + + this.setFieldInErr = function (element, message) { + if (message) { + element.addClass(self.classes.error); + element.children(self.selectors.tipError).addClass(self.classes.showing).removeClass(self.classes.hiding).text(message); + self.toggleSaveButton(false); + } + else { + element.removeClass(self.classes.error); + element.children(self.selectors.tipError).addClass(self.classes.hiding).removeClass(self.classes.showing); + // One "error" div is always present, but hidden or shown + if ($(self.selectors.error).length === 1) { + self.toggleSaveButton(true); + } + } + }; + + // One final check for empty values + this.hasInvalidRequiredFields = function () { + return _.reduce( + self.nonEmptyCheckFieldSelectors, + function (acc, element) { + var $element = $(element); + var error = self.validateRequiredField($element.val()); + self.setFieldInErr($element.parent(), error); + return error ? true : acc; + }, + false + ); + }; + + // Ensure that all fields are not empty + this.validateFilledFields = function () { + return _.reduce( + self.nonEmptyCheckFieldSelectors, + function (acc, element) { + var $element = $(element); + return $element.val().length !== 0 ? acc : false; + }, + true + ); + }; + + // Handle validation asynchronously + this.configureHandlers = function () { + _.each( + self.keyFieldSelectors, + function (element) { + var $element = $(element); + $element.on('keyup', function (event) { + // Don't bother showing "required field" error when + // the user tabs into a new field; this is distracting + // and unnecessary + if (event.keyCode === $.ui.keyCode.TAB) { + return; + } + var error = self.validateURLItemEncoding($element.val(), $(self.selectors.allowUnicode).val() === 'True'); + self.setFieldInErr($element.parent(), error); + self.validateTotalKeyLength(); + if (!self.validateFilledFields()) { + self.toggleSaveButton(false); + } + }); + } + ); + + var $name = $(self.selectors.name); + $name.on('keyup', function () { + var error = self.validateRequiredField($name.val()); + self.setFieldInErr($name.parent(), error); + self.validateTotalKeyLength(); + if (!self.validateFilledFields()) { + self.toggleSaveButton(false); + } + }); + }; + + return { + validateTotalKeyLength: self.validateTotalKeyLength, + setFieldInErr: self.setFieldInErr, + hasInvalidRequiredFields: self.hasInvalidRequiredFields, + create: self.create, + validateFilledFields: self.validateFilledFields, + configureHandlers: self.configureHandlers + }; + } + } +); From b65f158ed51f421c1d5e718acc755cbd61f42dd4 Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Wed, 7 Jan 2015 20:05:44 +0000 Subject: [PATCH 20/20] Quality fixes. --- cms/djangoapps/contentstore/features/courses.py | 2 ++ common/djangoapps/terrain/ui_helpers.py | 1 + common/lib/xmodule/xmodule/tests/test_library_content.py | 1 - 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/features/courses.py b/cms/djangoapps/contentstore/features/courses.py index 075d8525b56e..2640b5c233b8 100644 --- a/cms/djangoapps/contentstore/features/courses.py +++ b/cms/djangoapps/contentstore/features/courses.py @@ -33,6 +33,7 @@ def i_create_a_course(step): create_a_course() +# pylint disable=unused-argument, invalid-name @step('I click the course link in Studio Home$') def i_click_the_course_link_in_studio_home(step): course_css = 'a.course-link' @@ -53,6 +54,7 @@ def courseware_page_has_loaded_in_studio(step): @step('I see the course listed in Studio Home$') +# pylint disable=unused-argument def i_see_the_course_in_studio_home(step): course_css = 'h3.class-title' assert world.css_has_text(course_css, world.scenario_dict['COURSE'].display_name) diff --git a/common/djangoapps/terrain/ui_helpers.py b/common/djangoapps/terrain/ui_helpers.py index 57e90820f8b1..f13811467adb 100644 --- a/common/djangoapps/terrain/ui_helpers.py +++ b/common/djangoapps/terrain/ui_helpers.py @@ -48,6 +48,7 @@ "js/base", "js/models/course", "js/models/location", "js/models/section"], # Dashboard + # pylint disable=anomalous-backslash-in-string re.compile('^Studio Home \|'): [ "js/sock", "gettext", "js/base", "jquery.ui", "coffee/src/main", "underscore"], diff --git a/common/lib/xmodule/xmodule/tests/test_library_content.py b/common/lib/xmodule/xmodule/tests/test_library_content.py index 63dfb327b9c2..7924cde65651 100644 --- a/common/lib/xmodule/xmodule/tests/test_library_content.py +++ b/common/lib/xmodule/xmodule/tests/test_library_content.py @@ -249,4 +249,3 @@ def test_non_editable_settings(self): non_editable_metadata_fields = self.lc_block.non_editable_metadata_fields self.assertIn(LibraryContentDescriptor.mode, non_editable_metadata_fields) self.assertNotIn(LibraryContentDescriptor.display_name, non_editable_metadata_fields) -