diff --git a/cms/djangoapps/contentstore/features/course-settings.feature b/cms/djangoapps/contentstore/features/course-settings.feature index 5c79dc7ee3..69183bc3da 100644 --- a/cms/djangoapps/contentstore/features/course-settings.feature +++ b/cms/djangoapps/contentstore/features/course-settings.feature @@ -57,6 +57,7 @@ Feature: Course Settings | Course Start Time | 11:00 | | Course Introduction Video | 4r7wHMg5Yjg | | Course Effort | 200:00 | + | Course Image URL | image.jpg | # Special case because we have to type in code mirror Scenario: Changes in Course Overview show a confirmation @@ -71,3 +72,10 @@ Feature: Course Settings When I select Schedule and Details And I change the "Course Start Date" field to "" Then the save button is disabled + + Scenario: User can upload course image + Given I have opened a new course in Studio + When I select Schedule and Details + And I upload a new course image + Then I should see the new course image + And the image URL should be present in the field diff --git a/cms/djangoapps/contentstore/features/course-settings.py b/cms/djangoapps/contentstore/features/course-settings.py index da72d893cf..0847c62a18 100644 --- a/cms/djangoapps/contentstore/features/course-settings.py +++ b/cms/djangoapps/contentstore/features/course-settings.py @@ -5,9 +5,13 @@ from lettuce import world, step from terrain.steps import reload_the_page from selenium.webdriver.common.keys import Keys from common import type_in_codemirror +from django.conf import settings +import os from nose.tools import assert_true, assert_false, assert_equal +TEST_ROOT = settings.COMMON_TEST_DATA_ROOT + COURSE_START_DATE_CSS = "#course-start-date" COURSE_END_DATE_CSS = "#course-end-date" ENROLLMENT_START_DATE_CSS = "#course-enrollment-start-date" @@ -146,6 +150,36 @@ def test_change_course_overview(_step): type_in_codemirror(0, "

Overview

