diff --git a/.gitignore b/.gitignore index f90cabec15..a94d37922f 100644 --- a/.gitignore +++ b/.gitignore @@ -85,4 +85,5 @@ private.py docs/_build/ course_discovery/static/bower_components/ node_modules/ +course_discovery/media/ docker/volumes/ diff --git a/course_discovery/apps/api/fields.py b/course_discovery/apps/api/fields.py new file mode 100644 index 0000000000..2ada952642 --- /dev/null +++ b/course_discovery/apps/api/fields.py @@ -0,0 +1,26 @@ +from rest_framework import serializers + + +class StdImageSerializerField(serializers.Field): + """ + Custom serializer field to render out proper JSON representation of the StdImage field on model + """ + def to_representation(self, obj): + serialized = {} + for size_key in obj.field.variations: + # Get different sizes specs from the model field + # Then get the file path from the available files + sized_file = getattr(obj, size_key, None) + if sized_file: + path = sized_file.url + serialized_image = serialized.setdefault(size_key, {}) + # In case MEDIA_URL does not include scheme+host, ensure that the URLs are absolute and not relative + serialized_image['url'] = self.context['request'].build_absolute_uri(path) + serialized_image['width'] = obj.field.variations[size_key]['width'] + serialized_image['height'] = obj.field.variations[size_key]['height'] + + return serialized + + def to_internal_value(self, obj): + """ We do not need to save/edit this banner image through serializer yet """ + pass diff --git a/course_discovery/apps/api/serializers.py b/course_discovery/apps/api/serializers.py index 2eab6e1356..bf3387ea11 100644 --- a/course_discovery/apps/api/serializers.py +++ b/course_discovery/apps/api/serializers.py @@ -9,6 +9,7 @@ from rest_framework.fields import DictField from taggit_serializer.serializers import TagListSerializerField, TaggitSerializer +from course_discovery.apps.api.fields import StdImageSerializerField from course_discovery.apps.catalogs.models import Catalog from course_discovery.apps.course_metadata.models import ( Course, CourseRun, Image, Organization, Person, Prerequisite, Seat, Subject, Video, Program, ProgramType, @@ -279,6 +280,7 @@ class ProgramSerializer(serializers.ModelSerializer): courses = serializers.SerializerMethodField() authoring_organizations = OrganizationSerializer(many=True) type = serializers.SlugRelatedField(slug_field='name', queryset=ProgramType.objects.all()) + banner_image = StdImageSerializerField() def get_courses(self, program): course_serializer = ProgramCourseSerializer( @@ -294,8 +296,8 @@ def get_courses(self, program): class Meta: model = Program fields = ('uuid', 'title', 'subtitle', 'type', 'marketing_slug', 'marketing_url', 'card_image_url', - 'banner_image_url', 'authoring_organizations', 'courses',) - read_only_fields = ('uuid', 'marketing_url',) + 'banner_image', 'banner_image_url', 'authoring_organizations', 'courses',) + read_only_fields = ('uuid', 'marketing_url', 'banner_image') class AffiliateWindowSerializer(serializers.ModelSerializer): diff --git a/course_discovery/apps/api/tests/test_serializers.py b/course_discovery/apps/api/tests/test_serializers.py index dbf3c27a9a..d990e24bd2 100644 --- a/course_discovery/apps/api/tests/test_serializers.py +++ b/course_discovery/apps/api/tests/test_serializers.py @@ -16,6 +16,7 @@ from course_discovery.apps.catalogs.tests.factories import CatalogFactory from course_discovery.apps.core.models import User from course_discovery.apps.core.tests.factories import UserFactory +from course_discovery.apps.core.tests.helpers import make_image_file from course_discovery.apps.course_metadata.models import CourseRun, Program from course_discovery.apps.course_metadata.search_indexes import OrganizationsMixin from course_discovery.apps.course_metadata.tests.factories import ( @@ -251,7 +252,20 @@ def test_data(self): org_list = OrganizationFactory.create_batch(1) course_list = CourseFactory.create_batch(3) program = ProgramFactory(authoring_organizations=org_list, courses=course_list) + program.banner_image = make_image_file('test_banner.jpg') + program.save() serializer = ProgramSerializer(program, context={'request': request}) + expected_banner_image_urls = { + size_key: { + 'url': '{}{}'.format( + 'http://testserver', + getattr(program.banner_image, size_key).url + ), + 'width': program.banner_image.field.variations[size_key]['width'], + 'height': program.banner_image.field.variations[size_key]['height'] + } + for size_key in program.banner_image.field.variations + } expected = { 'uuid': str(program.uuid), @@ -262,6 +276,7 @@ def test_data(self): 'marketing_url': program.marketing_url, 'card_image_url': program.card_image_url, 'banner_image_url': program.banner_image_url, + 'banner_image': expected_banner_image_urls, 'authoring_organizations': OrganizationSerializer(program.authoring_organizations, many=True).data, 'courses': ProgramCourseSerializer( program.courses, @@ -300,6 +315,7 @@ def test_with_exclusions(self): 'marketing_slug': program.marketing_slug, 'marketing_url': program.marketing_url, 'card_image_url': program.card_image_url, + 'banner_image': {}, 'banner_image_url': program.banner_image_url, 'authoring_organizations': OrganizationSerializer(program.authoring_organizations, many=True).data, 'courses': ProgramCourseSerializer( diff --git a/course_discovery/apps/core/tests/helpers.py b/course_discovery/apps/core/tests/helpers.py new file mode 100644 index 0000000000..53e4c41c05 --- /dev/null +++ b/course_discovery/apps/core/tests/helpers.py @@ -0,0 +1,22 @@ +""" +Helper methods for testing the processing of image files. +""" +from io import BytesIO +from PIL import Image + +from django.core.files.uploadedfile import SimpleUploadedFile + + +def make_image_stream(): + """ + Helper to generate values for program banner_image + """ + image = Image.new('RGB', (1440, 900), 'green') + bio = BytesIO() + image.save(bio, format='JPEG') + return bio + + +def make_image_file(name): + image_stream = make_image_stream() + return SimpleUploadedFile(name, image_stream.getvalue(), content_type='image/jpeg') diff --git a/course_discovery/apps/core/tests/utils.py b/course_discovery/apps/core/tests/utils.py index 5149181703..0d9dcd4b08 100644 --- a/course_discovery/apps/core/tests/utils.py +++ b/course_discovery/apps/core/tests/utils.py @@ -5,6 +5,8 @@ BaseFuzzyAttribute, FuzzyText, FuzzyChoice ) +from course_discovery.apps.core.tests.helpers import make_image_stream + class FuzzyDomain(BaseFuzzyAttribute): def fuzz(self): @@ -81,3 +83,12 @@ def request_callback(request): return 200, {}, json.dumps(body) return request_callback + + +def mock_jpeg_callback(): + def request_callback(request): # pylint: disable=unused-argument + image_stream = make_image_stream() + + return 200, {}, image_stream.getvalue() + + return request_callback diff --git a/course_discovery/apps/course_metadata/data_loaders/api.py b/course_discovery/apps/course_metadata/data_loaders/api.py index 17737c11f0..595b571a80 100644 --- a/course_discovery/apps/course_metadata/data_loaders/api.py +++ b/course_discovery/apps/course_metadata/data_loaders/api.py @@ -1,7 +1,10 @@ import logging from decimal import Decimal +from io import BytesIO +import requests from opaque_keys.edx.keys import CourseKey +from django.core.files import File from course_discovery.apps.core.models import Currency from course_discovery.apps.course_metadata.data_loaders import AbstractDataLoader @@ -299,6 +302,7 @@ def update_program(self, body): program, __ = Program.objects.update_or_create(uuid=uuid, defaults=defaults) self._update_program_organizations(body, program) self._update_program_courses_and_runs(body, program) + self._update_program_banner_image(body, program) program.save() except Exception: # pylint: disable=broad-except logger.exception('Failed to load program %s', uuid) @@ -335,3 +339,20 @@ def _get_banner_image_url(self, body): image_key = 'w{width}h{height}'.format(width=self.image_width, height=self.image_height) image_url = body.get('banner_image_urls', {}).get(image_key) return image_url + + def _update_program_banner_image(self, body, program): + image_url = self._get_banner_image_url(body) + if not image_url: + logger.warning('There are no banner image url for program %s', program.title) + return + + r = requests.get(image_url) + if r.status_code == 200: + banner_downloaded = File(BytesIO(r.content)) + program.banner_image.save( + 'banner.jpg', + banner_downloaded + ) + program.save() + else: + logger.exception('Loading the banner image %s for program %s failed', image_url, program.title) diff --git a/course_discovery/apps/course_metadata/data_loaders/tests/__init__.py b/course_discovery/apps/course_metadata/data_loaders/tests/__init__.py index f54caed6c0..60bf6fec80 100644 --- a/course_discovery/apps/course_metadata/data_loaders/tests/__init__.py +++ b/course_discovery/apps/course_metadata/data_loaders/tests/__init__.py @@ -1 +1,2 @@ JSON = 'application/json' +JPEG = 'image/jpeg' diff --git a/course_discovery/apps/course_metadata/data_loaders/tests/test_api.py b/course_discovery/apps/course_metadata/data_loaders/tests/test_api.py index e6049358b1..01948b427c 100644 --- a/course_discovery/apps/course_metadata/data_loaders/tests/test_api.py +++ b/course_discovery/apps/course_metadata/data_loaders/tests/test_api.py @@ -7,11 +7,11 @@ from django.test import TestCase from pytz import UTC -from course_discovery.apps.core.tests.utils import mock_api_callback +from course_discovery.apps.core.tests.utils import mock_api_callback, mock_jpeg_callback from course_discovery.apps.course_metadata.data_loaders.api import ( OrganizationsApiDataLoader, CoursesApiDataLoader, EcommerceApiDataLoader, AbstractDataLoader, ProgramsApiDataLoader ) -from course_discovery.apps.course_metadata.data_loaders.tests import JSON +from course_discovery.apps.course_metadata.data_loaders.tests import JSON, JPEG from course_discovery.apps.course_metadata.data_loaders.tests.mixins import ApiClientTestMixin, DataLoaderTestMixin from course_discovery.apps.course_metadata.models import ( Course, CourseRun, Organization, Seat, Program, ProgramType, @@ -418,6 +418,22 @@ def assert_program_loaded(self, body): # Verify the additional course runs added in create_mock_courses_and_runs are excluded. self.assertEqual(program.excluded_course_runs.count(), len(course_codes)) + def assert_program_banner_image_loaded(self, body): + """ Assert a program corresponding to the specified data body has banner image loaded into DB """ + program = Program.objects.get(uuid=AbstractDataLoader.clean_string(body['uuid']), partner=self.partner) + banner_image_url = body.get('banner_image_urls', {}).get('w1440h480') + if banner_image_url: + for size_key in program.banner_image.field.variations: + # Get different sizes specs from the model field + # Then get the file path from the available files + sized_image = getattr(program.banner_image, size_key, None) + self.assertIsNotNone(sized_image) + if sized_image: + path = getattr(program.banner_image, size_key).url + self.assertIsNotNone(path) + self.assertIsNotNone(program.banner_image.field.variations[size_key]['width']) + self.assertIsNotNone(program.banner_image.field.variations[size_key]['height']) + @responses.activate def test_ingest(self): """ Verify the method ingests data from the Organizations API. """ @@ -427,7 +443,7 @@ def test_ingest(self): self.loader.ingest() # Verify the API was called with the correct authorization header - self.assert_api_called(1) + self.assert_api_called(2) # Verify the Programs were created correctly self.assertEqual(Program.objects.count(), len(api_data)) @@ -452,3 +468,25 @@ def test_ingest_with_missing_organizations(self): self.assertEqual(Program.objects.count(), len(api_data)) self.assertEqual(Organization.objects.count(), 0) + + @responses.activate + def test_ingest_with_existing_banner_image(self): + programs = self.mock_api() + + for program_data in programs: + banner_image_url = program_data.get('banner_image_urls', {}).get('w1440h480') + if banner_image_url: + responses.add_callback( + responses.GET, + banner_image_url, + callback=mock_jpeg_callback(), + content_type=JPEG + ) + + self.loader.ingest() + # Verify the API was called with the correct authorization header + self.assert_api_called(2) + + for program in programs: + self.assert_program_loaded(program) + self.assert_program_banner_image_loaded(program) diff --git a/course_discovery/apps/course_metadata/migrations/0019_program_banner_image.py b/course_discovery/apps/course_metadata/migrations/0019_program_banner_image.py new file mode 100644 index 0000000000..99fa66dcc6 --- /dev/null +++ b/course_discovery/apps/course_metadata/migrations/0019_program_banner_image.py @@ -0,0 +1,21 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +import stdimage.models +import stdimage.utils + + +class Migration(migrations.Migration): + + dependencies = [ + ('course_metadata', '0018_auto_20160815_2252'), + ] + + operations = [ + migrations.AddField( + model_name='program', + name='banner_image', + field=stdimage.models.StdImageField(upload_to=stdimage.utils.UploadToAutoSlugClassNameDir('uuid', path='/media/programs/banner_images'), null=True, blank=True), + ), + ] diff --git a/course_discovery/apps/course_metadata/models.py b/course_discovery/apps/course_metadata/models.py index e3bcc29b78..6a827eb150 100644 --- a/course_discovery/apps/course_metadata/models.py +++ b/course_discovery/apps/course_metadata/models.py @@ -14,6 +14,8 @@ from haystack.query import SearchQuerySet from simple_history.models import HistoricalRecords from sortedm2m.fields import SortedManyToManyField +from stdimage.models import StdImageField +from stdimage.utils import UploadToAutoSlugClassNameDir from taggit.managers import TaggableManager from course_discovery.apps.core.models import Currency, Partner @@ -584,7 +586,16 @@ class ProgramStatus(DjangoChoices): min_hours_effort_per_week = models.PositiveSmallIntegerField(null=True, blank=True) max_hours_effort_per_week = models.PositiveSmallIntegerField(null=True, blank=True) authoring_organizations = SortedManyToManyField(Organization, blank=True, related_name='authored_programs') - + banner_image = StdImageField( + upload_to=UploadToAutoSlugClassNameDir(path='/media/programs/banner_images', populate_from='uuid'), + blank=True, + null=True, + variations={ + 'large': (1440, 480), + 'medium': (726, 242), + 'small': (435, 145), + 'x-small': (348, 116)} + ) banner_image_url = models.URLField(null=True, blank=True, help_text=_('Image used atop detail pages')) card_image_url = models.URLField(null=True, blank=True, help_text=_('Image used for discovery cards')) video = models.ForeignKey(Video, default=None, null=True, blank=True) diff --git a/course_discovery/apps/course_metadata/tests/test_models.py b/course_discovery/apps/course_metadata/tests/test_models.py index 107144441f..21333d5847 100644 --- a/course_discovery/apps/course_metadata/tests/test_models.py +++ b/course_discovery/apps/course_metadata/tests/test_models.py @@ -5,6 +5,7 @@ import ddt import pytz from dateutil.parser import parse +from django.conf import settings from django.db import IntegrityError from django.test import TestCase from freezegun import freeze_time @@ -15,6 +16,7 @@ AbstractNamedModel, AbstractMediaModel, AbstractValueModel, CourseOrganization, Course, CourseRun, SeatType) from course_discovery.apps.course_metadata.tests import factories +from course_discovery.apps.core.tests.helpers import make_image_file from course_discovery.apps.ietf_language_tags.models import LanguageTag @@ -357,6 +359,18 @@ def test_instructors(self): self.assertEqual(self.program.instructors, set(instructors)) + def test_banner_image(self): + self.program.banner_image = make_image_file('test_banner.jpg') + self.program.save() + image_url_prefix = '{}program/'.format(settings.MEDIA_URL) + self.assertIn(image_url_prefix, self.program.banner_image.url) + for size_key in self.program.banner_image.field.variations: + # Get different sizes specs from the model field + # Then get the file path from the available files + sized_file = getattr(self.program.banner_image, size_key, None) + self.assertIsNotNone(sized_file) + self.assertIn(image_url_prefix, sized_file.url) + class PersonSocialNetworkTests(TestCase): """Tests of the PersonSocialNetwork model.""" diff --git a/course_discovery/urls.py b/course_discovery/urls.py index e93ba71b24..0152f10b96 100644 --- a/course_discovery/urls.py +++ b/course_discovery/urls.py @@ -18,6 +18,7 @@ from auth_backends.urls import auth_urlpatterns from django.conf import settings from django.conf.urls import include, url +from django.conf.urls.static import static from django.contrib import admin from course_discovery.apps.core import views as core_views @@ -43,7 +44,11 @@ url(r'^comments/', include('django_comments.urls')), ] -if settings.DEBUG and os.environ.get('ENABLE_DJANGO_TOOLBAR', False): # pragma: no cover - import debug_toolbar # pylint: disable=wrong-import-order,wrong-import-position,import-error +if settings.DEBUG: # pragma: no cover + # We need this url pattern to serve user uploaded assets according to + # https://docs.djangoproject.com/en/1.10/howto/static-files/#serving-files-uploaded-by-a-user-during-development + urlpatterns += static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) + if os.environ.get('ENABLE_DJANGO_TOOLBAR', False): + import debug_toolbar # pylint: disable=wrong-import-order,wrong-import-position,import-error - urlpatterns.append(url(r'^__debug__/', include(debug_toolbar.urls))) + urlpatterns.append(url(r'^__debug__/', include(debug_toolbar.urls))) diff --git a/requirements/base.txt b/requirements/base.txt index 79ce0a5b08..71f79210cb 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -12,6 +12,7 @@ django-haystack==2.5.0 django-libsass==0.7 django-simple-history==1.8.1 django-sortedm2m==1.3.2 +django-stdimage==2.3.3 django-storages==1.5.0 django-taggit==0.20.2 django-taggit-serializer==0.1.5