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_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index ee7a6644cd..53af73fdb0 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -2,63 +2,44 @@ Tests for Studio Course Settings. """ import datetime +import ddt import json import copy import mock from mock import patch import unittest +from django.conf import settings from django.utils.timezone import UTC from django.test.utils import override_settings -from django.conf import settings -from models.settings.course_details import (CourseDetails, CourseSettingsEncoder) -from models.settings.course_grading import CourseGradingModel from contentstore.utils import reverse_course_url, reverse_usage_url -from xmodule.modulestore.tests.factories import CourseFactory +from contentstore.views.component import ADVANCED_COMPONENT_POLICY_KEY +from models.settings.course_grading import CourseGradingModel +from models.settings.course_metadata import CourseMetadata +from models.settings.encoder import CourseSettingsEncoder +from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration +from openedx.core.djangoapps.models.course_details import CourseDetails from student.roles import CourseInstructorRole from student.tests.factories import UserFactory - - -from models.settings.course_metadata import CourseMetadata from xmodule.fields import Date +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.factories import CourseFactory from xmodule.tabs import InvalidTabsException +from util.milestones_helpers import seed_milestone_relationship_types from .utils import CourseTestCase -from xmodule.modulestore.django import modulestore -from contentstore.views.component import ADVANCED_COMPONENT_POLICY_KEY -import ddt -from xmodule.modulestore import ModuleStoreEnum - -from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration -from util.milestones_helpers import seed_milestone_relationship_types def get_url(course_id, handler_name='settings_handler'): return reverse_course_url(handler_name, course_id) -class CourseDetailsTestCase(CourseTestCase): +class CourseSettingsEncoderTest(CourseTestCase): """ - Tests the first course settings page (course dates, overview, etc.). + Tests for CourseSettingsEncoder. """ - def test_virgin_fetch(self): - details = CourseDetails.fetch(self.course.id) - self.assertEqual(details.org, self.course.location.org, "Org not copied into") - self.assertEqual(details.course_id, self.course.location.course, "Course_id not copied into") - self.assertEqual(details.run, self.course.location.name, "Course name not copied into") - self.assertEqual(details.course_image_name, self.course.course_image) - self.assertIsNotNone(details.start_date.tzinfo) - self.assertIsNone(details.end_date, "end date somehow initialized " + str(details.end_date)) - self.assertIsNone(details.enrollment_start, "enrollment_start date somehow initialized " + str(details.enrollment_start)) - self.assertIsNone(details.enrollment_end, "enrollment_end date somehow initialized " + str(details.enrollment_end)) - self.assertIsNone(details.syllabus, "syllabus somehow initialized" + str(details.syllabus)) - self.assertIsNone(details.intro_video, "intro_video somehow initialized" + str(details.intro_video)) - self.assertIsNone(details.effort, "effort somehow initialized" + str(details.effort)) - self.assertIsNone(details.language, "language somehow initialized" + str(details.language)) - self.assertIsNone(details.has_cert_config) - self.assertFalse(details.self_paced) - def test_encoder(self): details = CourseDetails.fetch(self.course.id) jsondetails = json.dumps(details, cls=CourseSettingsEncoder) @@ -87,60 +68,163 @@ class CourseDetailsTestCase(CourseTestCase): self.assertEquals(1, jsondetails['number']) self.assertEqual(jsondetails['string'], 'string') + +@ddt.ddt +class CourseDetailsViewTest(CourseTestCase): + """ + Tests for modifying content on the first course settings page (course dates, overview, etc.). + """ + def setUp(self): + super(CourseDetailsViewTest, self).setUp() + + def alter_field(self, url, details, field, val): + """ + Change the one field to the given value and then invoke the update post to see if it worked. + """ + setattr(details, field, val) + # Need to partially serialize payload b/c the mock doesn't handle it correctly + payload = copy.copy(details.__dict__) + payload['start_date'] = CourseDetailsViewTest.convert_datetime_to_iso(details.start_date) + payload['end_date'] = CourseDetailsViewTest.convert_datetime_to_iso(details.end_date) + payload['enrollment_start'] = CourseDetailsViewTest.convert_datetime_to_iso(details.enrollment_start) + payload['enrollment_end'] = CourseDetailsViewTest.convert_datetime_to_iso(details.enrollment_end) + resp = self.client.ajax_post(url, payload) + self.compare_details_with_encoding(json.loads(resp.content), details.__dict__, field + str(val)) + + @staticmethod + def convert_datetime_to_iso(datetime_obj): + """ + Use the xblock serializer to convert the datetime + """ + return Date().to_json(datetime_obj) + def test_update_and_fetch(self): SelfPacedConfiguration(enabled=True).save() - jsondetails = CourseDetails.fetch(self.course.id) - jsondetails.syllabus = "bar" - # encode - decode to convert date fields and other data which changes form + details = CourseDetails.fetch(self.course.id) + + # resp s/b json from here on + url = get_url(self.course.id) + resp = self.client.get_json(url) + self.compare_details_with_encoding(json.loads(resp.content), details.__dict__, "virgin get") + + utc = UTC() + self.alter_field(url, details, 'start_date', datetime.datetime(2012, 11, 12, 1, 30, tzinfo=utc)) + self.alter_field(url, details, 'start_date', datetime.datetime(2012, 11, 1, 13, 30, tzinfo=utc)) + self.alter_field(url, details, 'end_date', datetime.datetime(2013, 2, 12, 1, 30, tzinfo=utc)) + self.alter_field(url, details, 'enrollment_start', datetime.datetime(2012, 10, 12, 1, 30, tzinfo=utc)) + + self.alter_field(url, details, 'enrollment_end', datetime.datetime(2012, 11, 15, 1, 30, tzinfo=utc)) + self.alter_field(url, details, 'short_description', "Short Description") + self.alter_field(url, details, 'overview', "Overview") + self.alter_field(url, details, 'intro_video', "intro_video") + self.alter_field(url, details, 'effort', "effort") + self.alter_field(url, details, 'course_image_name', "course_image_name") + self.alter_field(url, details, 'language', "en") + self.alter_field(url, details, 'self_paced', "true") + + def compare_details_with_encoding(self, encoded, details, context): + """ + compare all of the fields of the before and after dicts + """ + self.compare_date_fields(details, encoded, context, 'start_date') + self.compare_date_fields(details, encoded, context, 'end_date') + self.compare_date_fields(details, encoded, context, 'enrollment_start') + self.compare_date_fields(details, encoded, context, 'enrollment_end') self.assertEqual( - CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).syllabus, - jsondetails.syllabus, "After set syllabus" - ) - jsondetails.short_description = "Short Description" - self.assertEqual( - CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).short_description, - jsondetails.short_description, "After set short_description" - ) - jsondetails.overview = "Overview" - self.assertEqual( - CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).overview, - jsondetails.overview, "After set overview" - ) - jsondetails.intro_video = "intro_video" - self.assertEqual( - CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).intro_video, - jsondetails.intro_video, "After set intro_video" - ) - jsondetails.effort = "effort" - self.assertEqual( - CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).effort, - jsondetails.effort, "After set effort" - ) - jsondetails.self_paced = True - self.assertEqual( - CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).self_paced, - jsondetails.self_paced - ) - jsondetails.start_date = datetime.datetime(2010, 10, 1, 0, tzinfo=UTC()) - self.assertEqual( - CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).start_date, - jsondetails.start_date - ) - jsondetails.end_date = datetime.datetime(2011, 10, 1, 0, tzinfo=UTC()) - self.assertEqual( - CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).end_date, - jsondetails.end_date - ) - jsondetails.course_image_name = "an_image.jpg" - self.assertEqual( - CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).course_image_name, - jsondetails.course_image_name - ) - jsondetails.language = "hr" - self.assertEqual( - CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).language, - jsondetails.language + details['short_description'], encoded['short_description'], context + " short_description not ==" ) + self.assertEqual(details['overview'], encoded['overview'], context + " overviews not ==") + self.assertEqual(details['intro_video'], encoded.get('intro_video', None), context + " intro_video not ==") + self.assertEqual(details['effort'], encoded['effort'], context + " efforts not ==") + self.assertEqual(details['course_image_name'], encoded['course_image_name'], context + " images not ==") + self.assertEqual(details['language'], encoded['language'], context + " languages not ==") + + def compare_date_fields(self, details, encoded, context, field): + """ + Compare the given date fields between the before and after doing json deserialization + """ + if details[field] is not None: + date = Date() + if field in encoded and encoded[field] is not None: + dt1 = date.from_json(encoded[field]) + dt2 = details[field] + + self.assertEqual(dt1, dt2, msg="{} != {} at {}".format(dt1, dt2, context)) + else: + self.fail(field + " missing from encoded but in details at " + context) + elif field in encoded and encoded[field] is not None: + self.fail(field + " included in encoding but missing from details at " + context) + + @mock.patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True, 'MILESTONES_APP': True}) + def test_pre_requisite_course_list_present(self): + seed_milestone_relationship_types() + settings_details_url = get_url(self.course.id) + response = self.client.get_html(settings_details_url) + self.assertContains(response, "Prerequisite Course") + + @mock.patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True, 'MILESTONES_APP': True}) + def test_pre_requisite_course_update_and_fetch(self): + seed_milestone_relationship_types() + url = get_url(self.course.id) + resp = self.client.get_json(url) + course_detail_json = json.loads(resp.content) + # assert pre_requisite_courses is initialized + self.assertEqual([], course_detail_json['pre_requisite_courses']) + + # update pre requisite courses with a new course keys + pre_requisite_course = CourseFactory.create(org='edX', course='900', run='test_run') + pre_requisite_course2 = CourseFactory.create(org='edX', course='902', run='test_run') + pre_requisite_course_keys = [unicode(pre_requisite_course.id), unicode(pre_requisite_course2.id)] + course_detail_json['pre_requisite_courses'] = pre_requisite_course_keys + self.client.ajax_post(url, course_detail_json) + + # fetch updated course to assert pre_requisite_courses has new values + resp = self.client.get_json(url) + course_detail_json = json.loads(resp.content) + self.assertEqual(pre_requisite_course_keys, course_detail_json['pre_requisite_courses']) + + # remove pre requisite course + course_detail_json['pre_requisite_courses'] = [] + self.client.ajax_post(url, course_detail_json) + resp = self.client.get_json(url) + course_detail_json = json.loads(resp.content) + self.assertEqual([], course_detail_json['pre_requisite_courses']) + + @mock.patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True, 'MILESTONES_APP': True}) + def test_invalid_pre_requisite_course(self): + seed_milestone_relationship_types() + url = get_url(self.course.id) + resp = self.client.get_json(url) + course_detail_json = json.loads(resp.content) + + # update pre requisite courses one valid and one invalid key + pre_requisite_course = CourseFactory.create(org='edX', course='900', run='test_run') + pre_requisite_course_keys = [unicode(pre_requisite_course.id), 'invalid_key'] + course_detail_json['pre_requisite_courses'] = pre_requisite_course_keys + response = self.client.ajax_post(url, course_detail_json) + self.assertEqual(400, response.status_code) + + @ddt.data( + (False, False, False), + (True, False, True), + (False, True, False), + (True, True, True), + ) + def test_visibility_of_entrance_exam_section(self, feature_flags): + """ + Tests entrance exam section is available if ENTRANCE_EXAMS feature is enabled no matter any other + feature is enabled or disabled i.e ENABLE_MKTG_SITE. + """ + with patch.dict("django.conf.settings.FEATURES", { + 'ENTRANCE_EXAMS': feature_flags[0], + 'ENABLE_MKTG_SITE': feature_flags[1] + }): + course_details_url = get_url(self.course.id) + resp = self.client.get_html(course_details_url) + self.assertEqual( + feature_flags[2], + '

' in resp.content + ) @override_settings(MKTG_URLS={'ROOT': 'dummy-root'}) def test_marketing_site_fetch(self): @@ -296,175 +380,6 @@ class CourseDetailsTestCase(CourseTestCase): self.assertContains(response, "Course Introduction Video") self.assertContains(response, "Requirements") - def test_toggle_pacing_during_course_run(self): - SelfPacedConfiguration(enabled=True).save() - self.course.start = datetime.datetime.now() - modulestore().update_item(self.course, self.user.id) - - details = CourseDetails.fetch(self.course.id) - updated_details = CourseDetails.update_from_json( - self.course.id, - dict(details.__dict__, self_paced=True), - self.user - ) - self.assertFalse(updated_details.self_paced) - - -@ddt.ddt -class CourseDetailsViewTest(CourseTestCase): - """ - Tests for modifying content on the first course settings page (course dates, overview, etc.). - """ - def setUp(self): - super(CourseDetailsViewTest, self).setUp() - - def alter_field(self, url, details, field, val): - """ - Change the one field to the given value and then invoke the update post to see if it worked. - """ - setattr(details, field, val) - # Need to partially serialize payload b/c the mock doesn't handle it correctly - payload = copy.copy(details.__dict__) - payload['start_date'] = CourseDetailsViewTest.convert_datetime_to_iso(details.start_date) - payload['end_date'] = CourseDetailsViewTest.convert_datetime_to_iso(details.end_date) - payload['enrollment_start'] = CourseDetailsViewTest.convert_datetime_to_iso(details.enrollment_start) - payload['enrollment_end'] = CourseDetailsViewTest.convert_datetime_to_iso(details.enrollment_end) - resp = self.client.ajax_post(url, payload) - self.compare_details_with_encoding(json.loads(resp.content), details.__dict__, field + str(val)) - - @staticmethod - def convert_datetime_to_iso(datetime_obj): - """ - Use the xblock serializer to convert the datetime - """ - return Date().to_json(datetime_obj) - - def test_update_and_fetch(self): - SelfPacedConfiguration(enabled=True).save() - details = CourseDetails.fetch(self.course.id) - - # resp s/b json from here on - url = get_url(self.course.id) - resp = self.client.get_json(url) - self.compare_details_with_encoding(json.loads(resp.content), details.__dict__, "virgin get") - - utc = UTC() - self.alter_field(url, details, 'start_date', datetime.datetime(2012, 11, 12, 1, 30, tzinfo=utc)) - self.alter_field(url, details, 'start_date', datetime.datetime(2012, 11, 1, 13, 30, tzinfo=utc)) - self.alter_field(url, details, 'end_date', datetime.datetime(2013, 2, 12, 1, 30, tzinfo=utc)) - self.alter_field(url, details, 'enrollment_start', datetime.datetime(2012, 10, 12, 1, 30, tzinfo=utc)) - - self.alter_field(url, details, 'enrollment_end', datetime.datetime(2012, 11, 15, 1, 30, tzinfo=utc)) - self.alter_field(url, details, 'short_description', "Short Description") - self.alter_field(url, details, 'overview', "Overview") - self.alter_field(url, details, 'intro_video', "intro_video") - self.alter_field(url, details, 'effort', "effort") - self.alter_field(url, details, 'course_image_name', "course_image_name") - self.alter_field(url, details, 'language', "en") - self.alter_field(url, details, 'self_paced', "true") - - def compare_details_with_encoding(self, encoded, details, context): - """ - compare all of the fields of the before and after dicts - """ - self.compare_date_fields(details, encoded, context, 'start_date') - self.compare_date_fields(details, encoded, context, 'end_date') - self.compare_date_fields(details, encoded, context, 'enrollment_start') - self.compare_date_fields(details, encoded, context, 'enrollment_end') - self.assertEqual(details['short_description'], encoded['short_description'], context + " short_description not ==") - self.assertEqual(details['overview'], encoded['overview'], context + " overviews not ==") - self.assertEqual(details['intro_video'], encoded.get('intro_video', None), context + " intro_video not ==") - self.assertEqual(details['effort'], encoded['effort'], context + " efforts not ==") - self.assertEqual(details['course_image_name'], encoded['course_image_name'], context + " images not ==") - self.assertEqual(details['language'], encoded['language'], context + " languages not ==") - - def compare_date_fields(self, details, encoded, context, field): - """ - Compare the given date fields between the before and after doing json deserialization - """ - if details[field] is not None: - date = Date() - if field in encoded and encoded[field] is not None: - dt1 = date.from_json(encoded[field]) - dt2 = details[field] - - self.assertEqual(dt1, dt2, msg="{} != {} at {}".format(dt1, dt2, context)) - else: - self.fail(field + " missing from encoded but in details at " + context) - elif field in encoded and encoded[field] is not None: - self.fail(field + " included in encoding but missing from details at " + context) - - @mock.patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True, 'MILESTONES_APP': True}) - def test_pre_requisite_course_list_present(self): - seed_milestone_relationship_types() - settings_details_url = get_url(self.course.id) - response = self.client.get_html(settings_details_url) - self.assertContains(response, "Prerequisite Course") - - @mock.patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True, 'MILESTONES_APP': True}) - def test_pre_requisite_course_update_and_fetch(self): - seed_milestone_relationship_types() - url = get_url(self.course.id) - resp = self.client.get_json(url) - course_detail_json = json.loads(resp.content) - # assert pre_requisite_courses is initialized - self.assertEqual([], course_detail_json['pre_requisite_courses']) - - # update pre requisite courses with a new course keys - pre_requisite_course = CourseFactory.create(org='edX', course='900', run='test_run') - pre_requisite_course2 = CourseFactory.create(org='edX', course='902', run='test_run') - pre_requisite_course_keys = [unicode(pre_requisite_course.id), unicode(pre_requisite_course2.id)] - course_detail_json['pre_requisite_courses'] = pre_requisite_course_keys - self.client.ajax_post(url, course_detail_json) - - # fetch updated course to assert pre_requisite_courses has new values - resp = self.client.get_json(url) - course_detail_json = json.loads(resp.content) - self.assertEqual(pre_requisite_course_keys, course_detail_json['pre_requisite_courses']) - - # remove pre requisite course - course_detail_json['pre_requisite_courses'] = [] - self.client.ajax_post(url, course_detail_json) - resp = self.client.get_json(url) - course_detail_json = json.loads(resp.content) - self.assertEqual([], course_detail_json['pre_requisite_courses']) - - @mock.patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True, 'MILESTONES_APP': True}) - def test_invalid_pre_requisite_course(self): - seed_milestone_relationship_types() - url = get_url(self.course.id) - resp = self.client.get_json(url) - course_detail_json = json.loads(resp.content) - - # update pre requisite courses one valid and one invalid key - pre_requisite_course = CourseFactory.create(org='edX', course='900', run='test_run') - pre_requisite_course_keys = [unicode(pre_requisite_course.id), 'invalid_key'] - course_detail_json['pre_requisite_courses'] = pre_requisite_course_keys - response = self.client.ajax_post(url, course_detail_json) - self.assertEqual(400, response.status_code) - - @ddt.data( - (False, False, False), - (True, False, True), - (False, True, False), - (True, True, True), - ) - def test_visibility_of_entrance_exam_section(self, feature_flags): - """ - Tests entrance exam section is available if ENTRANCE_EXAMS feature is enabled no matter any other - feature is enabled or disabled i.e ENABLE_MKTG_SITE. - """ - with patch.dict("django.conf.settings.FEATURES", { - 'ENTRANCE_EXAMS': feature_flags[0], - 'ENABLE_MKTG_SITE': feature_flags[1] - }): - course_details_url = get_url(self.course.id) - resp = self.client.get_html(course_details_url) - self.assertEqual( - feature_flags[2], - '

' in resp.content - ) - @ddt.ddt class CourseGradingTest(CourseTestCase): @@ -1067,40 +982,6 @@ class CourseMetadataEditingTest(CourseTestCase): tab_list.append(self.notes_tab) self.assertEqual(tab_list, course.tabs) - @override_settings(FEATURES={'CERTIFICATES_HTML_VIEW': True}) - def test_web_view_certifcate_configuration_settings(self): - """ - Test that has_cert_config is updated based on cert_html_view_enabled setting. - """ - test_model = CourseMetadata.update_from_json( - self.course, - { - "cert_html_view_enabled": {"value": "true"} - }, - user=self.user - ) - self.assertIn('cert_html_view_enabled', test_model) - url = get_url(self.course.id) - response = self.client.get_json(url) - course_detail_json = json.loads(response.content) - self.assertFalse(course_detail_json['has_cert_config']) - - # Now add a certificate configuration - certificates = [ - { - 'id': 1, - 'name': 'Certificate Config Name', - 'course_title': 'Title override', - 'signatories': [], - 'is_active': True - } - ] - self.course.certificates = {'certificates': certificates} - modulestore().update_item(self.course, self.user.id) - response = self.client.get_json(url) - course_detail_json = json.loads(response.content) - self.assertTrue(course_detail_json['has_cert_config']) - class CourseGraderUpdatesTest(CourseTestCase): """ diff --git a/cms/djangoapps/contentstore/tests/test_courseware_index.py b/cms/djangoapps/contentstore/tests/test_courseware_index.py index 1b290a29d6..de4b6f6845 100644 --- a/cms/djangoapps/contentstore/tests/test_courseware_index.py +++ b/cms/djangoapps/contentstore/tests/test_courseware_index.py @@ -15,6 +15,7 @@ from unittest import skip from django.conf import settings from course_modes.models import CourseMode +from openedx.core.djangoapps.models.course_details import CourseDetails from xmodule.library_tools import normalize_key_for_search from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import SignalHandler, modulestore @@ -192,26 +193,6 @@ class MixedWithOptionsTestCase(MixedSplitTestCase): with store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): store.update_item(item, ModuleStoreEnum.UserID.test) - def update_about_item(self, store, about_key, data): - """ - Update the about item with the new data blob. If data is None, then - delete the about item. - """ - temploc = self.course.id.make_usage_key('about', about_key) - if data is None: - try: - self.delete_item(store, temploc) - # Ignore an attempt to delete an item that doesn't exist - except ValueError: - pass - else: - try: - about_item = store.get_item(temploc) - except ItemNotFoundError: - about_item = store.create_xblock(self.course.runtime, self.course.id, 'about', about_key) - about_item.data = data - store.update_item(about_item, ModuleStoreEnum.UserID.test, allow_not_found=True) - @ddt.ddt class TestCoursewareSearchIndexer(MixedWithOptionsTestCase): @@ -487,7 +468,9 @@ class TestCoursewareSearchIndexer(MixedWithOptionsTestCase): def _test_course_about_store_index(self, store): """ Test that informational properties in the about store end up in the course_info index """ short_description = "Not just anybody" - self.update_about_item(store, "short_description", short_description) + CourseDetails.update_about_item( + self.course, "short_description", short_description, ModuleStoreEnum.UserID.test, store + ) self.reindex_course(store) response = self.searcher.search( doc_type=CourseAboutSearchIndexer.DISCOVERY_DOCUMENT_TYPE, 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..711b3947f8 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -3,7 +3,6 @@ Common utility functions useful throughout the contentstore """ import logging -from opaque_keys import InvalidKeyError import re from datetime import datetime from pytz import UTC @@ -14,7 +13,6 @@ from django.utils.translation import ugettext as _ from django_comment_common.models import assign_default_role from django_comment_common.utils import seed_permissions_roles -from xmodule.contentstore.content import StaticContent from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError @@ -158,16 +156,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): """ @@ -314,25 +302,6 @@ def reverse_usage_url(handler_name, usage_key, kwargs=None): return reverse_url(handler_name, 'usage_key_string', usage_key, kwargs) -def has_active_web_certificate(course): - """ - Returns True if given course has active web certificate configuration. - If given course has no active web certificate configuration returns False. - Returns None If `CERTIFICATES_HTML_VIEW` is not enabled of course has not enabled - `cert_html_view_enabled` settings. - """ - cert_config = None - if settings.FEATURES.get('CERTIFICATES_HTML_VIEW', False) and course.cert_html_view_enabled: - cert_config = False - certificates = getattr(course, 'certificates', {}) - configurations = certificates.get('certificates', []) - for config in configurations: - if config.get('is_active'): - cert_config = True - break - return cert_config - - def get_user_partition_info(xblock, schemes=None, course=None): """ Retrieve user partition information for an XBlock for display in editors. diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 41a9bccf08..e40fbe5ea7 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -58,16 +58,18 @@ from course_action_state.models import CourseRerunState, CourseRerunUIStateManag from course_creators.views import get_course_creator_status, add_user_with_status_unrequested from edxmako.shortcuts import render_to_response from microsite_configuration import microsite -from models.settings.course_details import CourseDetails, CourseSettingsEncoder from models.settings.course_grading import CourseGradingModel from models.settings.course_metadata import CourseMetadata +from models.settings.encoder import CourseSettingsEncoder from openedx.core.djangoapps.content.course_structures.api.v0 import api, errors from openedx.core.djangoapps.credit.api import is_credit_course, get_credit_requirements from openedx.core.djangoapps.credit.tasks import update_credit_course_requirements +from openedx.core.djangoapps.models.course_details import CourseDetails 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 +941,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/encoder.py b/cms/djangoapps/models/settings/encoder.py new file mode 100644 index 0000000000..36b6e1fb44 --- /dev/null +++ b/cms/djangoapps/models/settings/encoder.py @@ -0,0 +1,28 @@ +""" +CourseSettingsEncoder +""" +import datetime +import json +from json.encoder import JSONEncoder + +from opaque_keys.edx.locations import Location +from openedx.core.djangoapps.models.course_details import CourseDetails +from xmodule.fields import Date + +from .course_grading import CourseGradingModel + + +class CourseSettingsEncoder(json.JSONEncoder): + """ + Serialize CourseDetails, CourseGradingModel, datetime, and old + Locations + """ + def default(self, obj): # pylint: disable=method-hidden + if isinstance(obj, (CourseDetails, CourseGradingModel)): + return obj.__dict__ + elif isinstance(obj, Location): + return obj.dict() + elif isinstance(obj, datetime.datetime): + return Date().to_json(obj) + else: + return JSONEncoder.default(self, obj) diff --git a/cms/static/js/spec/views/settings/main_spec.js b/cms/static/js/spec/views/settings/main_spec.js index e8284620f8..f39d669c5d 100644 --- a/cms/static/js/spec/views/settings/main_spec.js +++ b/cms/static/js/spec/views/settings/main_spec.js @@ -31,8 +31,7 @@ define([ entrance_exam_enabled : '', entrance_exam_minimum_score_pct: '50', license: null, - language: '', - has_cert_config: false + language: '' }, mockSettingsPage = readFixtures('mock/mock-settings-page.underscore'); diff --git a/cms/templates/settings_graders.html b/cms/templates/settings_graders.html index 580e831c14..acb5f35e7f 100644 --- a/cms/templates/settings_graders.html +++ b/cms/templates/settings_graders.html @@ -8,8 +8,8 @@ import json from contentstore import utils from django.utils.translation import ugettext as _ + from models.settings.encoder import CourseSettingsEncoder from openedx.core.lib.js_utils import escape_json_dumps - from models.settings.course_details import CourseSettingsEncoder %> <%block name="header_extras"> 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/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index 1519cf50d7..48c1cc7d59 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -660,7 +660,8 @@ def check_mongo_calls(num_finds=0, num_sends=None): # This dict represents the attribute keys for a course's 'about' info. # Note: The 'video' attribute is intentionally excluded as it must be # handled separately; its value maps to an alternate key name. -# Reference : cms/djangoapps/models/settings/course_details.py +# Reference : from openedx.core.djangoapps.models.course_details.py + ABOUT_ATTRIBUTES = { 'effort': "Testing effort", 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..f891365853 100644 --- a/lms/djangoapps/course_api/serializers.py +++ b/lms/djangoapps/course_api/serializers.py @@ -9,7 +9,9 @@ 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 openedx.core.djangoapps.models.course_details import CourseDetails from xmodule.course_module import DEFAULT_START_DATE @@ -35,6 +37,10 @@ class _CourseApiMediaCollectionSerializer(serializers.Serializer): # pylint: di Nested serializer to represent a collection of media objects """ course_image = _MediaSerializer(source='*', uri_parser=course_image_url) + course_video = _MediaSerializer( + source='*', + uri_parser=lambda course: CourseDetails.fetch_video_url(course.id), + ) class CourseSerializer(serializers.Serializer): # pylint: disable=abstract-method @@ -46,14 +52,19 @@ class CourseSerializer(serializers.Serializer): # pylint: disable=abstract-meth name = serializers.CharField(source='display_name_with_default') number = serializers.CharField(source='display_number_with_default') org = serializers.CharField(source='display_org_with_default') - description = serializers.SerializerMethodField() + short_description = serializers.SerializerMethodField() + effort = serializers.SerializerMethodField() + media = _CourseApiMediaCollectionSerializer(source='*') + start = serializers.DateTimeField() start_type = serializers.SerializerMethodField() start_display = serializers.SerializerMethodField() end = serializers.DateTimeField() + enrollment_start = serializers.DateTimeField() enrollment_end = serializers.DateTimeField() + blocks_url = serializers.SerializerMethodField() def get_start_type(self, course): @@ -78,9 +89,9 @@ class CourseSerializer(serializers.Serializer): # pylint: disable=abstract-meth else: return None - def get_description(self, course): + def get_short_description(self, course): """ - Get the representation for SerializerMethodField `description` + Get the representation for SerializerMethodField `short_description` """ return get_course_about_section(self.context['request'], course, 'short_description').strip() @@ -93,3 +104,9 @@ class CourseSerializer(serializers.Serializer): # pylint: disable=abstract-meth urllib.urlencode({'course_id': course.id}), ]) return self.context['request'].build_absolute_uri(base_url) + + def get_effort(self, course): + """ + Get the representation for SerializerMethodField `effort` + """ + return CourseDetails.fetch_effort(course.id) diff --git a/lms/djangoapps/course_api/tests/test_api.py b/lms/djangoapps/course_api/tests/test_api.py index d5b0413367..bf77026518 100644 --- a/lms/djangoapps/course_api/tests/test_api.py +++ b/lms/djangoapps/course_api/tests/test_api.py @@ -19,44 +19,18 @@ class CourseApiTestMixin(CourseApiFactoryMixin): """ Establish basic functionality for Course API tests """ - - maxDiff = 5000 # long enough to show mismatched dicts - - expected_course_data = { - 'course_id': u'edX/toy/2012_Fall', - 'name': u'Toy Course', - 'number': u'toy', - 'org': u'edX', - 'description': u'A course about toys.', - 'media': { - 'course_image': { - 'uri': u'/c4x/edX/toy/asset/just_a_test.jpg', - } - }, - 'start': u'2015-07-17T12:00:00Z', - 'start_type': u'timestamp', - 'start_display': u'July 17, 2015', - 'end': u'2015-09-19T18:00:00Z', - 'enrollment_start': u'2015-06-15T00:00:00Z', - 'enrollment_end': u'2015-07-15T00:00:00Z', - 'blocks_url': '/api/courses/v1/blocks/?course_id=edX%2Ftoy%2F2012_Fall', - } - - def verify_course(self, course, course_id=None): - """ - Ensure that the returned course is the course we just created - """ - - if course_id is None: - course_id = self.expected_course_data['course_id'] - self.assertIsInstance(course, CourseDescriptor) - self.assertEqual(course_id, str(course.id)) - @classmethod def setUpClass(cls): super(CourseApiTestMixin, cls).setUpClass() cls.request_factory = APIRequestFactory() + def verify_course(self, course, course_id=u'edX/toy/2012_Fall'): + """ + Ensure that the returned course is the course we just created + """ + self.assertIsInstance(course, CourseDescriptor) + self.assertEqual(course_id, str(course.id)) + class CourseDetailTestMixin(CourseApiTestMixin): """ diff --git a/lms/djangoapps/course_api/tests/test_serializers.py b/lms/djangoapps/course_api/tests/test_serializers.py index 936dc42b8a..093e8131d9 100644 --- a/lms/djangoapps/course_api/tests/test_serializers.py +++ b/lms/djangoapps/course_api/tests/test_serializers.py @@ -4,6 +4,7 @@ Test data created by CourseSerializer from datetime import datetime +from openedx.core.djangoapps.models.course_details import CourseDetails from rest_framework.test import APIRequestFactory from rest_framework.request import Request @@ -18,6 +19,7 @@ class TestCourseSerializerFields(CourseApiFactoryMixin, ModuleStoreTestCase): """ Test variations of start_date field responses """ + maxDiff = 5000 # long enough to show mismatched dicts, in case of error def setUp(self): super(TestCourseSerializerFields, self).setUp() @@ -35,6 +37,35 @@ class TestCourseSerializerFields(CourseApiFactoryMixin, ModuleStoreTestCase): request.user = user return request + def test_basic(self): + expected_data = { + 'course_id': u'edX/toy/2012_Fall', + 'name': u'Toy Course', + 'number': u'toy', + 'org': u'edX', + 'short_description': u'A course about toys.', + 'media': { + 'course_image': { + 'uri': u'/c4x/edX/toy/asset/just_a_test.jpg', + }, + 'course_video': { + 'uri': u'http://www.youtube.com/watch?v=test_youtube_id', + } + }, + 'start': u'2015-07-17T12:00:00Z', + 'start_type': u'timestamp', + 'start_display': u'July 17, 2015', + 'end': u'2015-09-19T18:00:00', + 'enrollment_start': u'2015-06-15T00:00:00', + 'enrollment_end': u'2015-07-15T00:00:00', + 'blocks_url': u'http://testserver/api/courses/v1/blocks/?course_id=edX%2Ftoy%2F2012_Fall', + 'effort': u'6 hours', + } + course = self.create_course() + CourseDetails.update_about_video(course, 'test_youtube_id', self.staff_user.id) # pylint: disable=no-member + result = CourseSerializer(course, context={'request': self._get_request()}).data + self.assertDictEqual(result, expected_data) + def test_advertised_start(self): course = self.create_course( course=u'custom', @@ -52,16 +83,3 @@ class TestCourseSerializerFields(CourseApiFactoryMixin, ModuleStoreTestCase): self.assertEqual(result['course_id'], u'edX/custom/2012_Fall') self.assertEqual(result['start_type'], u'empty') self.assertIsNone(result['start_display']) - - def test_description(self): - course = self.create_course() - result = CourseSerializer(course, context={'request': self._get_request()}).data - self.assertEqual(result['description'], u'A course about toys.') - - def test_blocks_url(self): - course = self.create_course() - result = CourseSerializer(course, context={'request': self._get_request()}).data - self.assertEqual( - result['blocks_url'], - u'http://testserver/api/courses/v1/blocks/?course_id=edX%2Ftoy%2F2012_Fall' - ) 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/djangoapps/models/__init__.py b/openedx/core/djangoapps/models/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cms/djangoapps/models/settings/course_details.py b/openedx/core/djangoapps/models/course_details.py similarity index 71% rename from cms/djangoapps/models/settings/course_details.py rename to openedx/core/djangoapps/models/course_details.py index f09ec7b1a3..c2fce5cafe 100644 --- a/cms/djangoapps/models/settings/course_details.py +++ b/openedx/core/djangoapps/models/course_details.py @@ -1,19 +1,18 @@ +""" +CourseDetails +""" import re import logging -import datetime -import json -from json.encoder import JSONEncoder 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 models.settings import course_grading -from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration from xmodule.fields import Date +from xmodule.modulestore.exceptions import ItemNotFoundError +from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration +from openedx.core.lib.courses import course_image_url from xmodule.modulestore.django import modulestore + # This list represents the attribute keys for a course's 'about' info. # Note: The 'video' attribute is intentionally excluded as it must be # handled separately; its value maps to an alternate key name. @@ -29,8 +28,12 @@ ABOUT_ATTRIBUTES = [ class CourseDetails(object): + """ + An interface for extracting course information from the modulestore. + """ def __init__(self, org, course_id, run): - # still need these for now b/c the client's screen shows these 3 fields + # still need these for now b/c the client's screen shows these 3 + # fields self.org = org self.course_id = course_id self.run = run @@ -43,7 +46,7 @@ class CourseDetails(object): self.short_description = "" self.overview = "" # html to render as the overview self.intro_video = None # a video pointer - self.effort = None # int hours/week + self.effort = None # hours/week self.license = "all-rights-reserved" # default course license is all rights reserved self.course_image_name = "" self.course_image_asset_path = "" # URL of the course image @@ -54,7 +57,6 @@ class CourseDetails(object): 'ENTRANCE_EXAM_MIN_SCORE_PCT', '50' ) # minimum passing score for entrance exam content module/tree, - self.has_cert_config = None # course has active certificate configuration self.self_paced = None @classmethod @@ -72,7 +74,8 @@ class CourseDetails(object): @classmethod def fetch(cls, course_key): """ - Fetch the course details for the given course from persistence and return a CourseDetails model. + Fetch the course details for the given course from persistence + and return a CourseDetails model. """ descriptor = modulestore().get_course(course_key) course_details = cls(course_key.org, course_key.course, course_key.run) @@ -85,33 +88,56 @@ class CourseDetails(object): course_details.course_image_name = descriptor.course_image course_details.course_image_asset_path = course_image_url(descriptor) course_details.language = descriptor.language + course_details.self_paced = descriptor.self_paced + # Default course license is "All Rights Reserved" course_details.license = getattr(descriptor, "license", "all-rights-reserved") - course_details.has_cert_config = has_active_web_certificate(descriptor) - course_details.self_paced = descriptor.self_paced + + course_details.intro_video = cls.fetch_youtube_video_id(course_key) for attribute in ABOUT_ATTRIBUTES: value = cls._fetch_about_attribute(course_key, attribute) if value is not None: setattr(course_details, attribute, value) - raw_video = cls._fetch_about_attribute(course_key, 'video') - if raw_video: - course_details.intro_video = CourseDetails.parse_video_tag(raw_video) - return course_details @classmethod - def update_about_item(cls, course_key, about_key, data, course, user): + def fetch_youtube_video_id(cls, course_key): """ - Update the about item with the new data blob. If data is None, then - delete the about item. + Returns the course about video ID. """ - temploc = course_key.make_usage_key('about', about_key) - store = modulestore() + raw_video = cls._fetch_about_attribute(course_key, 'video') + if raw_video: + return cls.parse_video_tag(raw_video) + + @classmethod + def fetch_video_url(cls, course_key): + """ + Returns the course about video URL. + """ + video_id = cls.fetch_youtube_video_id(course_key) + if video_id: + return "http://www.youtube.com/watch?v={0}".format(video_id) + + @classmethod + def fetch_effort(cls, course_key): + """ + Returns the hours per week of effort for the course. + """ + return cls._fetch_about_attribute(course_key, 'effort') + + @classmethod + def update_about_item(cls, course, about_key, data, user_id, store=None): + """ + Update the about item with the new data blob. If data is None, + then delete the about item. + """ + temploc = course.id.make_usage_key('about', about_key) + store = store or modulestore() if data is None: try: - store.delete_item(temploc, user.id) + store.delete_item(temploc, user_id) # Ignore an attempt to delete an item that doesn't exist except ValueError: pass @@ -121,10 +147,18 @@ class CourseDetails(object): except ItemNotFoundError: about_item = store.create_xblock(course.runtime, course.id, 'about', about_key) about_item.data = data - store.update_item(about_item, user.id, allow_not_found=True) + store.update_item(about_item, user_id, allow_not_found=True) @classmethod - def update_from_json(cls, course_key, jsondict, user): + def update_about_video(cls, course, video_id, user_id): + """ + Updates the Course's about video to the given video ID. + """ + recomposed_video_tag = CourseDetails.recompose_video_tag(video_id) + cls.update_about_item(course, 'video', recomposed_video_tag, user_id) + + @classmethod + def update_from_json(cls, course_key, jsondict, user): # pylint: disable=too-many-statements """ Decode the json into CourseDetails and save any changed attrs to the db """ @@ -133,10 +167,11 @@ class CourseDetails(object): dirty = False - # In the descriptor's setter, the date is converted to JSON using Date's to_json method. - # Calling to_json on something that is already JSON doesn't work. Since reaching directly - # into the model is nasty, convert the JSON Date to a Python date, which is what the - # setter expects as input. + # In the descriptor's setter, the date is converted to JSON + # using Date's to_json method. Calling to_json on something that + # is already JSON doesn't work. Since reaching directly into the + # model is nasty, convert the JSON Date to a Python date, which + # is what the setter expects as input. date = Date() if 'start_date' in jsondict: @@ -201,25 +236,28 @@ class CourseDetails(object): if dirty: module_store.update_item(descriptor, user.id) - # NOTE: below auto writes to the db w/o verifying that any of the fields actually changed - # to make faster, could compare against db or could have client send over a list of which fields changed. + # NOTE: below auto writes to the db w/o verifying that any of + # the fields actually changed to make faster, could compare + # against db or could have client send over a list of which + # fields changed. for attribute in ABOUT_ATTRIBUTES: if attribute in jsondict: - cls.update_about_item(course_key, attribute, jsondict[attribute], descriptor, user) + cls.update_about_item(descriptor, attribute, jsondict[attribute], user.id) - recomposed_video_tag = CourseDetails.recompose_video_tag(jsondict['intro_video']) - cls.update_about_item(course_key, 'video', recomposed_video_tag, descriptor, user) + cls.update_about_video(descriptor, jsondict['intro_video'], user.id) - # Could just return jsondict w/o doing any db reads, but I put the reads in as a means to confirm - # it persisted correctly + # Could just return jsondict w/o doing any db reads, but I put + # the reads in as a means to confirm it persisted correctly return CourseDetails.fetch(course_key) @staticmethod def parse_video_tag(raw_video): """ - Because the client really only wants the author to specify the youtube key, that's all we send to and get from the client. - The problem is that the db stores the html markup as well (which, of course, makes any sitewide changes to how we do videos - next to impossible.) + Because the client really only wants the author to specify the + youtube key, that's all we send to and get from the client. The + problem is that the db stores the html markup as well (which, of + course, makes any site-wide changes to how we do videos next to + impossible.) """ if not raw_video: return None @@ -236,26 +274,17 @@ class CourseDetails(object): @staticmethod def recompose_video_tag(video_key): - # TODO should this use a mako template? Of course, my hope is that this is a short-term workaround for the db not storing - # the right thing + """ + Returns HTML string to embed the video in an iFrame. + """ + # TODO should this use a mako template? Of course, my hope is + # that this is a short-term workaround for the db not storing + # the right thing result = None if video_key: - result = '' + result = ( + '' + ) return result - - -# TODO move to a more general util? -class CourseSettingsEncoder(json.JSONEncoder): - """ - Serialize CourseDetails, CourseGradingModel, datetime, and old Locations - """ - def default(self, obj): - if isinstance(obj, (CourseDetails, course_grading.CourseGradingModel)): - return obj.__dict__ - elif isinstance(obj, Location): - return obj.dict() - elif isinstance(obj, datetime.datetime): - return Date().to_json(obj) - else: - return JSONEncoder.default(self, obj) diff --git a/openedx/core/djangoapps/models/tests/test_course_details.py b/openedx/core/djangoapps/models/tests/test_course_details.py new file mode 100644 index 0000000000..e5e490f4ec --- /dev/null +++ b/openedx/core/djangoapps/models/tests/test_course_details.py @@ -0,0 +1,126 @@ +""" +Tests for CourseDetails +""" + +import datetime +from django.utils.timezone import UTC + +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + +from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration +from openedx.core.djangoapps.models.course_details import CourseDetails + + +class CourseDetailsTestCase(ModuleStoreTestCase): + """ + Tests the first course settings page (course dates, overview, etc.). + """ + def setUp(self): + super(CourseDetailsTestCase, self).setUp() + self.course = CourseFactory.create() + + def test_virgin_fetch(self): + details = CourseDetails.fetch(self.course.id) + self.assertEqual(details.org, self.course.location.org, "Org not copied into") + self.assertEqual(details.course_id, self.course.location.course, "Course_id not copied into") + self.assertEqual(details.run, self.course.location.name, "Course name not copied into") + self.assertEqual(details.course_image_name, self.course.course_image) + self.assertIsNotNone(details.start_date.tzinfo) + self.assertIsNone(details.end_date, "end date somehow initialized " + str(details.end_date)) + self.assertIsNone( + details.enrollment_start, "enrollment_start date somehow initialized " + str(details.enrollment_start) + ) + self.assertIsNone( + details.enrollment_end, "enrollment_end date somehow initialized " + str(details.enrollment_end) + ) + self.assertIsNone(details.syllabus, "syllabus somehow initialized" + str(details.syllabus)) + self.assertIsNone(details.intro_video, "intro_video somehow initialized" + str(details.intro_video)) + self.assertIsNone(details.effort, "effort somehow initialized" + str(details.effort)) + self.assertIsNone(details.language, "language somehow initialized" + str(details.language)) + self.assertFalse(details.self_paced) + + def test_update_and_fetch(self): + SelfPacedConfiguration(enabled=True).save() + jsondetails = CourseDetails.fetch(self.course.id) + jsondetails.syllabus = "bar" + # encode - decode to convert date fields and other data which changes form + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.course.id): + self.assertEqual( + CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).syllabus, + jsondetails.syllabus, "After set syllabus" + ) + jsondetails.short_description = "Short Description" + self.assertEqual( + CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).short_description, + jsondetails.short_description, "After set short_description" + ) + jsondetails.overview = "Overview" + self.assertEqual( + CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).overview, + jsondetails.overview, "After set overview" + ) + jsondetails.intro_video = "intro_video" + self.assertEqual( + CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).intro_video, + jsondetails.intro_video, "After set intro_video" + ) + jsondetails.effort = "effort" + self.assertEqual( + CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).effort, + jsondetails.effort, "After set effort" + ) + jsondetails.self_paced = True + self.assertEqual( + CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).self_paced, + jsondetails.self_paced + ) + jsondetails.start_date = datetime.datetime(2010, 10, 1, 0, tzinfo=UTC()) + self.assertEqual( + CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).start_date, + jsondetails.start_date + ) + jsondetails.end_date = datetime.datetime(2011, 10, 1, 0, tzinfo=UTC()) + self.assertEqual( + CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).end_date, + jsondetails.end_date + ) + jsondetails.course_image_name = "an_image.jpg" + self.assertEqual( + CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).course_image_name, + jsondetails.course_image_name + ) + jsondetails.language = "hr" + self.assertEqual( + CourseDetails.update_from_json(self.course.id, jsondetails.__dict__, self.user).language, + jsondetails.language + ) + + def test_toggle_pacing_during_course_run(self): + SelfPacedConfiguration(enabled=True).save() + self.course.start = datetime.datetime.now() + self.store.update_item(self.course, self.user.id) + + details = CourseDetails.fetch(self.course.id) + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.course.id): + updated_details = CourseDetails.update_from_json( + self.course.id, + dict(details.__dict__, self_paced=True), + self.user + ) + self.assertFalse(updated_details.self_paced) + + def test_fetch_effort(self): + effort_value = 'test_hours_of_effort' + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.course.id): + CourseDetails.update_about_item(self.course, 'effort', effort_value, self.user.id) + self.assertEqual(CourseDetails.fetch_effort(self.course.id), effort_value) + + def test_fetch_video(self): + video_value = 'test_video_id' + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.course.id): + CourseDetails.update_about_video(self.course, video_value, self.user.id) + self.assertEqual(CourseDetails.fetch_youtube_video_id(self.course.id), video_value) + video_url = CourseDetails.fetch_video_url(self.course.id) + self.assertRegexpMatches(video_url, r'http://.*{}'.format(video_value)) 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), + )