") +@step('I upload a new course image$') +def upload_new_course_image(_step): + upload_css = '.action-upload-image' + world.css_click(upload_css) + file_css = '.upload-dialog input[type=file]' + upload = world.css_find(file_css) + path = os.path.join(TEST_ROOT, 'image.jpg') + upload._element.send_keys(os.path.abspath(path)) + button_css = '.upload-dialog .action-upload' + world.css_click(button_css) + + +@step('I should see the new course image$') +def i_see_new_course_image(_step): + img_css = '#course-image' + images = world.css_find(img_css) + assert len(images) == 1 + img = images[0] + expected_src = '/c4x/MITx/999/asset/image.jpg' + # Don't worry about the domain in the URL + assert img['src'].endswith(expected_src) + + +@step('the image URL should be present in the field') +def image_url_present(_step): + field_css = '#course-image-url' + field = world.css_find(field_css).first + expected_value = '/c4x/MITx/999/asset/image.jpg' + assert field.value == expected_value + ############### HELPER METHODS #################### def set_date_or_time(css, date_or_time): diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 96b0b84e36..216edc6b88 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1608,6 +1608,29 @@ class ContentStoreTest(ModuleStoreTestCase): # is this test too strict? i.e., it requires the dicts to be == self.assertEqual(course.checklists, fetched_course.checklists) + def test_image_import(self): + """Test backwards compatibilty of course image.""" + module_store = modulestore('direct') + + content_store = contentstore() + + # Use conditional_and_poll, as it's got an image already + import_from_xml( + module_store, + 'common/test/data/', + ['conditional_and_poll'], + static_content_store=content_store + ) + + course = module_store.get_courses()[0] + + # Make sure the course image is set to the right place + self.assertEqual(course.course_image, 'images_course_image.jpg') + + # Ensure that the imported course image is present -- this shouldn't raise an exception + location = course.location._replace(tag='c4x', category='asset', name=course.course_image) + content_store.find(location) + class MetadataSaveTestCase(ModuleStoreTestCase): """Test that metadata is correctly cached and decached.""" diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index 2007ba2f69..dbdf8b3f6e 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -30,6 +30,7 @@ class CourseDetailsTestCase(CourseTestCase): def test_virgin_fetch(self): details = CourseDetails.fetch(self.course.location) self.assertEqual(details.course_location, self.course.location, "Location not copied into") + self.assertEqual(details.course_image_name, self.course.course_image) self.assertIsNotNone(details.start_date.tzinfo) self.assertIsNone(details.end_date, "end date somehow initialized " + str(details.end_date)) self.assertIsNone(details.enrollment_start, "enrollment_start date somehow initialized " + str(details.enrollment_start)) @@ -43,6 +44,7 @@ class CourseDetailsTestCase(CourseTestCase): jsondetails = json.dumps(details, cls=CourseSettingsEncoder) jsondetails = json.loads(jsondetails) self.assertTupleEqual(Location(jsondetails['course_location']), self.course.location, "Location !=") + self.assertEqual(jsondetails['course_image_name'], self.course.course_image) self.assertIsNone(jsondetails['end_date'], "end date somehow initialized ") self.assertIsNone(jsondetails['enrollment_start'], "enrollment_start date somehow initialized ") self.assertIsNone(jsondetails['enrollment_end'], "enrollment_end date somehow initialized ") @@ -97,6 +99,11 @@ class CourseDetailsTestCase(CourseTestCase): CourseDetails.update_from_json(jsondetails.__dict__).start_date, jsondetails.start_date ) + jsondetails.course_image_name = "an_image.jpg" + self.assertEqual( + CourseDetails.update_from_json(jsondetails.__dict__).course_image_name, + jsondetails.course_image_name + ) @override_settings(MKTG_URLS={'ROOT': 'dummy-root'}) def test_marketing_site_fetch(self): @@ -188,6 +195,7 @@ class CourseDetailsViewTest(CourseTestCase): self.alter_field(url, details, 'overview', "Overview") self.alter_field(url, details, 'intro_video', "intro_video") self.alter_field(url, details, 'effort', "effort") + self.alter_field(url, details, 'course_image_name', "course_image_name") def compare_details_with_encoding(self, encoded, details, context): self.compare_date_fields(details, encoded, context, 'start_date') @@ -197,6 +205,7 @@ class CourseDetailsViewTest(CourseTestCase): self.assertEqual(details['overview'], encoded['overview'], context + " overviews not ==") self.assertEqual(details['intro_video'], encoded.get('intro_video', None), context + " intro_video not ==") self.assertEqual(details['effort'], encoded['effort'], context + " efforts not ==") + self.assertEqual(details['course_image_name'], encoded['course_image_name'], context + " images not ==") def compare_date_fields(self, details, encoded, context, field): if details[field] is not None: diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index c3335aaaa0..3d6d1d0c56 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -5,6 +5,7 @@ import collections import copy from django.test import TestCase from django.test.utils import override_settings +from xmodule.modulestore.tests.factories import CourseFactory class LMSLinksTestCase(TestCase): @@ -150,3 +151,13 @@ class ExtraPanelTabTestCase(TestCase): changed, actual_tabs = utils.remove_extra_panel_tab(tab_type, course) self.assertFalse(changed) self.assertEqual(actual_tabs, expected_tabs) + + +class CourseImageTestCase(TestCase): + """Tests for course image URLs.""" + + def test_get_image_url(self): + """Test image URL formatting.""" + course = CourseFactory.create(org='edX', course='999') + url = utils.course_image_url(course) + self.assertEquals(url, '/c4x/edX/999/asset/{0}'.format(course.course_image)) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index d956a903b6..e5ae6bb66b 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -4,6 +4,7 @@ from django.conf import settings from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.contentstore.content import StaticContent from django.core.urlresolvers import reverse import copy import logging @@ -153,6 +154,13 @@ def get_lms_link_for_about_page(location): return lms_link +def course_image_url(course): + """Returns the image url for the course.""" + loc = course.location._replace(tag='c4x', category='asset', name=course.course_image) + path = StaticContent.get_url_path_from_location(loc) + return path + + class UnitState(object): draft = 'draft' private = 'private' diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 753df66fe0..aad56e4a2e 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -276,7 +276,12 @@ def get_course_settings(request, org, course, name): "section": "details"}), 'about_page_editable': not settings.MITX_FEATURES.get( 'ENABLE_MKTG_SITE', False - ) + ), + 'upload_asset_url': reverse('upload_asset', kwargs={ + 'org': org, + 'course': course, + 'coursename': name, + }) }) diff --git a/cms/djangoapps/models/settings/course_details.py b/cms/djangoapps/models/settings/course_details.py index 78c5dcff33..99ce00b891 100644 --- a/cms/djangoapps/models/settings/course_details.py +++ b/cms/djangoapps/models/settings/course_details.py @@ -3,7 +3,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.inheritance import own_metadata import json from json.encoder import JSONEncoder -from contentstore.utils import get_modulestore +from contentstore.utils import get_modulestore, course_image_url from models.settings import course_grading from contentstore.utils import update_item from xmodule.fields import Date @@ -23,6 +23,8 @@ 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.course_image_name = "" + self.course_image_asset_path = "" # URL of the course image @classmethod def fetch(cls, course_location): @@ -40,6 +42,8 @@ class CourseDetails(object): course.end_date = descriptor.end course.enrollment_start = descriptor.enrollment_start course.enrollment_end = descriptor.enrollment_end + course.course_image_name = descriptor.course_image + course.course_image_asset_path = course_image_url(descriptor) temploc = course_location.replace(category='about', name='syllabus') try: @@ -121,6 +125,10 @@ class CourseDetails(object): dirty = True descriptor.enrollment_end = converted + if 'course_image_name' in jsondict and jsondict['course_image_name'] != descriptor.course_image: + descriptor.course_image = jsondict['course_image_name'] + dirty = True + if dirty: # Save the data that we've just changed to the underlying # MongoKeyValueStore before we update the mongo datastore. diff --git a/cms/static/js/models/settings/course_details.js b/cms/static/js/models/settings/course_details.js index 4d048bab81..b66f2bbba9 100644 --- a/cms/static/js/models/settings/course_details.js +++ b/cms/static/js/models/settings/course_details.js @@ -10,7 +10,9 @@ CMS.Models.Settings.CourseDetails = Backbone.Model.extend({ syllabus: null, overview: "", intro_video: null, - effort: null // an int or null + effort: null, // an int or null, + course_image_name: '', // the filename + course_image_asset_path: '' // the full URL (/c4x/org/course/num/asset/filename) }, // When init'g from html script, ensure you pass {parse: true} as an option (2nd arg to reset) diff --git a/cms/static/js/views/settings/main_settings_view.js b/cms/static/js/views/settings/main_settings_view.js index 0cbf573ba9..36bee79d80 100644 --- a/cms/static/js/views/settings/main_settings_view.js +++ b/cms/static/js/views/settings/main_settings_view.js @@ -13,8 +13,8 @@ CMS.Views.Settings.Details = CMS.Views.ValidatingView.extend({ 'mouseover #timezone' : "updateTime", // would love to move to a general superclass, but event hashes don't inherit in backbone :-( 'focus :input' : "inputFocus", - 'blur :input' : "inputUnfocus" - + 'blur :input' : "inputUnfocus", + 'click .action-upload-image': "uploadImage" }, initialize : function() { @@ -51,6 +51,10 @@ CMS.Views.Settings.Details = CMS.Views.ValidatingView.extend({ this.$el.find('#' + this.fieldToSelectorMap['effort']).val(this.model.get('effort')); + var imageURL = this.model.get('course_image_asset_path'); + this.$el.find('#course-image-url').val(imageURL) + this.$el.find('#course-image').attr('src', imageURL); + return this; }, fieldToSelectorMap : { @@ -60,7 +64,8 @@ CMS.Views.Settings.Details = CMS.Views.ValidatingView.extend({ 'enrollment_end' : 'enrollment-end', 'overview' : 'course-overview', 'intro_video' : 'course-introduction-video', - 'effort' : "course-effort" + 'effort' : "course-effort", + 'course_image_asset_path': 'course-image-url' }, updateTime : function(e) { @@ -121,6 +126,17 @@ CMS.Views.Settings.Details = CMS.Views.ValidatingView.extend({ updateModel: function(event) { switch (event.currentTarget.id) { + case 'course-image-url': + this.setField(event); + var url = $(event.currentTarget).val(); + var image_name = _.last(url.split('/')); + this.model.set('course_image_name', image_name); + // Wait to set the image src until the user stops typing + clearTimeout(this.imageTimer); + this.imageTimer = setTimeout(function() { + $('#course-image').attr('src', $(event.currentTarget).val()); + }, 1000); + break; case 'course-effort': this.setField(event); break; @@ -216,6 +232,30 @@ CMS.Views.Settings.Details = CMS.Views.ValidatingView.extend({ this.save_message, _.bind(this.saveView, this), _.bind(this.revertView, this)); + }, + + uploadImage: function(event) { + event.preventDefault(); + var upload = new CMS.Models.FileUpload({ + title: gettext("Upload your course image."), + message: gettext("Files must be in JPG format."), + mimeType: "image/jpeg", + fileType: "JPG" + }); + var self = this; + var modal = new CMS.Views.UploadDialog({ + model: upload, + onSuccess: function(response) { + var options = { + 'course_image_name': response.displayname, + 'course_image_asset_path': response.url + } + self.model.set(options); + self.render(); + $('#course-image').attr('src', self.model.get('course_image_asset_path')) + } + }); + $('.wrapper-view').after(modal.show().el); } }); diff --git a/cms/static/sass/views/_settings.scss b/cms/static/sass/views/_settings.scss index 1430c41368..ca48244f64 100644 --- a/cms/static/sass/views/_settings.scss +++ b/cms/static/sass/views/_settings.scss @@ -432,6 +432,20 @@ body.course.settings { } } + // specific fields - course image + #field-course-image { + .current-course-image { + position: relative; + + .action-upload-image { + @extend .ui-btn-flat-outline; + position: absolute; + bottom: 3px; + right: 0; + } + } + } + // specific fields - requirements &.requirements { diff --git a/cms/templates/settings.html b/cms/templates/settings.html index 1f5d89b2b9..96a8e59d9d 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -2,7 +2,7 @@ <%inherit file="base.html" /> <%block name="title">${_("Schedule & Details Settings")} -<%block name="bodyclass">is-signedin course schedule settings +<%block name="bodyclass">is-signedin course schedule settings file-upload-dialog <%namespace name='static' file='static_content.html'/> <%! @@ -22,6 +22,10 @@ from contentstore import utils + + @@ -208,6 +214,21 @@ from contentstore import utils ${overview_text()} +
  • + +
    + % if context_course.course_image: + ${_('Course Image')} + % endif + +
    + +
    + + ${_("Enter your course image's filename.")} +
    +
  • +
  • diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 57b13c10b3..4555395fef 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -338,6 +338,12 @@ class CourseFields(object): show_timezone = Boolean(help="True if timezones should be shown on dates in the courseware", scope=Scope.settings, default=True) enrollment_domain = String(help="External login method associated with user accounts allowed to register in course", scope=Scope.settings) + course_image = String( + help="Filename of the course image", + scope=Scope.settings, + # Ensure that courses imported from XML keep their image + default="images_course_image.jpg" + ) # An extra property is used rather than the wiki_slug/number because # there are courses that change the number for different runs. This allows diff --git a/common/test/data/uploads/image.jpg b/common/test/data/uploads/image.jpg new file mode 100644 index 0000000000..21ece4ef43 Binary files /dev/null and b/common/test/data/uploads/image.jpg differ diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 5f31658cf2..78cf5a1585 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -84,7 +84,7 @@ def course_image_url(course): if modulestore().get_modulestore_type(course.location.course_id) == XML_MODULESTORE_TYPE: return '/static/' + course.data_dir + "/images/course_image.jpg" else: - loc = course.location._replace(tag='c4x', category='asset', name='images_course_image.jpg') + loc = course.location._replace(tag='c4x', category='asset', name=course.course_image) _path = StaticContent.get_url_path_from_location(loc) return _path