Allow course image uploads in the settings page.
Authors can upload an image (or choose an existing one) from the settings page, using the in-context uploader from PDF textbooks. Includes tests for backwards compatibility with XML courses -- they used a magic filename (images/course_image.jpg) which is mapped to a location in the Mongo contentstore. Still needs some UX work, though the backend plumbing is there.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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, "<h1>Overview</h1>")
|
||||
|
||||
|
||||
@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):
|
||||
|
||||
@@ -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."""
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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))
|
||||
|
||||
@@ -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'
|
||||
|
||||
@@ -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,
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
<%inherit file="base.html" />
|
||||
<%block name="title">${_("Schedule & Details Settings")}</%block>
|
||||
<%block name="bodyclass">is-signedin course schedule settings</%block>
|
||||
<%block name="bodyclass">is-signedin course schedule settings file-upload-dialog</%block>
|
||||
|
||||
<%namespace name='static' file='static_content.html'/>
|
||||
<%!
|
||||
@@ -22,6 +22,10 @@ from contentstore import utils
|
||||
<script type="text/javascript" src="${static.url('js/views/settings/main_settings_view.js')}"></script>
|
||||
<script type="text/javascript" src="${static.url('js/models/settings/course_details.js')}"></script>
|
||||
|
||||
<script type="text/template" id="upload-dialog-tpl">
|
||||
<%static:include path="js/upload-dialog.underscore" />
|
||||
</script>
|
||||
|
||||
<script type="text/javascript">
|
||||
$(document).ready(function(){
|
||||
|
||||
@@ -43,6 +47,8 @@ from contentstore import utils
|
||||
},
|
||||
reset: true
|
||||
});
|
||||
|
||||
CMS.URL.UPLOAD_ASSET = '${upload_asset_url}';
|
||||
});
|
||||
|
||||
</script>
|
||||
@@ -208,6 +214,21 @@ from contentstore import utils
|
||||
<span class="tip tip-stacked">${overview_text()}</span>
|
||||
</li>
|
||||
|
||||
<li class="field image" id="field-course-image">
|
||||
<label>${_("Course Image")}</label>
|
||||
<div class="current current-course-image">
|
||||
% if context_course.course_image:
|
||||
<img id="course-image" width="200" src="${utils.course_image_url(context_course)}" alt="${_('Course Image')}"/>
|
||||
% endif
|
||||
<button type="button" class="action action-upload-image">${_("Upload Course Image")}</button>
|
||||
</div>
|
||||
|
||||
<div class="input">
|
||||
<input type="text" class="long new-course-image-url" id="course-image-url" value="" placeholder="Your course image URL" autocomplete="off" />
|
||||
<span class="tip tip-stacked">${_("Enter your course image's filename.")}</span>
|
||||
</div>
|
||||
</li>
|
||||
|
||||
<li class="field video" id="field-course-introduction-video">
|
||||
<label for="course-overview">${_("Course Introduction Video")}</label>
|
||||
<div class="input input-existing">
|
||||
|
||||
@@ -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
|
||||
|
||||
BIN
common/test/data/uploads/image.jpg
Normal file
BIN
common/test/data/uploads/image.jpg
Normal file
Binary file not shown.
|
After Width: | Height: | Size: 14 KiB |
@@ -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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user