diff --git a/common/djangoapps/course_about/__init__.py b/common/djangoapps/course_about/__init__.py index 1d03d49c45..e69de29bb2 100644 --- a/common/djangoapps/course_about/__init__.py +++ b/common/djangoapps/course_about/__init__.py @@ -1,6 +0,0 @@ -""" -API for retrieving Course metadata. -This API is not intended for exposing course content, but allowing general access to descriptive course -details. - -""" diff --git a/common/djangoapps/course_about/api.py b/common/djangoapps/course_about/api.py index 8c3187cfdb..d5ddd3bf82 100644 --- a/common/djangoapps/course_about/api.py +++ b/common/djangoapps/course_about/api.py @@ -10,10 +10,13 @@ This API is exposed via the RESTful layer (views.py) but may be used directly in import logging from django.conf import settings from django.utils import importlib +from django.core.cache import cache from course_about import errors DEFAULT_DATA_API = 'course_about.data' +COURSE_ABOUT_API_CACHE_PREFIX = 'course_about_api_' + log = logging.getLogger(__name__) @@ -21,6 +24,9 @@ def get_course_about_details(course_id): """Get course about details for the given course ID. Given a Course ID, retrieve all the metadata necessary to fully describe the Course. + First its checks the default cache for given course id if its exists then returns + the course otherwise it get the course from module store and set the cache. + By default cache expiry set to 5 minutes. Args: course_id (str): The String representation of a Course ID. Used to look up the requested @@ -46,7 +52,17 @@ def get_course_about_details(course_id): }, } """ - return _data_api().get_course_about_details(course_id) + cache_key = "{}_{}".format(course_id, COURSE_ABOUT_API_CACHE_PREFIX) + cache_course_info = cache.get(cache_key) + + if cache_course_info: + return cache_course_info + + course_info = _data_api().get_course_about_details(course_id) + time_out = getattr(settings, 'COURSE_INFO_API_CACHE_TIME_OUT', 300) + cache.set(cache_key, course_info, time_out) + + return course_info def _data_api(): diff --git a/common/djangoapps/course_about/data.py b/common/djangoapps/course_about/data.py index fe2213e929..472bc7773b 100644 --- a/common/djangoapps/course_about/data.py +++ b/common/djangoapps/course_about/data.py @@ -11,6 +11,7 @@ from course_about.errors import CourseNotFoundError from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError + log = logging.getLogger(__name__) ABOUT_ATTRIBUTES = [ @@ -23,8 +24,10 @@ def get_course_about_details(course_id): # pylint: disable=unused-argument Return course information for a given course id. Args: course_id(str) : The course id to retrieve course information for. + Returns: Serializable dictionary of the Course About Information. + Raises: CourseNotFoundError """ @@ -35,10 +38,14 @@ def get_course_about_details(course_id): # pylint: disable=unused-argument raise CourseNotFoundError("course not found") except InvalidKeyError as err: raise CourseNotFoundError(err.message) - about_descriptor = {} - for attribute in ABOUT_ATTRIBUTES: - about_descriptor[attribute] = _fetch_course_detail(course_key, attribute) - return serialize_content(course_descriptor=course_descriptor, about_descriptor=about_descriptor) + + about_descriptor = { + attribute: _fetch_course_detail(course_key, attribute) + for attribute in ABOUT_ATTRIBUTES + } + + course_info = serialize_content(course_descriptor=course_descriptor, about_descriptor=about_descriptor) + return course_info def _fetch_course_detail(course_key, attribute): diff --git a/common/djangoapps/course_about/serializers.py b/common/djangoapps/course_about/serializers.py index 73d35d5471..362ea8beec 100644 --- a/common/djangoapps/course_about/serializers.py +++ b/common/djangoapps/course_about/serializers.py @@ -2,58 +2,67 @@ Serializers for all Course Descriptor and Course About Descriptor related return objects. """ -from util.parsing_utils import course_image_url +from xmodule.contentstore.content import StaticContent from django.conf import settings +DATE_FORMAT = getattr(settings, 'API_DATE_FORMAT', '%Y-%m-%d') + def serialize_content(course_descriptor, about_descriptor): - """Serialize the course descriptor and about descriptor - + """ Returns a serialized representation of the course_descriptor and about_descriptor - Args: course_descriptor(CourseDescriptor) : course descriptor object about_descriptor(dict) : Dictionary of CourseAboutDescriptor objects - - Returns: - Serializable dictionary of course information. - + return: + serialize data for course information. """ - date_format = getattr(settings, 'API_DATE_FORMAT', '%Y-%m-%d') - data = dict({"media": {}}) - data['display_name'] = getattr(course_descriptor, 'display_name', None) - start = getattr(course_descriptor, 'start', None) - end = getattr(course_descriptor, 'end', None) + data = { + 'media': {}, + 'display_name': getattr(course_descriptor, 'display_name', None), + 'course_number': course_descriptor.location.course, + 'course_id': None, + 'advertised_start': getattr(course_descriptor, 'advertised_start', None), + 'is_new': getattr(course_descriptor, 'is_new', None), + 'start': _formatted_datetime(course_descriptor, 'start'), + 'end': _formatted_datetime(course_descriptor, 'end'), + 'announcement': None, + 'effort': about_descriptor.get("effort", None) + + } + + content_id = unicode(course_descriptor.id) + data["course_id"] = unicode(content_id) + if getattr(course_descriptor, 'course_image', False): + data['media']['course_image'] = course_image_url(course_descriptor) + announcement = getattr(course_descriptor, 'announcement', None) - data['start'] = start.strftime(date_format) if start else None - data['end'] = end.strftime(date_format) if end else None - data["announcement"] = announcement.strftime(date_format) if announcement else None - data['advertised_start'] = getattr(course_descriptor, 'advertised_start', None) - data['is_new'] = getattr(course_descriptor, 'is_new', None) - image_url = '' - if hasattr(course_descriptor, 'course_image') and course_descriptor.course_image: - image_url = course_image_url(course_descriptor) - data['course_number'] = course_descriptor.location.course - data['course_id'] = unicode(course_descriptor.id) - data['media']['course_image'] = image_url - # Following code is getting the course about descriptor information - course_about_data = _course_about_serialize_content(about_descriptor) - data.update(course_about_data) + data["announcement"] = announcement.strftime(DATE_FORMAT) if announcement else None + return data -def _course_about_serialize_content(about_descriptor): - """Serialize the course about descriptor - - Returns a serialized representation of the about_descriptor - - Args: - course_descriptor(dict) : dictionary of course descriptor object - - Returns: - Serialize data for about descriptor. - +def course_image_url(course): """ - data = dict() - data["effort"] = about_descriptor.get("effort", None) - return data + 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 + + +def _formatted_datetime(course_descriptor, date_type): + """ + Return formatted date. + Args: + course_descriptor(CourseDescriptor) : The CourseDescriptor Object. + date_type (str) : Either start or end. + Returns: + formatted date or None . + """ + course_date_ = getattr(course_descriptor, date_type, None) + return course_date_.strftime(DATE_FORMAT) if course_date_ else None diff --git a/common/djangoapps/course_about/tests/__init__.py b/common/djangoapps/course_about/tests/__init__.py index fa9e685e52..e69de29bb2 100644 --- a/common/djangoapps/course_about/tests/__init__.py +++ b/common/djangoapps/course_about/tests/__init__.py @@ -1,4 +0,0 @@ -""" -Packages all tests relative to the Course About API. - -""" diff --git a/common/djangoapps/course_about/tests/test_api.py b/common/djangoapps/course_about/tests/test_api.py new file mode 100644 index 0000000000..bfa690393d --- /dev/null +++ b/common/djangoapps/course_about/tests/test_api.py @@ -0,0 +1,63 @@ +""" +Tests the logical Python API layer of the Course About API. +""" + +import ddt +import json +import unittest + +from django.test.utils import override_settings +from django.core.urlresolvers import reverse +from rest_framework.test import APITestCase +from rest_framework import status +from django.conf import settings +from xmodule.modulestore.tests.django_utils import ( + ModuleStoreTestCase, mixed_store_config +) +from xmodule.modulestore.tests.factories import CourseFactory, CourseAboutFactory +from student.tests.factories import UserFactory + +# Since we don't need any XML course fixtures, use a modulestore configuration +# that disables the XML modulestore. + +MODULESTORE_CONFIG = mixed_store_config(settings.COMMON_TEST_DATA_ROOT, {}, include_xml=False) + + +@ddt.ddt +@override_settings(MODULESTORE=MODULESTORE_CONFIG) +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') +class CourseInfoTest(ModuleStoreTestCase, APITestCase): + """ + Test course information. + """ + USERNAME = "Bob" + EMAIL = "bob@example.com" + PASSWORD = "edx" + + def setUp(self): + """ Create a course""" + super(CourseInfoTest, self).setUp() + + self.course = CourseFactory.create() + self.user = UserFactory.create(username=self.USERNAME, email=self.EMAIL, password=self.PASSWORD) + self.client.login(username=self.USERNAME, password=self.PASSWORD) + + def test_get_course_details_from_cache(self): + kwargs = dict() + kwargs["course_id"] = self.course.id + kwargs["course_runtime"] = self.course.runtime + kwargs["user_id"] = self.user.id + CourseAboutFactory.create(**kwargs) + resp = self.client.get( + reverse('courseabout', kwargs={"course_id": unicode(self.course.id)}) + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + resp_data = json.loads(resp.content) + self.assertIsNotNone(resp_data) + + resp = self.client.get( + reverse('courseabout', kwargs={"course_id": unicode(self.course.id)}) + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + resp_data = json.loads(resp.content) + self.assertIsNotNone(resp_data) diff --git a/common/djangoapps/course_about/tests/test_data.py b/common/djangoapps/course_about/tests/test_data.py index 62810e5516..e73c597464 100644 --- a/common/djangoapps/course_about/tests/test_data.py +++ b/common/djangoapps/course_about/tests/test_data.py @@ -4,7 +4,9 @@ Tests specific to the Data Aggregation Layer of the Course About API. """ import unittest from django.test.utils import override_settings +from datetime import datetime from django.conf import settings +from nose.tools import raises from xmodule.modulestore.tests.django_utils import ( ModuleStoreTestCase, mixed_store_config ) @@ -12,9 +14,7 @@ from xmodule.modulestore.tests.factories import CourseFactory from student.tests.factories import UserFactory from course_about import data from course_about.errors import CourseNotFoundError -from nose.tools import raises from xmodule.modulestore.django import modulestore -from datetime import datetime # Since we don't need any XML course fixtures, use a modulestore configuration # that disables the XML modulestore. diff --git a/common/djangoapps/course_about/tests/test_views.py b/common/djangoapps/course_about/tests/test_views.py index d8a93fb1db..074048be33 100644 --- a/common/djangoapps/course_about/tests/test_views.py +++ b/common/djangoapps/course_about/tests/test_views.py @@ -10,17 +10,17 @@ from django.core.urlresolvers import reverse from rest_framework.test import APITestCase from rest_framework import status from django.conf import settings +from datetime import datetime +from mock import patch from xmodule.modulestore.tests.django_utils import ( ModuleStoreTestCase, mixed_store_config ) from xmodule.modulestore.tests.factories import CourseFactory, CourseAboutFactory from student.tests.factories import UserFactory -from util.parsing_utils import course_image_url +from course_about.serializers import course_image_url from course_about import api from course_about.errors import CourseNotFoundError, CourseAboutError -from mock import patch from xmodule.modulestore.django import modulestore -from datetime import datetime # Since we don't need any XML course fixtures, use a modulestore configuration # that disables the XML modulestore. @@ -50,19 +50,13 @@ class CourseInfoTest(ModuleStoreTestCase, APITestCase): def test_user_not_authenticated(self): # Log out, so we're no longer authenticated self.client.logout() - - resp = self.client.get( - reverse('courseabout', kwargs={"course_id": unicode(self.course.id)}) - ) - resp_data = json.loads(resp.content) - self.assertEqual(resp.status_code, status.HTTP_200_OK) + resp_data, status_code = self._get_course_about(self.course.id) + self.assertEqual(status_code, status.HTTP_200_OK) self.assertIsNotNone(resp_data) def test_with_valid_course_id(self): - resp = self.client.get( - reverse('courseabout', kwargs={"course_id": unicode(self.course.id)}) - ) - self.assertEqual(resp.status_code, status.HTTP_200_OK) + _resp_data, status_code = self._get_course_about(self.course.id) + self.assertEqual(status_code, status.HTTP_200_OK) def test_with_invalid_course_id(self): resp = self.client.get( @@ -75,15 +69,15 @@ class CourseInfoTest(ModuleStoreTestCase, APITestCase): kwargs["course_id"] = self.course.id kwargs["course_runtime"] = self.course.runtime CourseAboutFactory.create(**kwargs) - resp = self.client.get( - reverse('courseabout', kwargs={"course_id": unicode(self.course.id)}) - ) - resp_data = json.loads(resp.content) + + resp_data, status_code = self._get_course_about(self.course.id) + all_attributes = ['display_name', 'start', 'end', 'announcement', 'advertised_start', 'is_new', 'course_number', 'course_id', 'effort', 'media', 'course_image'] for attr in all_attributes: self.assertIn(attr, str(resp_data)) + self.assertEqual(status_code, status.HTTP_200_OK) def test_get_course_about_valid_date(self): module_store = modulestore() @@ -91,13 +85,12 @@ class CourseInfoTest(ModuleStoreTestCase, APITestCase): self.course.end = datetime.now() self.course.announcement = datetime.now() module_store.update_item(self.course, self.user.id) - resp = self.client.get( - reverse('courseabout', kwargs={"course_id": unicode(self.course.id)}) - ) - course_info = json.loads(resp.content) - self.assertIsNotNone(course_info["start"]) - self.assertIsNotNone(course_info["end"]) - self.assertIsNotNone(course_info["announcement"]) + + resp_data, _status_code = self._get_course_about(self.course.id) + + self.assertIsNotNone(resp_data["start"]) + self.assertIsNotNone(resp_data["end"]) + self.assertIsNotNone(resp_data["announcement"]) def test_get_course_about_none_date(self): module_store = modulestore() @@ -105,13 +98,11 @@ class CourseInfoTest(ModuleStoreTestCase, APITestCase): self.course.end = None self.course.announcement = None module_store.update_item(self.course, self.user.id) - resp = self.client.get( - reverse('courseabout', kwargs={"course_id": unicode(self.course.id)}) - ) - course_info = json.loads(resp.content) - self.assertIsNone(course_info["start"]) - self.assertIsNone(course_info["end"]) - self.assertIsNone(course_info["announcement"]) + + resp_data, _status_code = self._get_course_about(self.course.id) + self.assertIsNone(resp_data["start"]) + self.assertIsNone(resp_data["end"]) + self.assertIsNone(resp_data["announcement"]) def test_get_course_details(self): kwargs = dict() @@ -119,12 +110,9 @@ class CourseInfoTest(ModuleStoreTestCase, APITestCase): kwargs["course_runtime"] = self.course.runtime kwargs["user_id"] = self.user.id CourseAboutFactory.create(**kwargs) - resp = self.client.get( - reverse('courseabout', kwargs={"course_id": unicode(self.course.id)}) - ) - self.assertEqual(resp.status_code, status.HTTP_200_OK) - resp_data = json.loads(resp.content) + resp_data, status_code = self._get_course_about(self.course.id) + self.assertEqual(status_code, status.HTTP_200_OK) self.assertEqual(unicode(self.course.id), resp_data['course_id']) self.assertIn('Run', resp_data['display_name']) @@ -134,31 +122,36 @@ class CourseInfoTest(ModuleStoreTestCase, APITestCase): @patch.object(api, "get_course_about_details") def test_get_enrollment_course_not_found_error(self, mock_get_course_about_details): mock_get_course_about_details.side_effect = CourseNotFoundError("Something bad happened.") - resp = self.client.get( - reverse('courseabout', kwargs={"course_id": unicode(self.course.id)}) - ) - self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND) + _resp_data, status_code = self._get_course_about(self.course.id) + self.assertEqual(status_code, status.HTTP_404_NOT_FOUND) @patch.object(api, "get_course_about_details") def test_get_enrollment_invalid_key_error(self, mock_get_course_about_details): mock_get_course_about_details.side_effect = CourseNotFoundError('a/a/a', "Something bad happened.") - resp = self.client.get( - reverse('courseabout', kwargs={"course_id": unicode(self.course.id)}) - ) - self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND) + resp_data, status_code = self._get_course_about(self.course.id) + self.assertEqual(status_code, status.HTTP_404_NOT_FOUND) + self.assertIn('An error occurred', resp_data["message"]) @patch.object(api, "get_course_about_details") def test_get_enrollment_internal_error(self, mock_get_course_about_details): mock_get_course_about_details.side_effect = CourseAboutError('error') - resp = self.client.get( - reverse('courseabout', kwargs={"course_id": unicode(self.course.id)}) - ) - self.assertEqual(resp.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR) + resp_data, status_code = self._get_course_about(self.course.id) + self.assertEqual(status_code, status.HTTP_500_INTERNAL_SERVER_ERROR) + self.assertIn('An error occurred', resp_data["message"]) @override_settings(COURSE_ABOUT_DATA_API='foo') def test_data_api_config_error(self): - # Enroll in the course and verify the URL we get sent to + # Retrive the invalid course + resp_data, status_code = self._get_course_about(self.course.id) + self.assertEqual(status_code, status.HTTP_500_INTERNAL_SERVER_ERROR) + self.assertIn('An error occurred', resp_data["message"]) + + def _get_course_about(self, course_id): + """ + helper function to get retrieve course about information. + args course_id (str): course id + """ resp = self.client.get( - reverse('courseabout', kwargs={"course_id": unicode(self.course.id)}) + reverse('courseabout', kwargs={"course_id": unicode(course_id)}) ) - self.assertEqual(resp.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR) + return json.loads(resp.content), resp.status_code diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index a06bb72799..d0229cc200 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -35,7 +35,6 @@ class XModuleFactory(Factory): @lazy_attribute def modulestore(self): from xmodule.modulestore.django import modulestore - return modulestore()