From d6ced072c41485a2eb26165cd65e3078152bb88d Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Thu, 3 Dec 2015 16:29:59 -0500 Subject: [PATCH] Refactor CourseDetails --- .../tests/test_course_settings.py | 421 +++++++----------- .../tests/test_courseware_index.py | 25 +- cms/djangoapps/contentstore/utils.py | 21 - cms/djangoapps/contentstore/views/course.py | 3 +- cms/djangoapps/models/settings/encoder.py | 28 ++ cms/templates/settings_graders.html | 2 +- .../xmodule/modulestore/tests/factories.py | 3 +- openedx/core/djangoapps/models/__init__.py | 0 .../core/djangoapps/models}/course_details.py | 156 ++++--- .../models/tests/test_course_details.py | 127 ++++++ 10 files changed, 435 insertions(+), 351 deletions(-) create mode 100644 cms/djangoapps/models/settings/encoder.py create mode 100644 openedx/core/djangoapps/models/__init__.py rename {cms/djangoapps/models/settings => openedx/core/djangoapps/models}/course_details.py (67%) create mode 100644 openedx/core/djangoapps/models/tests/test_course_details.py diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index ee7a6644cd..238c93fa4f 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): @@ -1068,7 +983,7 @@ class CourseMetadataEditingTest(CourseTestCase): self.assertEqual(tab_list, course.tabs) @override_settings(FEATURES={'CERTIFICATES_HTML_VIEW': True}) - def test_web_view_certifcate_configuration_settings(self): + def test_web_view_certificate_configuration_settings(self): """ Test that has_cert_config is updated based on cert_html_view_enabled setting. """ 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/utils.py b/cms/djangoapps/contentstore/utils.py index f59f66ba6b..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 @@ -304,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 f457e44e38..e40fbe5ea7 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -58,12 +58,13 @@ 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 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/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/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/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 67% rename from cms/djangoapps/models/settings/course_details.py rename to openedx/core/djangoapps/models/course_details.py index b6a6862003..930d44d901 100644 --- a/cms/djangoapps/models/settings/course_details.py +++ b/openedx/core/djangoapps/models/course_details.py @@ -1,20 +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.fields import Date from xmodule.modulestore.exceptions import ItemNotFoundError -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 + # 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. @@ -30,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 @@ -44,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 @@ -73,7 +75,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) @@ -86,33 +89,57 @@ 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 @@ -122,10 +149,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 """ @@ -134,10 +169,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: @@ -202,25 +238,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 @@ -237,26 +276,37 @@ 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): +# TODO - we may no longer need this - check with Zia +def has_active_web_certificate(course): """ - Serialize CourseDetails, CourseGradingModel, datetime, and old Locations + 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 or course has not enabled `cert_html_view_enabled` + settings. """ - 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) + 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'): + return True + return cert_config 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..e82581091a --- /dev/null +++ b/openedx/core/djangoapps/models/tests/test_course_details.py @@ -0,0 +1,127 @@ +""" +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.assertIsNone(details.has_cert_config) + 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))