diff --git a/CHANGELOG.rst b/CHANGELOG.rst index cd43777e96..749b9ef56e 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,9 @@ These are notable changes in edx-platform. This is a rolling list of changes, in roughly chronological order, most recent first. Add your entries at or near the top. Include a label indicating the component affected. +Studio: Allow course authors to set their course image on the schedule +and details page, with support for JPEG and PNG images. + Blades: Took videoalpha out of alpha, replacing the old video player Common: Allow instructors to input complicated expressions as answers to diff --git a/cms/djangoapps/contentstore/features/common.py b/cms/djangoapps/contentstore/features/common.py index 516659fadb..1cc71097a0 100644 --- a/cms/djangoapps/contentstore/features/common.py +++ b/cms/djangoapps/contentstore/features/common.py @@ -5,9 +5,11 @@ from lettuce import world, step from nose.tools import assert_true from auth.authz import get_user_by_email, get_course_groupname_for_role +from django.conf import settings from selenium.webdriver.common.keys import Keys import time +import os from django.contrib.auth.models import Group from logging import getLogger @@ -15,6 +17,8 @@ logger = getLogger(__name__) from terrain.browser import reset_data +TEST_ROOT = settings.COMMON_TEST_DATA_ROOT + ########### STEP HELPERS ############## @@ -257,3 +261,12 @@ def type_in_codemirror(index, text): g._element.send_keys(text) if world.is_firefox(): world.trigger_event('div.CodeMirror', index=index, event='blur') + + +def upload_file(filename): + file_css = '.upload-dialog input[type=file]' + upload = world.css_find(file_css).first + path = os.path.join(TEST_ROOT, filename) + upload._element.send_keys(os.path.abspath(path)) + button_css = '.upload-dialog .action-upload' + world.css_click(button_css) diff --git a/cms/djangoapps/contentstore/features/course-settings.feature b/cms/djangoapps/contentstore/features/course-settings.feature index 5c79dc7ee3..8f00452efe 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,11 @@ 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 click the "Upload Course Image" button + 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..570c49a8c4 100644 --- a/cms/djangoapps/contentstore/features/course-settings.py +++ b/cms/djangoapps/contentstore/features/course-settings.py @@ -4,10 +4,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 common import type_in_codemirror, upload_file +from django.conf import settings 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 +149,35 @@ def test_change_course_overview(_step): type_in_codemirror(0, "

Overview

