From ca64c665cc6de2888b1495958e947b5068289282 Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Thu, 14 May 2015 16:54:20 -0400 Subject: [PATCH] Default course license to All Rights Reserved Clicking on conflicting option box unchecks all conflicts LMS: Clicking license text should bring to new window updated styles to reflect html reuse inside of xblock edit modal area. Add ARIA attributes to license for a11y Gracefully handle re-selecting of selected license --- .../models/settings/course_details.py | 5 +- cms/static/js/spec/views/license_spec.js | 28 ++---- cms/static/js/views/license.js | 37 ++++--- cms/static/sass/elements/_xblocks.scss | 93 ++++++++++++++++++ cms/static/sass/views/_settings.scss | 97 ------------------- cms/templates/js/license-selector.underscore | 13 +-- common/lib/xmodule/xmodule/course_module.py | 5 +- common/lib/xmodule/xmodule/mixin.py | 4 +- common/templates/license.html | 11 ++- .../test/acceptance/pages/lms/courseware.py | 3 + .../test/acceptance/pages/studio/container.py | 3 + .../test/acceptance/pages/studio/overview.py | 15 +-- .../test/acceptance/pages/studio/settings.py | 14 +++ .../tests/studio/test_studio_settings.py | 18 ++-- .../tests/video/test_video_license.py | 12 ++- lms/envs/common.py | 3 + lms/envs/devstack.py | 3 + lms/templates/courseware/courseware.html | 7 +- 18 files changed, 202 insertions(+), 169 deletions(-) diff --git a/cms/djangoapps/models/settings/course_details.py b/cms/djangoapps/models/settings/course_details.py index 5e0b93fb84..e2f8119841 100644 --- a/cms/djangoapps/models/settings/course_details.py +++ b/cms/djangoapps/models/settings/course_details.py @@ -42,7 +42,7 @@ class CourseDetails(object): self.overview = "" # html to render as the overview self.intro_video = None # a video pointer self.effort = None # int hours/week - self.license = None + self.license = "all-rights-reserved" # default course license is all rights reserved self.course_image_name = "" self.course_image_asset_path = "" # URL of the course image self.pre_requisite_courses = [] # pre-requisite courses @@ -80,7 +80,8 @@ class CourseDetails(object): course_details.pre_requisite_courses = descriptor.pre_requisite_courses course_details.course_image_name = descriptor.course_image course_details.course_image_asset_path = course_image_url(descriptor) - course_details.license = getattr(descriptor, "license", None) + # Default course license is "All Rights Reserved" + course_details.license = getattr(descriptor, "license", "all-rights-reserved") for attribute in ABOUT_ATTRIBUTES: value = cls._fetch_about_attribute(course_key, attribute) diff --git a/cms/static/js/spec/views/license_spec.js b/cms/static/js/spec/views/license_spec.js index 510a33fba1..dd56cc1928 100644 --- a/cms/static/js/spec/views/license_spec.js +++ b/cms/static/js/spec/views/license_spec.js @@ -106,33 +106,18 @@ define(["js/views/license", "js/models/license", "js/common_helpers/template_hel ); // SA and ND conflict var SA = this.view.$("li[data-option=SA]"); - expect(SA).toHaveClass("is-disabled"); - // try to turn on SA option, fail - SA.click() - // no change - expect(this.model.get("options")).toEqual( - {"ver": "4.0", "BY": true, "NC": true, "ND": true, "SA": false} - ); - // turn off ND - var ND = this.view.$("li[data-option=ND]"); - expect(ND).not.toHaveClass("is-disabled"); - ND.click() - expect(this.model.get("options")).toEqual( - {"ver": "4.0", "BY": true, "NC": true, "ND": false, "SA": false} - ); - // turn on SA - SA = this.view.$("li[data-option=SA]"); - expect(SA).not.toHaveClass("is-disabled"); + // try to turn on SA option SA.click() + // ND should no longer be selected expect(this.model.get("options")).toEqual( {"ver": "4.0", "BY": true, "NC": true, "ND": false, "SA": true} ); - // try to turn on ND option, fail + + // try to turn on ND option ND = this.view.$("li[data-option=ND]"); - expect(ND).toHaveClass("is-disabled"); ND.click(); expect(this.model.get("options")).toEqual( - {"ver": "4.0", "BY": true, "NC": true, "ND": false, "SA": true} + {"ver": "4.0", "BY": true, "NC": true, "ND": true, "SA": false} ); }); @@ -147,7 +132,8 @@ define(["js/views/license", "js/models/license", "js/common_helpers/template_hel this.view = new LicenseView({model: this.model, showPreview: true}); this.view.render() expect(this.view.$(".license-preview").length).toEqual(1) - expect(this.view.$(".license-preview")).toHaveText(""); + // Expect default text to be "All Rights Reserved" + expect(this.view.$(".license-preview")).toContainText("All Rights Reserved"); this.view.$("li[data-license=creative-commons] button").click(); expect(this.view.$(".license-preview").length).toEqual(1) expect(this.view.$(".license-preview")).toContainText("Some Rights Reserved"); diff --git a/cms/static/js/views/license.js b/cms/static/js/views/license.js index 013b79e8b5..3e8f9a1cdf 100644 --- a/cms/static/js/views/license.js +++ b/cms/static/js/views/license.js @@ -92,10 +92,17 @@ define(["js/views/baseview", "underscore"], function(BaseView, _) { onLicenseClick: function(e) { var $li = $(e.srcElement || e.target).closest('li'); var licenseType = $li.data("license"); - this.model.set({ - "type": licenseType, - "options": this.getDefaultOptionsForLicenseType(licenseType) - }); + + // Check that we've selected a different license type than what's currently selected + if (licenseType != this.model.attributes.type) { + this.model.set({ + "type": licenseType, + "options": this.getDefaultOptionsForLicenseType(licenseType) + }); + // Fire the change event manually + this.model.trigger("change change:type") + } + e.preventDefault(); }, onOptionClick: function(e) { @@ -117,17 +124,19 @@ define(["js/views/baseview", "underscore"], function(BaseView, _) { licenseOptions[optionKey] = currentOptionValue; } // check for conflicts - if (currentOptionValue && optionInfo.conflictsWith && - _.any(optionInfo.conflictsWith, function (key) { return licenseOptions[key];})) { - // conflict! don't set new options - // need some feedback here - return; - } else { - this.model.set({"options": licenseOptions}) - // Backbone has trouble identifying when objects change, so we'll - // fire the change event manually. - this.model.trigger("change change:options") + if (currentOptionValue && optionInfo.conflictsWith) { + var conflicts = optionInfo.conflictsWith; + for (var i=0; i <% if (optionInfo.type == "boolean") { %> <% var optionSelected = model.options[optionKey]; %> - <% var optionDisabled = optionInfo.disabled || - (optionInfo.conflictsWith && _.any(optionInfo.conflictsWith, function(key) {return model.options[key];})); - %> + <% var optionDisabled = optionInfo.disabled %>
  • @@ -76,7 +74,7 @@ <%= gettext("License Display") %>

    - <%= gettext("The following message will be displayed at the bottom of the courseware pages within your course.") %> + <%= gettext("The following message will be displayed at the bottom of the courseware pages within your course:") %>

    <% // keep this synchronized with the contents of common/templates/license.html %> @@ -99,14 +97,17 @@ alt="<%- typeof licenseString == "string" ? licenseString : "" %>" /> <% } else { %> - + <% // must come before icon or else spacing gets messed up %> + gettext("Creative Commons licensed content, with terms as follow:")  <% _.each(enabled, function(option) { %> - + <%- license.options[option.toUpperCase()].name %>  <% }); %> <%= gettext("Some Rights Reserved") %> <% } %> <% } else { %> <%= typeof licenseString == "string" ? licenseString : "" %> + <% // Default to ARR license %> + © <%= gettext("All Rights Reserved") %> <% } %>
    diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index bc9995c1a3..6a4788da7c 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -1023,8 +1023,9 @@ class CourseDescriptor(CourseFields, LicenseMixin, SequenceDescriptor): wiki_xml_object.set('slug', self.wiki_slug) xml_object.append(wiki_xml_object) - # handle license specifically - self.add_license_to_xml(xml_object) + # handle license specifically. Default the course to have a license + # of "All Rights Reserved", if a license is not explicitly set. + self.add_license_to_xml(xml_object, default="all-rights-reserved") return xml_object diff --git a/common/lib/xmodule/xmodule/mixin.py b/common/lib/xmodule/xmodule/mixin.py index 7468ccb512..361bd20688 100644 --- a/common/lib/xmodule/xmodule/mixin.py +++ b/common/lib/xmodule/xmodule/mixin.py @@ -41,7 +41,7 @@ class LicenseMixin(XBlockMixin): definition['license'] = license return definition - def add_license_to_xml(self, node): + def add_license_to_xml(self, node, default=None): """ When generating XML from an XBlock, this method will add the XBlock's license to the XML representation before it is serialized. @@ -49,7 +49,7 @@ class LicenseMixin(XBlockMixin): to this method, rather than reimplementing it in their XML export functions. """ - if getattr(self, "license", None): + if getattr(self, "license", default): node.set('license', self.license) diff --git a/common/templates/license.html b/common/templates/license.html index f0670a3a24..77a129e7a1 100644 --- a/common/templates/license.html +++ b/common/templates/license.html @@ -29,6 +29,10 @@ def parse_license(lic): % elif license_type == "creative-commons": <% possible = ["by", "nc", "nd", "sa"] + names = { + "by": _("Attribution"), "nc": _("Noncommercial"), + "nd": _("No Derivatives"), "sa": _("Share Alike") + } enabled = [opt for opt in possible if license_options.get(opt) or license_options.get(opt.upper())] version = license_options.get("ver", "4.0") @@ -36,16 +40,17 @@ def parse_license(lic): enabled = ["zero"] version = license_options.get("ver", "1.0") %> - + % if button: ${license} % else: - + ## must come before icon or else spacing gets messed up + ${_("Creative Commons licensed content, with terms as follow:")}  % for option in enabled: - + ${names[option]}  % endfor ${_("Some Rights Reserved")} % endif diff --git a/common/test/acceptance/pages/lms/courseware.py b/common/test/acceptance/pages/lms/courseware.py index 1d05324508..d5d3f09185 100644 --- a/common/test/acceptance/pages/lms/courseware.py +++ b/common/test/acceptance/pages/lms/courseware.py @@ -90,6 +90,9 @@ class CoursewarePage(CoursePage): @property def course_license(self): + """ + Returns the course license text, if present. Else returns None. + """ element = self.q(css="#content .container-footer .course-license") if element.is_present(): return element.text[0] diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py index e1b7b586b5..a33b0a2d19 100644 --- a/common/test/acceptance/pages/studio/container.py +++ b/common/test/acceptance/pages/studio/container.py @@ -488,6 +488,9 @@ class XBlockWrapper(PageObject): type_in_codemirror(self, index, text, find_prefix='$("{}").find'.format(self.editor_selector)) def set_license(self, license_type): + """ + Uses the UI to set the course's license to the given license_type (str) + """ css_selector = ( "ul.license-types li[data-license={license_type}] button" ).format(license_type=license_type) diff --git a/common/test/acceptance/pages/studio/overview.py b/common/test/acceptance/pages/studio/overview.py index 3955d5b8fd..387e42e614 100644 --- a/common/test/acceptance/pages/studio/overview.py +++ b/common/test/acceptance/pages/studio/overview.py @@ -12,7 +12,6 @@ from selenium.webdriver.common.action_chains import ActionChains from .course_page import CoursePage from .container import ContainerPage -from .settings import SettingsPage from .utils import set_input_value_and_save, set_input_value, click_css, confirm_prompt @@ -582,19 +581,11 @@ class CourseOutlinePage(CoursePage, CourseOutlineContainer): @property def license(self): + """ + Returns the course license text, if present. Else returns None. + """ return self.q(css=".license-value").first.text[0] - def edit_course_start_date(self): - self.q(css=".status-release .action-edit a.action-button").click() - sp = SettingsPage( - self.browser, - self.course_info['course_org'], - self.course_info['course_num'], - self.course_info['course_run'], - ) - sp.wait_for_page() - return sp - class CourseOutlineModal(object): MODAL_SELECTOR = ".wrapper-modal-window" diff --git a/common/test/acceptance/pages/studio/settings.py b/common/test/acceptance/pages/studio/settings.py index 22ee6f3e5c..f3c0bc0a77 100644 --- a/common/test/acceptance/pages/studio/settings.py +++ b/common/test/acceptance/pages/studio/settings.py @@ -81,6 +81,10 @@ class SettingsPage(CoursePage): @property def course_license(self): + """ + Property. Returns the text of the license type for the course + ("All Rights Reserved" or "Creative Commons") + """ license_types_css = "section.license ul.license-types li.license-type" self.wait_for_element_presence( license_types_css, @@ -89,10 +93,20 @@ class SettingsPage(CoursePage): selected = self.q(css=license_types_css + " button.is-selected") if selected.is_present(): return selected.text[0] + + # Look for the license text that will be displayed by default, + # if no button is yet explicitly selected + license_text = self.q(css='section.license span.license-text') + if license_text.is_present(): + return license_text.text[0] return None @course_license.setter def course_license(self, license_name): + """ + Sets the course license to the given license_name + (str, "All Rights Reserved" or "Creative Commons") + """ license_types_css = "section.license ul.license-types li.license-type" self.wait_for_element_presence( license_types_css, diff --git a/common/test/acceptance/tests/studio/test_studio_settings.py b/common/test/acceptance/tests/studio/test_studio_settings.py index 2c86a6fa1f..a51c3148c3 100644 --- a/common/test/acceptance/tests/studio/test_studio_settings.py +++ b/common/test/acceptance/tests/studio/test_studio_settings.py @@ -406,7 +406,11 @@ class AdvancedSettingsValidationTest(StudioCourseTest): @attr('shard_1') class ContentLicenseTest(StudioCourseTest): - def setUp(self): + """ + Tests for course-level licensing (that is, setting the license, + for an entire course's content, to All Rights Reserved or Creative Commons) + """ + def setUp(self): # pylint: disable=arguments-differ super(ContentLicenseTest, self).setUp() self.outline_page = CourseOutlinePage( self.browser, @@ -429,13 +433,13 @@ class ContentLicenseTest(StudioCourseTest): def test_empty_license(self): """ When I visit the Studio settings page, - I see that the course license is "None" by default. + I see that the course license is "All Rights Reserved" by default. Then I visit the LMS courseware page, - and I see that there is no course license displayed. + and I see that the default course license is displayed. """ - self.assertIsNone(self.settings_page.course_license) + self.assertEqual(self.settings_page.course_license, "All Rights Reserved") self.lms_courseware.visit() - self.assertIsNone(self.lms_courseware.course_license) + self.assertEqual(self.lms_courseware.course_license, "© All Rights Reserved") def test_arr_license(self): """ @@ -469,4 +473,6 @@ class ContentLicenseTest(StudioCourseTest): self.assertEqual(self.settings_page.course_license, "Creative Commons") self.lms_courseware.visit() - self.assertEqual(self.lms_courseware.course_license, "Some Rights Reserved") + # The course_license text will include a bunch of screen reader text to explain + # the selected options + self.assertIn("Some Rights Reserved", self.lms_courseware.course_license) diff --git a/common/test/acceptance/tests/video/test_video_license.py b/common/test/acceptance/tests/video/test_video_license.py index 866f2d8177..e07d49acb2 100644 --- a/common/test/acceptance/tests/video/test_video_license.py +++ b/common/test/acceptance/tests/video/test_video_license.py @@ -1,4 +1,7 @@ # coding: utf-8 +""" +Acceptance tests for licensing of the Video module +""" from __future__ import unicode_literals from nose.plugins.attrib import attr from ..studio.base_studio_test import StudioCourseTest @@ -11,8 +14,11 @@ from ...fixtures.course import XBlockFixtureDesc @attr('shard_1') class VideoLicenseTest(StudioCourseTest): - - def setUp(self): + """ + Tests for video module-level licensing (that is, setting the license, + for a specific video module, to All Rights Reserved or Creative Commons) + """ + def setUp(self): # pylint: disable=arguments-differ super(VideoLicenseTest, self).setUp() self.lms_courseware = CoursewarePage( @@ -107,4 +113,4 @@ class VideoLicenseTest(StudioCourseTest): self.assertTrue(video.is_present()) video_license = self.lms_courseware.q(css=".vert .xblock.xmodule_VideoModule .xblock-license") self.assertTrue(video_license.is_present()) - self.assertEqual(video_license.text[0], "Some Rights Reserved") + self.assertIn("Some Rights Reserved", video_license.text[0]) diff --git a/lms/envs/common.py b/lms/envs/common.py index 23282b00ac..063f1ba61b 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -369,6 +369,9 @@ FEATURES = { # enable beacons for lms onload event statistics 'ENABLE_ONLOAD_BEACON': False, + # Toggle platform-wide course licensing + 'LICENSING': False, + # Certificates Web/HTML Views 'CERTIFICATES_HTML_VIEW': False, diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index 577ab95b3b..a1eb5b9f58 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -115,6 +115,9 @@ FEATURES['MILESTONES_APP'] = True ########################### Entrance Exams ################################# FEATURES['ENTRANCE_EXAMS'] = True +################################ COURSE LICENSES ################################ +FEATURES['LICENSING'] = True + ########################## Courseware Search ####################### FEATURES['ENABLE_COURSEWARE_SEARCH'] = True diff --git a/lms/templates/courseware/courseware.html b/lms/templates/courseware/courseware.html index 7a6c43ba01..3aa2f8acb7 100644 --- a/lms/templates/courseware/courseware.html +++ b/lms/templates/courseware/courseware.html @@ -207,9 +207,14 @@ ${fragment.foot_html()}
    - % if settings.FEATURES.get("LICENSING", False) and getattr(course, "license", None): + % if settings.FEATURES.get("LICENSING", False):
    + % if getattr(course, "license", None): <%include file="../license.html" args="license=course.license" /> + % else: + ## Default course license: All Rights Reserved, if none is explicitly set. + <%include file="../license.html" args="license='all-rights-reserved'" /> + % endif
    % endif