Merge pull request #700 from edx/peter-fogg/course-image-upload

WIP: Peter fogg/course image upload

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

cms/djangoapps/contentstore/features/common.py

@@ -5,16 +5,20 @@
 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
 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)

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

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, "<h1>Overview</h1>")
 
 
+@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):

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')

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -1625,6 +1625,29 @@ def test_default_metadata_inheritance(self):
         # 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."""

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 @@ def test_encoder(self):
         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 @@ def test_update_and_fetch(self):
             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 @@ def test_update_and_fetch(self):
         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 @@ def compare_details_with_encoding(self, encoded, details, context):
         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:

cms/djangoapps/contentstore/tests/test_utils.py

@@ -5,6 +5,7 @@
 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 @@ def test_remove_extra_panel_tab(self):
                 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))

cms/djangoapps/contentstore/utils.py

@@ -4,6 +4,7 @@
 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'

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,
+        })
     })
 
 

cms/djangoapps/models/settings/course_details.py

@@ -3,7 +3,7 @@
 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 @@ def __init__(self, location):
         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 @@ def fetch(cls, course_location):
         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 @@ def update_from_json(cls, jsondict):
             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.

cms/envs/common.py

@@ -244,6 +244,7 @@
              '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',

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()