Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions lms/djangoapps/course_api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,38 @@ def get_uri(self, course_overview):
return getattr(course_overview, self.uri_attribute)


class _AbsolutMediaSerializer(_MediaSerializer): # pylint: disable=abstract-method
"""
Nested serializer to represent a media object and its absolute path.
"""
requires_context = True

def __call__(self, serializer_field):
self.context = serializer_field.context
return super(self).__call__(serializer_field)

url = serializers.SerializerMethodField(source="*")

def get_url(self, course_overview):
"""
Convert the media resource's URI to an absolute URL
"""
uri = getattr(course_overview, self.uri_attribute)

if not uri:
return

url = course_overview.apply_cdn_to_url(uri)
field = AbsoluteURLField()

# In order to use the AbsoluteURLField to have the same
# behaviour what ImageSerializer provides, we need to set
# the request for the field
field._context = {"request": self.context.get("request")}

return field.to_representation(url)


class ImageSerializer(serializers.Serializer): # pylint: disable=abstract-method
"""
Collection of URLs pointing to images of various sizes.
Expand All @@ -47,6 +79,7 @@ class _CourseApiMediaCollectionSerializer(serializers.Serializer): # pylint: di
"""
Nested serializer to represent a collection of media objects
"""
banner_image = _AbsolutMediaSerializer(source='*', uri_attribute='banner_image_url')
course_image = _MediaSerializer(source='*', uri_attribute='course_image_url')
course_video = _MediaSerializer(source='*', uri_attribute='course_video_url')
image = ImageSerializer(source='image_urls')
Expand Down
11 changes: 9 additions & 2 deletions lms/djangoapps/course_api/tests/test_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,22 @@ def setUp(self):
self.honor_user = self.create_user('honor', is_staff=False)
self.request_factory = APIRequestFactory()

course_id = u'edX/toy/2012_Fall'
banner_image_path = u'/c4x/edX/toy/asset/images_course_image.jpg'
banner_image_url = u'http://testserver' + banner_image_path
image_path = u'/c4x/edX/toy/asset/just_a_test.jpg'
image_url = u'http://testserver' + image_path
self.expected_data = {
'id': u'edX/toy/2012_Fall',
'id': course_id,
'name': u'Toy Course',
'number': u'toy',
'org': u'edX',
'short_description': u'A course about toys.',
'media': {
'banner_image': {
'uri': banner_image_path,
'url': banner_image_url,
},
'course_image': {
'uri': image_path,
},
Expand All @@ -74,7 +81,7 @@ def setUp(self):
'invitation_only': False,

# 'course_id' is a deprecated field, please use 'id' instead.
'course_id': u'edX/toy/2012_Fall',
'course_id': course_id,
}

def _get_request(self, user=None):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Generated by Django 2.2.16 on 2020-09-22 12:45

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('course_overviews', '0022_courseoverviewtab_is_hidden'),
]

operations = [
migrations.AddField(
model_name='courseoverview',
name='banner_image_url',
field=models.TextField(null=True),
),
migrations.AddField(
model_name='historicalcourseoverview',
name='banner_image_url',
field=models.TextField(null=True),
),
]
27 changes: 19 additions & 8 deletions openedx/core/djangoapps/content/course_overviews/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class Meta(object):
app_label = 'course_overviews'

# IMPORTANT: Bump this whenever you modify this model and/or add a migration.
VERSION = 11 # this one goes to eleven
VERSION = 12 # this one goes to thirteen

# Cache entry versioning.
version = IntegerField()
Expand All @@ -86,6 +86,7 @@ class Meta(object):
announcement = DateTimeField(null=True)

# URLs
banner_image_url = TextField(null=True)
course_image_url = TextField()
social_sharing_url = TextField(null=True)
end_of_course_survey_url = TextField(null=True)
Expand Down Expand Up @@ -196,6 +197,7 @@ def _create_or_update(cls, course):
course_overview.advertised_start = course.advertised_start
course_overview.announcement = course.announcement

course_overview.banner_image_url = CourseDetails.fetch_banner_image_url(course.id)
course_overview.course_image_url = course_image_url(course)
course_overview.social_sharing_url = course.social_sharing_url

Expand Down Expand Up @@ -728,6 +730,21 @@ def closest_released_language(self):
"""
return get_closest_released_language(self.language) if self.language else None

