Refactor course_image_url

cms/djangoapps/contentstore/courseware_index.py

@@ -10,10 +10,10 @@ from django.conf import settings
 from django.utils.translation import ugettext_lazy, ugettext as _
 from django.core.urlresolvers import resolve
 
-from contentstore.utils import course_image_url
 from contentstore.course_group_config import GroupConfiguration
 from course_modes.models import CourseMode
 from eventtracking import tracker
+from openedx.core.lib.courses import course_image_url
 from search.search_engine_base import SearchEngine
 from xmodule.annotator_mixin import html_to_text
 from xmodule.modulestore import ModuleStoreEnum

cms/djangoapps/contentstore/tests/test_utils.py

@@ -110,55 +110,6 @@ class ExtraPanelTabTestCase(TestCase):
         return course
 
 
-@ddt.ddt
-class CourseImageTestCase(ModuleStoreTestCase):
-    """Tests for course image URLs."""
-
-    def verify_url(self, expected_url, actual_url):
-        """
-        Helper method for verifying the URL is as expected.
-        """
-        if not expected_url.startswith("/"):
-            expected_url = "/" + expected_url
-        self.assertEquals(expected_url, actual_url)
-
-    def test_get_image_url(self):
-        """Test image URL formatting."""
-        course = CourseFactory.create()
-        self.verify_url(
-            unicode(course.id.make_asset_key('asset', course.course_image)),
-            utils.course_image_url(course)
-        )
-
-    def test_non_ascii_image_name(self):
-        """ Verify that non-ascii image names are cleaned """
-        course_image = u'before_\N{SNOWMAN}_after.jpg'
-        course = CourseFactory.create(course_image=course_image)
-        self.verify_url(
-            unicode(course.id.make_asset_key('asset', course_image.replace(u'\N{SNOWMAN}', '_'))),
-            utils.course_image_url(course)
-        )
-
-    def test_spaces_in_image_name(self):
-        """ Verify that image names with spaces in them are cleaned """
-        course_image = u'before after.jpg'
-        course = CourseFactory.create(course_image=u'before after.jpg')
-        self.verify_url(
-            unicode(course.id.make_asset_key('asset', course_image.replace(" ", "_"))),
-            utils.course_image_url(course)
-        )
-
-    @ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo)
-    def test_empty_image_name(self, default_store):
-        """ Verify that empty image names are cleaned """
-        course_image = u''
-        course = CourseFactory.create(course_image=course_image, default_store=default_store)
-        self.assertEquals(
-            course_image,
-            utils.course_image_url(course),
-        )
-
-
 class XBlockVisibilityTestCase(ModuleStoreTestCase):
     """Tests for xblock visibility for students."""
 

cms/djangoapps/contentstore/utils.py

@@ -158,16 +158,6 @@ def get_lms_link_for_certificate_web_view(user_id, course_key, mode):
     )
 
 
