From d170c92f3b3c654d6c406b2a73ae9108809f85f2 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Wed, 2 Dec 2015 14:31:54 -0500 Subject: [PATCH] Refactor course_image_url --- .../contentstore/courseware_index.py | 2 +- .../contentstore/tests/test_utils.py | 49 --------------- cms/djangoapps/contentstore/utils.py | 10 ---- cms/djangoapps/contentstore/views/course.py | 3 +- .../models/settings/course_details.py | 3 +- common/djangoapps/util/parsing_utils.py | 17 ------ lms/djangoapps/bulk_email/tasks.py | 3 +- lms/djangoapps/certificates/views/webview.py | 2 +- lms/djangoapps/course_api/serializers.py | 3 +- .../course_structure_api/v0/serializers.py | 2 +- lms/djangoapps/courseware/courses.py | 24 -------- .../courseware/tests/test_courses.py | 3 +- .../courseware/tests/test_module_render.py | 3 +- lms/templates/course.html | 3 +- lms/templates/courseware/course_about.html | 3 +- lms/templates/shoppingcart/receipt.html | 3 +- .../registration_code_receipt.html | 3 +- .../registration_code_redemption.html | 3 +- lms/templates/shoppingcart/shopping_cart.html | 4 +- .../content/course_overviews/models.py | 2 +- .../content/course_overviews/tests.py | 2 +- openedx/core/lib/courses.py | 30 ++++++++++ openedx/core/lib/tests/test_courses.py | 59 +++++++++++++++++++ 23 files changed, 118 insertions(+), 118 deletions(-) delete mode 100644 common/djangoapps/util/parsing_utils.py create mode 100644 openedx/core/lib/courses.py create mode 100644 openedx/core/lib/tests/test_courses.py diff --git a/cms/djangoapps/contentstore/courseware_index.py b/cms/djangoapps/contentstore/courseware_index.py index 21cb2b2398..99e0052e2e 100644 --- a/cms/djangoapps/contentstore/courseware_index.py +++ b/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 diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 2ed22b6a3c..f3b5362392 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/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.""" diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index eba7ecc678..f59f66ba6b 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/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): """ diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 41a9bccf08..f457e44e38 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/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, diff --git a/cms/djangoapps/models/settings/course_details.py b/cms/djangoapps/models/settings/course_details.py index f09ec7b1a3..b6a6862003 100644 --- a/cms/djangoapps/models/settings/course_details.py +++ b/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 diff --git a/common/djangoapps/util/parsing_utils.py b/common/djangoapps/util/parsing_utils.py deleted file mode 100644 index 41cdbe682f..0000000000 --- a/common/djangoapps/util/parsing_utils.py +++ /dev/null @@ -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 diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py index 9c92d21785..b4308ef2d0 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/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 ( diff --git a/lms/djangoapps/certificates/views/webview.py b/lms/djangoapps/certificates/views/webview.py index fc4a664108..5d47772d81 100644 --- a/lms/djangoapps/certificates/views/webview.py +++ b/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 diff --git a/lms/djangoapps/course_api/serializers.py b/lms/djangoapps/course_api/serializers.py index 02349049bf..24e5d43267 100644 --- a/lms/djangoapps/course_api/serializers.py +++ b/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 diff --git a/lms/djangoapps/course_structure_api/v0/serializers.py b/lms/djangoapps/course_structure_api/v0/serializers.py index bd079544e3..0c9caeba0f 100644 --- a/lms/djangoapps/course_structure_api/v0/serializers.py +++ b/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): diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 709cc2fd42..328396d21b 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/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. diff --git a/lms/djangoapps/courseware/tests/test_courses.py b/lms/djangoapps/courseware/tests/test_courses.py index 935b5d9d2e..bf528801a0 100644 --- a/lms/djangoapps/courseware/tests/test_courses.py +++ b/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 diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index cd2075f92f..4f87ba6b81 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/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, diff --git a/lms/templates/course.html b/lms/templates/course.html index b9251a37b3..a25dcee6bb 100644 --- a/lms/templates/course.html +++ b/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" />
diff --git a/lms/templates/courseware/course_about.html b/lms/templates/courseware/course_about.html index 60b862199a..ae0c41deaf 100644 --- a/lms/templates/courseware/course_about.html +++ b/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" /> diff --git a/lms/templates/shoppingcart/receipt.html b/lms/templates/shoppingcart/receipt.html index 3524ab6599..66209219b6 100644 --- a/lms/templates/shoppingcart/receipt.html +++ b/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"> diff --git a/lms/templates/shoppingcart/registration_code_receipt.html b/lms/templates/shoppingcart/registration_code_receipt.html index b03096cea9..94e21a8de7 100644 --- a/lms/templates/shoppingcart/registration_code_receipt.html +++ b/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'/> diff --git a/lms/templates/shoppingcart/registration_code_redemption.html b/lms/templates/shoppingcart/registration_code_redemption.html index b8b62da538..00ab425932 100644 --- a/lms/templates/shoppingcart/registration_code_redemption.html +++ b/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'/> diff --git a/lms/templates/shoppingcart/shopping_cart.html b/lms/templates/shoppingcart/shopping_cart.html index 01b71824fb..b81c431b9b 100644 --- a/lms/templates/shoppingcart/shopping_cart.html +++ b/lms/templates/shoppingcart/shopping_cart.html @@ -2,12 +2,12 @@ <%block name="review_highlight">class="active" <%! -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 %>
diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py index f0982b7593..6e07db130d 100644 --- a/openedx/core/djangoapps/content/course_overviews/models.py +++ b/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 diff --git a/openedx/core/djangoapps/content/course_overviews/tests.py b/openedx/core/djangoapps/content/course_overviews/tests.py index 77297f5736..b3869be9b7 100644 --- a/openedx/core/djangoapps/content/course_overviews/tests.py +++ b/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 diff --git a/openedx/core/lib/courses.py b/openedx/core/lib/courses.py new file mode 100644 index 0000000000..f420c1f54f --- /dev/null +++ b/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 diff --git a/openedx/core/lib/tests/test_courses.py b/openedx/core/lib/tests/test_courses.py new file mode 100644 index 0000000000..a9aed1ceab --- /dev/null +++ b/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), + )