def apply_cdn_to_url(self, image_url):
"""
Given a resolution -> url, return a copy with CDN applied.

If CDN does not exist or is disabled, just returns the original. The
URL that we store in CourseOverviewImageSet is already top level path,
so we don't need to go through the /static remapping magic that happens
with other course assets. We just need to add the CDN server if appropriate.
"""
cdn_config = AssetBaseUrlConfig.current()
if not cdn_config.enabled:
return image_url

return self._apply_cdn_to_url(image_url, cdn_config.base_url)

def apply_cdn_to_urls(self, image_urls):
"""
Given a dict of resolutions -> urls, return a copy with CDN applied.
Expand All @@ -738,14 +755,8 @@ def apply_cdn_to_urls(self, image_urls):
happens with other course assets. We just need to add the CDN server if
appropriate.
"""
cdn_config = AssetBaseUrlConfig.current()
if not cdn_config.enabled:
return image_urls

base_url = cdn_config.base_url

return {
resolution: self._apply_cdn_to_url(url, base_url)
resolution: self.apply_cdn_to_url(url)
for resolution, url in image_urls.items()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ def test_malformed_grading_policy(self):
course_overview = CourseOverview._create_or_update(course) # pylint: disable=protected-access
self.assertEqual(course_overview.lowest_passing_grade, None)

@ddt.data((ModuleStoreEnum.Type.mongo, 4, 4), (ModuleStoreEnum.Type.split, 3, 4))
@ddt.data((ModuleStoreEnum.Type.mongo, 5, 5), (ModuleStoreEnum.Type.split, 3, 3))
@ddt.unpack
def test_versioning(self, modulestore_type, min_mongo_calls, max_mongo_calls):
"""
Expand Down Expand Up @@ -789,6 +789,25 @@ def test_cdn_with_external_image(self, modulestore_type):
self.assertTrue(modified_urls['small'].startswith(expected_cdn_url))
self.assertEqual(modified_urls['large'], start_urls['large'])

@ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
def test_cdn_with_a_single_external_image(self, modulestore_type):
"""
Test that we return CDN prefixed URLs unless they're absolute.
"""
with self.store.default_store(modulestore_type):
course = CourseFactory.create(default_store=modulestore_type)
overview = CourseOverview.get_from_id(course.id)

# Now enable the CDN...
AssetBaseUrlConfig.objects.create(enabled=True, base_url='fakecdn.edx.org')
expected_cdn_url = "//fakecdn.edx.org"

start_url = "/static/overview.png"
modified_url = overview.apply_cdn_to_url(start_url)

self.assertNotEqual(start_url, modified_url)
self.assertTrue(modified_url.startswith(expected_cdn_url))

@ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
def test_error_generating_thumbnails(self, modulestore_type):
"""
Expand Down
7 changes: 7 additions & 0 deletions openedx/core/djangoapps/models/course_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,13 @@ def fetch_about_attribute(cls, course_key, attribute):
value = None
return value

@classmethod
def fetch_banner_image_url(cls, course_key):
"""
Fetch only the banner image url of the given course from the descriptor.
"""
return course_image_url(modulestore().get_course(course_key), 'banner_image')

@classmethod
def fetch(cls, course_key):
"""
Expand Down
9 changes: 9 additions & 0 deletions openedx/core/djangoapps/models/tests/test_course_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,3 +179,12 @@ def test_fetch_video(self):
self.assertEqual(CourseDetails.fetch_youtube_video_id(self.course.id), video_value)
video_url = CourseDetails.fetch_video_url(self.course.id)
self.assertRegex(video_url, r'http://.*{}'.format(video_value))

def test_fetch_banner_image_url(self):
course_details = CourseDetails.fetch(self.course.id)
banner_image_asset_path = CourseDetails.fetch_banner_image_url(self.course.id)

self.assertEqual(
course_details.banner_image_asset_path,
banner_image_asset_path
)