-def course_image_url(course):
-    """Returns the image url for the course."""
-    try:
-        loc = StaticContent.compute_location(course.location.course_key, course.course_image)
-    except InvalidKeyError:
-        return ''
-    path = StaticContent.serialize_asset_key_with_slash(loc)
-    return path
-
-
 # pylint: disable=invalid-name
 def is_currently_visible_to_students(xblock):
     """

cms/djangoapps/contentstore/views/course.py

@@ -68,6 +68,7 @@ from openedx.core.djangoapps.programs.models import ProgramsApiConfig
 from openedx.core.djangoapps.programs.utils import get_programs
 from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration
 from openedx.core.lib.course_tabs import CourseTabPluginManager
+from openedx.core.lib.courses import course_image_url
 from openedx.core.lib.js_utils import escape_json_dumps
 from student import auth
 from student.auth import has_course_author_access, has_studio_write_access, has_studio_read_access
@@ -939,7 +940,7 @@ def settings_handler(request, course_key_string):
                 'context_course': course_module,
                 'course_locator': course_key,
                 'lms_link_for_about_page': utils.get_lms_link_for_about_page(course_key),
-                'course_image_url': utils.course_image_url(course_module),
+                'course_image_url': course_image_url(course_module),
                 'details_url': reverse_course_url('settings_handler', course_key),
                 'about_page_editable': about_page_editable,
                 'short_description_editable': short_description_editable,

cms/djangoapps/models/settings/course_details.py

@@ -8,9 +8,10 @@ from django.conf import settings
 
 from opaque_keys.edx.locations import Location
 from xmodule.modulestore.exceptions import ItemNotFoundError
-from contentstore.utils import course_image_url, has_active_web_certificate
+from contentstore.utils import has_active_web_certificate
 from models.settings import course_grading
 from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration
+from openedx.core.lib.courses import course_image_url
 from xmodule.fields import Date
 from xmodule.modulestore.django import modulestore
 

common/djangoapps/util/parsing_utils.py

@@ -1,17 +0,0 @@
-"""
-Utility function for some parsing stuff
-"""
-from xmodule.contentstore.content import StaticContent
-
-
-def course_image_url(course):
-    """
-    Return url of course image.
-    Args:
-        course(CourseDescriptor) : The course id to retrieve course image url.
-    Returns:
-        Absolute url of course image.
-    """
-    loc = StaticContent.compute_location(course.id, course.course_image)
-    url = StaticContent.serialize_asset_key_with_slash(loc)
-    return url

lms/djangoapps/bulk_email/tasks.py

@@ -39,7 +39,8 @@ from bulk_email.models import (
     SEND_TO_MYSELF, SEND_TO_ALL, TO_OPTIONS,
     SEND_TO_STAFF,
 )
-from courseware.courses import get_course, course_image_url
+from courseware.courses import get_course
+from openedx.core.lib.courses import course_image_url
 from student.roles import CourseStaffRole, CourseInstructorRole
 from instructor_task.models import InstructorTask
 from instructor_task.subtasks import (

lms/djangoapps/certificates/views/webview.py

@@ -15,7 +15,6 @@ from django.utils.translation import ugettext as _
 from django.utils.encoding import smart_str
 from django.core.urlresolvers import reverse
 
-from courseware.courses import course_image_url
 from courseware.access import has_access
 from edxmako.shortcuts import render_to_response
 from edxmako.template import Template
@@ -23,6 +22,7 @@ from eventtracking import tracker
 from microsite_configuration import microsite
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey
+from openedx.core.lib.courses import course_image_url
 from student.models import LinkedInAddToProfileConfiguration
 from util import organizations_helpers as organization_api
 from util.views import handle_500

lms/djangoapps/course_api/serializers.py

@@ -9,7 +9,8 @@ from django.template import defaultfilters
 
 from rest_framework import serializers
 
-from lms.djangoapps.courseware.courses import course_image_url, get_course_about_section
+from lms.djangoapps.courseware.courses import get_course_about_section
+from openedx.core.lib.courses import course_image_url
 from xmodule.course_module import DEFAULT_START_DATE
 
 

lms/djangoapps/course_structure_api/v0/serializers.py

@@ -3,7 +3,7 @@
 from django.core.urlresolvers import reverse
 from rest_framework import serializers
 
-from courseware.courses import course_image_url
+from openedx.core.lib.courses import course_image_url
 
 
 class CourseSerializer(serializers.Serializer):

lms/djangoapps/courseware/courses.py

@@ -6,7 +6,6 @@ from datetime import datetime
 from collections import defaultdict
 from fs.errors import ResourceNotFoundError
 import logging
-import inspect
 
 from path import Path as path
 import pytz
@@ -17,7 +16,6 @@ from edxmako.shortcuts import render_to_string
 from xmodule.modulestore import ModuleStoreEnum
 from opaque_keys.edx.keys import CourseKey
 from xmodule.modulestore.django import modulestore
-from xmodule.contentstore.content import StaticContent
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from static_replace import replace_static_urls
 from xmodule.modulestore import ModuleStoreEnum
@@ -114,28 +112,6 @@ def get_course_with_access(user, action, course_key, depth=0, check_if_enrolled=
     return course
 
 
-def course_image_url(course):
-    """Try to look up the image url for the course.  If it's not found,
-    log an error and return the dead link"""
-    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
-        # 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
-        else:
-            url += '/images/course_image.jpg'
-    elif not course.course_image:
-        # if course_image 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)
-        url = StaticContent.serialize_asset_key_with_slash(loc)
-    return url
-
-
 def find_file(filesystem, dirs, filename):
     """
     Looks for a filename in a list of dirs on a filesystem, in the specified order.

lms/djangoapps/courseware/tests/test_courses.py

