Expose banner image URL in course API

Banner image URL is exposed on the course list and course details API
endpoints. The new `banner_image_url` API field has both the relative
and absolute URLs for the image it represents, if it is set.
This commit is contained in:
Gábor Boros
2020-09-22 17:40:11 +02:00
parent a90b66546d
commit 2a35410fe4
5 changed files with 112 additions and 11 deletions

View File

@@ -11,6 +11,7 @@ from django.urls import reverse
from rest_framework import serializers
from openedx.core.djangoapps.models.course_details import CourseDetails
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.lib.api.fields import AbsoluteURLField
@@ -32,6 +33,40 @@ class _MediaSerializer(serializers.Serializer): # pylint: disable=abstract-meth
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)
uri_absolute = serializers.SerializerMethodField(source="*")
def get_uri_absolute(self, course_overview):
"""
Convert the media resource's URI to an absolute URI.
"""
uri = getattr(course_overview, self.uri_attribute)
if not uri:
# Return empty string here, to keep the same
# response type in case uri is empty as well.
return ""
cdn_applied_uri = 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(cdn_applied_uri)
class ImageSerializer(serializers.Serializer): # pylint: disable=abstract-method
"""
Collection of URLs pointing to images of various sizes.
@@ -48,6 +83,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')

View File

@@ -39,15 +39,22 @@ class TestCourseSerializer(CourseApiFactoryMixin, ModuleStoreTestCase):
self.honor_user = self.create_user('honor', is_staff=False)
self.request_factory = APIRequestFactory()
course_id = u'edX/toy/2012_Fall'
banner_image_uri = u'/c4x/edX/toy/asset/images_course_image.jpg'
banner_image_absolute_uri = u'http://testserver' + banner_image_uri
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_uri,
'uri_absolute': banner_image_absolute_uri,
},
'course_image': {
'uri': image_path,
},
@@ -74,7 +81,7 @@ class TestCourseSerializer(CourseApiFactoryMixin, ModuleStoreTestCase):
'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):

View File

@@ -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(),
),
migrations.AddField(
model_name='historicalcourseoverview',
name='banner_image_url',
field=models.TextField(),
),
]

View File

@@ -63,7 +63,7 @@ class CourseOverview(TimeStampedModel):
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()
@@ -86,6 +86,8 @@ class CourseOverview(TimeStampedModel):
announcement = DateTimeField(null=True)
# URLs
# Not allowing null per django convention; not sure why many TextFields in this model do allow null
banner_image_url = TextField()
course_image_url = TextField()
social_sharing_url = TextField(null=True)
end_of_course_survey_url = TextField(null=True)
@@ -196,6 +198,7 @@ class CourseOverview(TimeStampedModel):
course_overview.advertised_start = course.advertised_start
course_overview.announcement = course.announcement
course_overview.banner_image_url = course_image_url(course, 'banner_image')
course_overview.course_image_url = course_image_url(course)
course_overview.social_sharing_url = course.social_sharing_url
@@ -728,6 +731,22 @@ class CourseOverview(TimeStampedModel):
"""
return get_closest_released_language(self.language) if self.language else None
def apply_cdn_to_url(self, image_url):
"""
Applies a new CDN/base URL to the given URLs if CDN configuration is
enabled.
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.
@@ -738,14 +757,8 @@ class CourseOverview(TimeStampedModel):
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()
}

View File

@@ -382,7 +382,7 @@ class CourseOverviewTestCase(CatalogIntegrationMixin, ModuleStoreTestCase, Cache
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, 4, 4), (ModuleStoreEnum.Type.split, 3, 3))
@ddt.unpack
def test_versioning(self, modulestore_type, min_mongo_calls, max_mongo_calls):
"""
@@ -789,6 +789,28 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
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 CDN is applied for a URL when apply_cdn_to_url called directly.
Apply CDN/base URL to the given URL if CDN configuration is enabled
and the URL is not 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):
"""