") +@step('I click the "Upload Course Image" button') +def click_upload_button(_step): + button_css = '.action-upload-image' + world.css_click(button_css) + + +@step('I upload a new course image$') +def upload_new_course_image(_step): + upload_file('image.jpg') + + +@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/features/textbooks.py b/cms/djangoapps/contentstore/features/textbooks.py index d9c08ec6eb..b432b84d4f 100644 --- a/cms/djangoapps/contentstore/features/textbooks.py +++ b/cms/djangoapps/contentstore/features/textbooks.py @@ -3,7 +3,7 @@ from lettuce import world, step from django.conf import settings -import os +from common import upload_file TEST_ROOT = settings.COMMON_TEST_DATA_ROOT @@ -24,14 +24,8 @@ def assert_create_new_textbook_msg(_step): @step(u'I upload the textbook "([^"]*)"$') -def upload_file(_step, file_name): - file_css = '.upload-dialog input[type=file]' - upload = world.css_find(file_css) - # uploading the file itself - path = os.path.join(TEST_ROOT, 'uploads', file_name) - upload._element.send_keys(os.path.abspath(path)) - button_css = ".upload-dialog .action-upload" - world.css_click(button_css) +def upload_textbook(_step, file_name): + upload_file(file_name) @step(u'I click (on )?the New Textbook button') diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 2f9f6600c6..7491e5ab4a 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1625,6 +1625,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/envs/common.py b/cms/envs/common.py index f9f12c6184..b89bdafdc4 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -244,6 +244,7 @@ PIPELINE_JS = { 'js/models/course.js', 'js/models/section.js', 'js/views/section.js', 'js/models/metadata_model.js', 'js/views/metadata_editor_view.js', + 'js/models/uploads.js', 'js/views/uploads.js', 'js/models/textbook.js', 'js/views/textbook.js', 'js/views/assets.js', 'js/utility.js'], 'output_filename': 'js/cms-application.js', diff --git a/cms/static/coffee/spec/models/textbook_spec.coffee b/cms/static/coffee/spec/models/textbook_spec.coffee index 6e601ecf68..d88e09f57a 100644 --- a/cms/static/coffee/spec/models/textbook_spec.coffee +++ b/cms/static/coffee/spec/models/textbook_spec.coffee @@ -196,32 +196,3 @@ describe "CMS.Collections.ChapterSet", -> # try going back one @collection.remove(@collection.last()) expect(@collection.nextOrder()).toEqual(2) - - -describe "CMS.Models.FileUpload", -> - beforeEach -> - @model = new CMS.Models.FileUpload() - - it "is unfinished by default", -> - expect(@model.get("finished")).toBeFalsy() - - it "is not uploading by default", -> - expect(@model.get("uploading")).toBeFalsy() - - it "is valid by default", -> - expect(@model.isValid()).toBeTruthy() - - it "is valid for PDF files", -> - file = {"type": "application/pdf"} - @model.set("selectedFile", file); - expect(@model.isValid()).toBeTruthy() - - it "is invalid for text files", -> - file = {"type": "text/plain"} - @model.set("selectedFile", file); - expect(@model.isValid()).toBeFalsy() - - it "is invalid for PNG files", -> - file = {"type": "image/png"} - @model.set("selectedFile", file); - expect(@model.isValid()).toBeFalsy() diff --git a/cms/static/coffee/spec/models/upload_spec.coffee b/cms/static/coffee/spec/models/upload_spec.coffee new file mode 100644 index 0000000000..610898745b --- /dev/null +++ b/cms/static/coffee/spec/models/upload_spec.coffee @@ -0,0 +1,56 @@ +describe "CMS.Models.FileUpload", -> + beforeEach -> + @model = new CMS.Models.FileUpload() + + it "is unfinished by default", -> + expect(@model.get("finished")).toBeFalsy() + + it "is not uploading by default", -> + expect(@model.get("uploading")).toBeFalsy() + + it "is valid by default", -> + expect(@model.isValid()).toBeTruthy() + + it "is invalid for text files by default", -> + file = {"type": "text/plain"} + @model.set("selectedFile", file); + expect(@model.isValid()).toBeFalsy() + + it "is invalid for PNG files by default", -> + file = {"type": "image/png"} + @model.set("selectedFile", file); + expect(@model.isValid()).toBeFalsy() + + it "can accept a file type when explicitly set", -> + file = {"type": "image/png"} + @model.set("mimeTypes": ["image/png"]) + @model.set("selectedFile", file) + expect(@model.isValid()).toBeTruthy() + + it "can accept multiple file types", -> + file = {"type": "image/gif"} + @model.set("mimeTypes": ["image/png", "image/jpeg", "image/gif"]) + @model.set("selectedFile", file) + expect(@model.isValid()).toBeTruthy() + + describe "fileTypes", -> + it "returns a list of the uploader's file types", -> + @model.set('mimeTypes', ['image/png', 'application/json']) + expect(@model.fileTypes()).toEqual(['PNG', 'JSON']) + + describe "formatValidTypes", -> + it "returns a map of formatted file types and extensions", -> + @model.set('mimeTypes', ['image/png', 'image/jpeg', 'application/json']) + formatted = @model.formatValidTypes() + expect(formatted).toEqual( + fileTypes: 'PNG, JPEG or JSON', + fileExtensions: '.png, .jpeg or .json' + ) + + it "does not format with only one mime type", -> + @model.set('mimeTypes', ['application/pdf']) + formatted = @model.formatValidTypes() + expect(formatted).toEqual( + fileTypes: 'PDF', + fileExtensions: '.pdf' + ) diff --git a/cms/static/coffee/spec/views/textbook_spec.coffee b/cms/static/coffee/spec/views/textbook_spec.coffee index 981659abfa..ade8c4cb6e 100644 --- a/cms/static/coffee/spec/views/textbook_spec.coffee +++ b/cms/static/coffee/spec/views/textbook_spec.coffee @@ -301,7 +301,7 @@ describe "CMS.Views.EditChapter", -> @view.render().$(".action-upload").click() ctorOptions = uploadSpies.constructor.mostRecentCall.args[0] expect(ctorOptions.model.get('title')).toMatch(/abcde/) - expect(ctorOptions.chapter).toBe(@model) + expect(typeof ctorOptions.onSuccess).toBe('function') expect(uploadSpies.show).toHaveBeenCalled() it "saves content when opening upload dialog", -> @@ -311,113 +311,3 @@ describe "CMS.Views.EditChapter", -> @view.$(".action-upload").click() expect(@model.get("name")).toEqual("rainbows") expect(@model.get("asset_path")).toEqual("unicorns") - - -describe "CMS.Views.UploadDialog", -> - tpl = readFixtures("upload-dialog.underscore") - - beforeEach -> - setFixtures($(" + + @@ -208,6 +214,34 @@ from contentstore import utils ${overview_text()} +
  • + +
    + % if context_course.course_image: + + ${_('Course Image')} + + + <% ctx_loc = context_course.location %> + ${_("You can manage this image along with all of your other")} ${_("files & uploads")} + + % else: + + ${_('Course Image')} + + ${_("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 +
    + +
    +
    + + ${_("Please provide a valid path and name to your course image (Note: only JPEG or PNG format supported)")} +
    + +
    +
    • +
  • diff --git a/cms/templates/textbooks.html b/cms/templates/textbooks.html index 28349b5436..d55e271858 100644 --- a/cms/templates/textbooks.html +++ b/cms/templates/textbooks.html @@ -4,7 +4,7 @@ <%! from django.utils.translation import ugettext as _ %> <%block name="title">${_("Textbooks")} -<%block name="bodyclass">is-signedin course textbooks +<%block name="bodyclass">is-signedin course textbooks feature-upload <%block name="header_extras"> % for template_name in ["edit-textbook", "show-textbook", "edit-chapter", "no-textbooks", "upload-dialog"]: 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