@@ -14,7 +14,7 @@ from django.test.client import RequestFactory
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 
 from courseware.courses import (
-    get_course_by_id, get_cms_course_link, course_image_url,
+    get_course_by_id, get_cms_course_link,
     get_course_info_section, get_course_about_section, get_cms_block_link
 )
 
@@ -23,6 +23,7 @@ from courseware.module_render import get_module_for_descriptor
 from courseware.tests.helpers import get_request_for_user
 from courseware.model_data import FieldDataCache
 from lms.djangoapps.courseware.courseware_access_exception import CoursewareAccessException
+from openedx.core.lib.courses import course_image_url
 from student.tests.factories import UserFactory
 from xmodule.modulestore.django import _get_modulestore_branch_setting, modulestore
 from xmodule.modulestore import ModuleStoreEnum

lms/djangoapps/courseware/tests/test_module_render.py

@@ -29,7 +29,7 @@ from xblock.fragment import Fragment
 from capa.tests.response_xml_factory import OptionResponseXMLFactory
 from course_modes.models import CourseMode
 from courseware import module_render as render
-from courseware.courses import get_course_with_access, course_image_url, get_course_info_section
+from courseware.courses import get_course_with_access, get_course_info_section
 from courseware.field_overrides import OverrideFieldData
 from courseware.model_data import FieldDataCache
 from courseware.module_render import hash_resource, get_module_for_descriptor
@@ -39,6 +39,7 @@ from courseware.tests.tests import LoginEnrollmentTestCase
 from courseware.tests.test_submitting_problems import TestSubmittingProblems
 from lms.djangoapps.lms_xblock.runtime import quote_slashes
 from lms.djangoapps.lms_xblock.field_data import LmsFieldData
