% if context_course.course_image:
- 
+
@@ -328,7 +352,7 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url}';
% else:
-
+
${_("Your course currently does not have an image. Please upload one (JPEG or PNG format, and minimum suggested dimensions are 375px wide by 200px tall)")}
% endif
@@ -340,13 +364,75 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url}';
${_("Please provide a valid path and name to your course image (Note: only JPEG or PNG format supported)")}
-
+
+ % if context_course.banner_image:
+
+ 
+
+
+
+ ${_("You can manage this image along with all of your other files & uploads").format(upload_asset_url)}
+
+
+ % else:
+
+
+
+ ${_("Your course currently does not have an image. Please upload one (JPEG or PNG format, and minimum suggested dimensions are 1440px wide by 400px tall)")}
+ % endif
+
+
+
+
+
+ ## Translators: This is the placeholder text for a field that requests the URL for a course banner image
+
+ ${_("Please provide a valid path and name to your banner image (Note: only JPEG or PNG format supported)")}
+
+
+
+ % if context_course.video_thumbnail_image:
+
+ 
+
+
+
+ ${_("You can manage this image along with all of your other files & uploads").format(upload_asset_url)}
+
+
+ % else:
+
+
+
+ ${_("Your course currently does not have a video thumbnail image. Please upload one (JPEG or PNG format, and minimum suggested dimensions are 375px wide by 200px tall)")}
+ % endif
+
+
+
+
+
+ ## Translators: This is the placeholder text for a field that requests the URL for a course video thumbnail image
+
+ ${_("Please provide a valid path and name to your video thumbnail image (Note: only JPEG or PNG format supported)")}
+
+
+
@@ -362,10 +448,44 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url}';
${_("Enter your YouTube video's ID (along with any restriction parameters)")}
- % endif
+ % endif
+ % if enable_extended_course_details:
+ +
${_("Learning Outcomes")}
+ ${_("Add the learning outcomes for this course")} +-
+
+
+
+
+ +
${_("Instructors")}
+ ${_("Add details about the instructors for this course")} +-
+
+
+
+
+ diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 93e1481417..85188e7ddb 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -475,6 +475,26 @@ class CourseFields(object): # Ensure that courses imported from XML keep their image default="images_course_image.jpg" ) + banner_image = String( + display_name=_("Course Banner Image"), + help=_( + "Edit the name of the banner image file. " + "You can set the banner image on the Settings & Details page." + ), + scope=Scope.settings, + # Ensure that courses imported from XML keep their image + default="images_course_image.jpg" + ) + video_thumbnail_image = String( + display_name=_("Course Video Thumbnail Image"), + help=_( + "Edit the name of the video thumbnail image file. " + "You can set the video thumbnail image on the Settings & Details page." + ), + scope=Scope.settings, + # Ensure that courses imported from XML keep their image + default="images_course_image.jpg" + ) issue_badges = Boolean( display_name=_("Issue Open Badges"), help=_( @@ -774,6 +794,36 @@ class CourseFields(object): scope=Scope.settings ) + learning_info = List( + display_name=_("Course Learning Information"), + help=_("Specify what student can learn from the course."), + default=[], + scope=Scope.settings + ) + + """ + instructor_info dict structure: + { + "instructors": [ + { + "name": "", + "title": "", + "organization": "", + "image": "", + "bio": "" + } + ] + } + """ + instructor_info = Dict( + display_name=_("Course Instructor"), + help=_("Enter the details for Course Instructor"), + default={ + "instructors": [] + }, + scope=Scope.settings + ) + class CourseModule(CourseFields, SequenceModule): # pylint: disable=abstract-method """ diff --git a/common/test/acceptance/pages/studio/settings.py b/common/test/acceptance/pages/studio/settings.py index 2adfb161ca..9546de0c2b 100644 --- a/common/test/acceptance/pages/studio/settings.py +++ b/common/test/acceptance/pages/studio/settings.py @@ -3,6 +3,7 @@ Course Schedule and Details Settings page. """ from __future__ import unicode_literals +import os from bok_choy.promise import EmptyPromise from bok_choy.javascript import requirejs @@ -17,6 +18,9 @@ class SettingsPage(CoursePage): """ url_path = "settings/details" + upload_image_browse_button_selector = 'form.upload-dialog input[type=file]' + upload_image_upload_button_selector = '.modal-actions li:nth-child(1) a' + upload_image_popup_window_selector = '.assetupload-modal' ################ # Helpers @@ -234,3 +238,53 @@ class SettingsPage(CoursePage): ).fulfill() self.wait_for_require_js() self.wait_for_ajax() + + @staticmethod + def get_asset_path(file_name): + """ + Returns the full path of the file to upload. + These files have been placed in edx-platform/common/test/data/uploads/ + """ + + # Separate the list of folders in the path reaching to the current file, + # e.g. '... common/test/acceptance/pages/lms/instructor_dashboard.py' will result in + # [..., 'common', 'test', 'acceptance', 'pages', 'lms', 'instructor_dashboard.py'] + folders_list_in_path = __file__.split(os.sep) + + # Get rid of the last 4 elements: 'acceptance', 'pages', 'lms', and 'instructor_dashboard.py' + # to point to the 'test' folder, a shared point in the path's tree. + folders_list_in_path = folders_list_in_path[:-4] + + # Append the folders in the asset's path + folders_list_in_path.extend(['data', 'uploads', file_name]) + + # Return the joined path of the required asset. + return os.sep.join(folders_list_in_path) + + def upload_image(self, upload_btn_selector, file_to_upload): + """ + Upload image specified by image_selector and file_to_upload + """ + + # wait for upload button + self.wait_for_element_presence(upload_btn_selector, 'upload button is present') + + self.q(css=upload_btn_selector).results[0].click() + + # wait for popup + self.wait_for_element_presence(self.upload_image_popup_window_selector, 'upload dialog is present') + + # upload image + filepath = SettingsPage.get_asset_path(file_to_upload) + self.q(css=self.upload_image_browse_button_selector).results[0].send_keys(filepath) + self.q(css=self.upload_image_upload_button_selector).results[0].click() + + # wait for popup closed + self.wait_for_element_absence(self.upload_image_popup_window_selector, 'upload dialog is hidden') + + def get_uploaded_image_path(self, image_selector): + """ + Returns the uploaded image path + """ + + return self.q(css=image_selector).attrs('src')[0] diff --git a/common/test/acceptance/pages/studio/settings_advanced.py b/common/test/acceptance/pages/studio/settings_advanced.py index e209207189..87acaf1785 100644 --- a/common/test/acceptance/pages/studio/settings_advanced.py +++ b/common/test/acceptance/pages/studio/settings_advanced.py @@ -172,6 +172,8 @@ class AdvancedSettingsPage(CoursePage): 'cert_name_short', 'certificates_display_behavior', 'course_image', + 'banner_image', + 'video_thumbnail_image', 'cosmetic_display_price', 'advertised_start', 'announcement', @@ -217,4 +219,6 @@ class AdvancedSettingsPage(CoursePage): 'enable_proctored_exams', 'enable_timed_exams', 'enable_subsection_gating', + 'learning_info', + 'instructor_info' ] diff --git a/common/test/acceptance/tests/studio/test_studio_settings.py b/common/test/acceptance/tests/studio/test_studio_settings.py index d050493600..a783c0d81f 100644 --- a/common/test/acceptance/tests/studio/test_studio_settings.py +++ b/common/test/acceptance/tests/studio/test_studio_settings.py @@ -475,3 +475,36 @@ class ContentLicenseTest(StudioCourseTest): # 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) + + +@attr('a11y') +class StudioSettingsA11yTest(StudioCourseTest): + + """ + Class to test Studio pages accessibility. + """ + + def setUp(self): # pylint: disable=arguments-differ + super(StudioSettingsA11yTest, self).setUp() + self.settings_page = SettingsPage(self.browser, self.course_info['org'], self.course_info['number'], + self.course_info['run']) + + def test_studio_settings_page_a11y(self): + """ + Check accessibility of SettingsPage. + """ + self.settings_page.visit() + self.settings_page.wait_for_page() + + # There are several existing color contrast errors on this page, + # we will ignore this error in the test until we fix them. + self.settings_page.a11y_audit.config.set_rules({ + "ignore": [ + 'color-contrast', # TODO: AC-225 + 'link-href', # TODO: AC-226 + 'nav-aria-label', # TODO: AC-227 + 'icon-aria-hidden', # TODO: AC-229 + ], + }) + + self.settings_page.a11y_audit.check_for_accessibility_errors() diff --git a/openedx/core/djangoapps/models/course_details.py b/openedx/core/djangoapps/models/course_details.py index cdf0c76340..0db50a3803 100644 --- a/openedx/core/djangoapps/models/course_details.py +++ b/openedx/core/djangoapps/models/course_details.py @@ -18,6 +18,10 @@ from xmodule.modulestore.django import modulestore # handled separately; its value maps to an alternate key name. ABOUT_ATTRIBUTES = [ 'syllabus', + 'title', + 'subtitle', + 'duration', + 'description', 'short_description', 'overview', 'effort', @@ -43,6 +47,10 @@ class CourseDetails(object): self.enrollment_start = None self.enrollment_end = None self.syllabus = None # a pdf file asset + self.title = "" + self.subtitle = "" + self.duration = "" + self.description = "" self.short_description = "" self.overview = "" # html to render as the overview self.intro_video = None # a video pointer @@ -50,6 +58,10 @@ class CourseDetails(object): 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.banner_image_name = "" + self.banner_image_asset_path = "" + self.video_thumbnail_image_name = "" + self.video_thumbnail_image_asset_path = "" self.pre_requisite_courses = [] # pre-requisite courses self.entrance_exam_enabled = "" # is entrance exam enabled self.entrance_exam_id = "" # the content location for the entrance exam @@ -58,6 +70,8 @@ class CourseDetails(object): '50' ) # minimum passing score for entrance exam content module/tree, self.self_paced = None + self.learning_info = [] + self.instructor_info = [] @classmethod def fetch_about_attribute(cls, course_key, attribute): @@ -89,9 +103,15 @@ class CourseDetails(object): course_details.enrollment_end = descriptor.enrollment_end 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.course_image_asset_path = course_image_url(descriptor, 'course_image') + course_details.banner_image_name = descriptor.banner_image + course_details.banner_image_asset_path = course_image_url(descriptor, 'banner_image') + course_details.video_thumbnail_image_name = descriptor.video_thumbnail_image + course_details.video_thumbnail_image_asset_path = course_image_url(descriptor, 'video_thumbnail_image') course_details.language = descriptor.language course_details.self_paced = descriptor.self_paced + course_details.learning_info = descriptor.learning_info + course_details.instructor_info = descriptor.instructor_info # Default course license is "All Rights Reserved" course_details.license = getattr(descriptor, "license", "all-rights-reserved") @@ -209,6 +229,15 @@ class CourseDetails(object): descriptor.course_image = jsondict['course_image_name'] dirty = True + if 'banner_image_name' in jsondict and jsondict['banner_image_name'] != descriptor.banner_image: + descriptor.banner_image = jsondict['banner_image_name'] + dirty = True + + if 'video_thumbnail_image_name' in jsondict \ + and jsondict['video_thumbnail_image_name'] != descriptor.video_thumbnail_image: + descriptor.video_thumbnail_image = jsondict['video_thumbnail_image_name'] + dirty = True + if 'pre_requisite_courses' in jsondict \ and sorted(jsondict['pre_requisite_courses']) != sorted(descriptor.pre_requisite_courses): descriptor.pre_requisite_courses = jsondict['pre_requisite_courses'] @@ -218,6 +247,14 @@ class CourseDetails(object): descriptor.license = jsondict['license'] dirty = True + if 'learning_info' in jsondict: + descriptor.learning_info = jsondict['learning_info'] + dirty = True + + if 'instructor_info' in jsondict: + descriptor.instructor_info = jsondict['instructor_info'] + dirty = True + if 'language' in jsondict and jsondict['language'] != descriptor.language: descriptor.language = jsondict['language'] dirty = True diff --git a/openedx/core/djangoapps/models/tests/test_course_details.py b/openedx/core/djangoapps/models/tests/test_course_details.py index 46f39982cf..661ddfe456 100644 --- a/openedx/core/djangoapps/models/tests/test_course_details.py +++ b/openedx/core/djangoapps/models/tests/test_course_details.py @@ -95,11 +95,41 @@ class CourseDetailsTestCase(ModuleStoreTestCase): CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).course_image_name, jsondetails.course_image_name ) + jsondetails.banner_image_name = "an_image.jpg" + self.assertEqual( + CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).banner_image_name, + jsondetails.banner_image_name + ) + jsondetails.video_thumbnail_image_name = "an_image.jpg" + self.assertEqual( + CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).video_thumbnail_image_name, + jsondetails.video_thumbnail_image_name + ) jsondetails.language = "hr" self.assertEqual( CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).language, jsondetails.language ) + jsondetails.learning_info = ["test", "test"] + self.assertEqual( + CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).learning_info, + jsondetails.learning_info + ) + jsondetails.instructor_info = { + "instructors": [ + { + "name": "test", + "title": "test", + "organization": "test", + "image": "test", + "bio": "test" + } + ] + } + self.assertEqual( + CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).instructor_info, + jsondetails.instructor_info + ) def test_toggle_pacing_during_course_run(self): SelfPacedConfiguration(enabled=True).save() diff --git a/openedx/core/lib/courses.py b/openedx/core/lib/courses.py index 2936cc93d8..a62cdc4e73 100644 --- a/openedx/core/lib/courses.py +++ b/openedx/core/lib/courses.py @@ -10,24 +10,25 @@ from xmodule.modulestore.django import modulestore from xmodule.modulestore import ModuleStoreEnum -def course_image_url(course): +def course_image_url(course, image_key='course_image'): """Try to look up the image url for the course. If it's not found, - log an error and return the dead link""" + log an error and return the dead link. + image_key can be one of the three: 'course_image', 'hero_image', 'thumbnail_image' """ if course.static_asset_path or modulestore().get_modulestore_type(course.id) == ModuleStoreEnum.Type.xml: - # If we are a static course with the course_image attribute + # If we are a static course with the image_key attribute # set different than the default, return that path so that # courses can use custom course image paths, otherwise just # return the default static path. url = '/static/' + (course.static_asset_path or getattr(course, 'data_dir', '')) - if hasattr(course, 'course_image') and course.course_image != course.fields['course_image'].default: - url += '/' + course.course_image + if hasattr(course, image_key) and getattr(course, image_key) != course.fields[image_key].default: + url += '/' + getattr(course, image_key) else: - url += '/images/course_image.jpg' - elif not course.course_image: - # if course_image is empty, use the default image url from settings + url += '/images/' + image_key + '.jpg' + elif not getattr(course, image_key): + # if image_key is empty, use the default image url from settings url = settings.STATIC_URL + settings.DEFAULT_COURSE_ABOUT_IMAGE_URL else: - loc = StaticContent.compute_location(course.id, course.course_image) + loc = StaticContent.compute_location(course.id, getattr(course, image_key)) url = StaticContent.serialize_asset_key_with_slash(loc) return url diff --git a/openedx/core/lib/tests/test_courses.py b/openedx/core/lib/tests/test_courses.py index 696f9e503b..45a8b47cdb 100644 --- a/openedx/core/lib/tests/test_courses.py +++ b/openedx/core/lib/tests/test_courses.py @@ -65,3 +65,21 @@ class CourseImageTestCase(ModuleStoreTestCase): 'static/test.png', course_image_url(course), ) + + def test_get_banner_image_url(self): + """Test banner image URL formatting.""" + banner_image = u'banner_image.jpg' + course = CourseFactory.create(banner_image=banner_image) + self.verify_url( + unicode(course.id.make_asset_key('asset', banner_image)), + course_image_url(course, 'banner_image') + ) + + def test_get_video_thumbnail_image_url(self): + """Test video thumbnail image URL formatting.""" + thumbnail_image = u'thumbnail_image.jpg' + course = CourseFactory.create(video_thumbnail_image=thumbnail_image) + self.verify_url( + unicode(course.id.make_asset_key('asset', thumbnail_image)), + course_image_url(course, 'video_thumbnail_image') + )