+from openedx.core.lib.courses import course_image_url
 from student.models import anonymous_id_for_user
 from xmodule.modulestore.tests.django_utils import (
     TEST_DATA_MIXED_TOY_MODULESTORE,

lms/templates/course.html

@@ -2,7 +2,8 @@
 <%!
 from django.utils.translation import ugettext as _
 from django.core.urlresolvers import reverse
-from courseware.courses import course_image_url, get_course_about_section
+from courseware.courses import get_course_about_section
+from openedx.core.lib.courses import course_image_url
 %>
 <%page args="course" />
 <article class="course" id="${course.id | h}" role="region" aria-label="${get_course_about_section(request, course, 'title')}">

lms/templates/courseware/course_about.html

@@ -3,9 +3,10 @@
 from microsite_configuration import microsite
 from django.utils.translation import ugettext as _
 from django.core.urlresolvers import reverse
-from courseware.courses import course_image_url, get_course_about_section
+from courseware.courses import get_course_about_section
 from django.conf import settings
 from edxmako.shortcuts import marketing_link
+from openedx.core.lib.courses import course_image_url
 %>
 
 <%inherit file="../main.html" />

lms/templates/shoppingcart/receipt.html

@@ -3,9 +3,10 @@
 from django.utils.translation import ugettext as _
 from django.utils.translation import ungettext
 from django.core.urlresolvers import reverse
-from courseware.courses import course_image_url, get_course_about_section, get_course_by_id
+from courseware.courses import get_course_about_section, get_course_by_id
 from markupsafe import escape
 from microsite_configuration import microsite
+from openedx.core.lib.courses import course_image_url
 %>
 
 <%block name="billing_details_highlight">

lms/templates/shoppingcart/registration_code_receipt.html

@@ -1,7 +1,8 @@
 <%!
 from django.utils.translation import ugettext as _
 from django.core.urlresolvers import reverse
-from courseware.courses import course_image_url, get_course_about_section
+from courseware.courses import get_course_about_section
+from openedx.core.lib.courses import course_image_url
 %>
 <%inherit file="../main.html" />
 <%namespace name='static' file='/static_content.html'/>

lms/templates/shoppingcart/registration_code_redemption.html

@@ -1,7 +1,8 @@
 <%!
 from django.utils.translation import ugettext as _
 from django.core.urlresolvers import reverse
-from courseware.courses import course_image_url, get_course_about_section
+from courseware.courses import get_course_about_section
+from openedx.core.lib.courses import course_image_url
 %>
 <%inherit file="../main.html" />
 <%namespace name='static' file='/static_content.html'/>

lms/templates/shoppingcart/shopping_cart.html

@@ -2,12 +2,12 @@
 <%block name="review_highlight">class="active"</%block>
 
 <%!
-from courseware.courses import course_image_url, get_course_about_section
+from courseware.courses import get_course_about_section
 from django.core.urlresolvers import reverse
 from edxmako.shortcuts import marketing_link
 from django.utils.translation import ugettext as _
 from django.utils.translation import ungettext
-
+from openedx.core.lib.courses import course_image_url
 %>
 
 <section class="wrapper confirm-enrollment shopping-cart cart-view">

openedx/core/djangoapps/content/course_overviews/models.py

@@ -97,7 +97,7 @@ class CourseOverview(TimeStampedModel):
             CourseOverview: overview extracted from the given course
         """
         from lms.djangoapps.certificates.api import get_active_web_certificate
-        from lms.djangoapps.courseware.courses import course_image_url
+        from openedx.core.lib.courses import course_image_url
 
         # Workaround for a problem discovered in https://openedx.atlassian.net/browse/TNL-2806.
         # If the course has a malformed grading policy such that

openedx/core/djangoapps/content/course_overviews/tests.py

@@ -11,7 +11,7 @@ import pytz
 from django.utils import timezone
 
 from lms.djangoapps.certificates.api import get_active_web_certificate
-from lms.djangoapps.courseware.courses import course_image_url
+from openedx.core.lib.courses import course_image_url
 from xmodule.course_metadata_utils import DEFAULT_START_DATE
 from xmodule.error_module import ErrorDescriptor
 from xmodule.modulestore import ModuleStoreEnum

openedx/core/lib/courses.py

@@ -0,0 +1,30 @@
+"""
+Common utility functions related to courses.
+"""
+from django.conf import settings
+
+from xmodule.modulestore.django import modulestore
+from xmodule.contentstore.content import StaticContent
+from xmodule.modulestore import ModuleStoreEnum
+
+
+def course_image_url(course):
+    """Try to look up the image url for the course.  If it's not found,
+    log an error and return the dead link"""
+    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
+        # 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
+        else:
+            url += '/images/course_image.jpg'
+    elif not course.course_image:
+        # if course_image 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)
+        url = StaticContent.serialize_asset_key_with_slash(loc)
+    return url

openedx/core/lib/tests/test_courses.py

@@ -0,0 +1,59 @@
+"""
+Tests for functionality in openedx/core/lib/courses.py.
+"""
+
+import ddt
+from xmodule.modulestore import ModuleStoreEnum
+from xmodule.modulestore.tests.factories import CourseFactory
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+
+from ..courses import course_image_url
+
+
+@ddt.ddt
+class CourseImageTestCase(ModuleStoreTestCase):
+    """Tests for course image URLs."""
+
+    def verify_url(self, expected_url, actual_url):
+        """
+        Helper method for verifying the URL is as expected.
+        """
+        if not expected_url.startswith("/"):
+            expected_url = "/" + expected_url
+        self.assertEquals(expected_url, actual_url)
+
+    def test_get_image_url(self):
+        """Test image URL formatting."""
+        course = CourseFactory.create()
+        self.verify_url(
+            unicode(course.id.make_asset_key('asset', course.course_image)),
+            course_image_url(course)
+        )
+
+    def test_non_ascii_image_name(self):
+        """ Verify that non-ascii image names are cleaned """
+        course_image = u'before_\N{SNOWMAN}_after.jpg'
+        course = CourseFactory.create(course_image=course_image)
+        self.verify_url(
+            unicode(course.id.make_asset_key('asset', course_image.replace(u'\N{SNOWMAN}', '_'))),
+            course_image_url(course)
+        )
+
+    def test_spaces_in_image_name(self):
+        """ Verify that image names with spaces in them are cleaned """
+        course_image = u'before after.jpg'
+        course = CourseFactory.create(course_image=u'before after.jpg')
+        self.verify_url(
+            unicode(course.id.make_asset_key('asset', course_image.replace(" ", "_"))),
+            course_image_url(course)
+        )
+
+    @ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo)
+    def test_empty_image_name(self, default_store):
+        """ Verify that empty image names are cleaned """
+        course_image = u''
+        course = CourseFactory.create(course_image=course_image, default_store=default_store)
+        self.assertEquals(
+            course_image,
+            course_image_url(course),
+        )