From ac7eceb636aed39769bc59a9bb3ca693958dc1d5 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 14 Jan 2013 11:16:48 -0500 Subject: [PATCH 01/10] be sure to first import 'extra' course content from base folder as well as a special override folder for the course run --- common/lib/xmodule/xmodule/modulestore/xml.py | 54 ++++++++++--------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 47a9ec282e..e01084d231 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -442,33 +442,39 @@ class XMLModuleStore(ModuleStoreBase): log.debug('========> Done with course import from {0}'.format(course_dir)) return course_descriptor - def load_extra_content(self, system, course_descriptor, category, base_dir, course_dir, url_name): - if url_name: - path = base_dir / url_name - if not os.path.exists(path): - path = base_dir + def load_extra_content(self, system, course_descriptor, category, base_dir, course_dir, url_name): + + self._load_extra_content(system, course_descriptor, category, base_dir, course_dir) + + # then look in a override folder based on the course run + if os.path.isdir(base_dir / url_name): + self._load_extra_content(system, course_descriptor, category, base_dir / url_name, course_dir) + + + def _load_extra_content(self, system, course_descriptor, category, path, course_dir): for filepath in glob.glob(path/ '*'): - with open(filepath) as f: - try: - html = f.read().decode('utf-8') - # tabs are referenced in policy.json through a 'slug' which is just the filename without the .html suffix - slug = os.path.splitext(os.path.basename(filepath))[0] - loc = Location('i4x', course_descriptor.location.org, course_descriptor.location.course, category, slug) - module = HtmlDescriptor(system, definition={'data' : html}, **{'location' : loc}) - # VS[compat]: - # Hack because we need to pull in the 'display_name' for static tabs (because we need to edit them) - # from the course policy - if category == "static_tab": - for tab in course_descriptor.tabs or []: - if tab.get('url_slug') == slug: - module.metadata['display_name'] = tab['name'] - module.metadata['data_dir'] = course_dir - self.modules[course_descriptor.id][module.location] = module - except Exception, e: - logging.exception("Failed to load {0}. Skipping... Exception: {1}".format(filepath, str(e))) - system.error_tracker("ERROR: " + str(e)) + if not os.path.isdir(filepath): + with open(filepath) as f: + try: + html = f.read().decode('utf-8') + # tabs are referenced in policy.json through a 'slug' which is just the filename without the .html suffix + slug = os.path.splitext(os.path.basename(filepath))[0] + loc = Location('i4x', course_descriptor.location.org, course_descriptor.location.course, category, slug) + module = HtmlDescriptor(system, definition={'data' : html}, **{'location' : loc}) + # VS[compat]: + # Hack because we need to pull in the 'display_name' for static tabs (because we need to edit them) + # from the course policy + if category == "static_tab": + for tab in course_descriptor.tabs or []: + if tab.get('url_slug') == slug: + module.metadata['display_name'] = tab['name'] + module.metadata['data_dir'] = course_dir + self.modules[course_descriptor.id][module.location] = module + except Exception, e: + logging.exception("Failed to load {0}. Skipping... Exception: {1}".format(filepath, str(e))) + system.error_tracker("ERROR: " + str(e)) def get_instance(self, course_id, location, depth=0): """ From edb913f6ac360f434d962f1ddf7d6fdb7476e5da Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 14 Jan 2013 12:12:40 -0500 Subject: [PATCH 02/10] add tests for about content overrides --- cms/djangoapps/contentstore/tests/tests.py | 7 +++++++ common/test/data/full/about/6.002_Spring_2012/effort.html | 1 + common/test/data/full/about/effort.html | 1 + 3 files changed, 9 insertions(+) create mode 100644 common/test/data/full/about/6.002_Spring_2012/effort.html create mode 100644 common/test/data/full/about/effort.html diff --git a/cms/djangoapps/contentstore/tests/tests.py b/cms/djangoapps/contentstore/tests/tests.py index d0b8261908..5e1c364abd 100644 --- a/cms/djangoapps/contentstore/tests/tests.py +++ b/cms/djangoapps/contentstore/tests/tests.py @@ -350,6 +350,13 @@ class ContentStoreTest(TestCase): def test_edit_unit_full(self): self.check_edit_unit('full') + def test_about_overrides(self): + import_from_xml(modulestore(), 'common/test/data/', ['full']) + ms = modulestore('direct') + effort = ms.get_item(Location(['i4x','edX','full','about','effort', None])) + self.assertEqual(effort.definition['data'],'6 hours') + + def test_clone_course(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) diff --git a/common/test/data/full/about/6.002_Spring_2012/effort.html b/common/test/data/full/about/6.002_Spring_2012/effort.html new file mode 100644 index 0000000000..3ef8bf8ce1 --- /dev/null +++ b/common/test/data/full/about/6.002_Spring_2012/effort.html @@ -0,0 +1 @@ +6 hours \ No newline at end of file diff --git a/common/test/data/full/about/effort.html b/common/test/data/full/about/effort.html new file mode 100644 index 0000000000..c983fdcb5c --- /dev/null +++ b/common/test/data/full/about/effort.html @@ -0,0 +1 @@ +12 hours \ No newline at end of file From b5776a775e4b52d93848069d5d3b07769fab4537 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 14 Jan 2013 12:17:51 -0500 Subject: [PATCH 03/10] add another assert test --- cms/djangoapps/contentstore/tests/tests.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cms/djangoapps/contentstore/tests/tests.py b/cms/djangoapps/contentstore/tests/tests.py index 5e1c364abd..2597ac64fd 100644 --- a/cms/djangoapps/contentstore/tests/tests.py +++ b/cms/djangoapps/contentstore/tests/tests.py @@ -351,11 +351,19 @@ class ContentStoreTest(TestCase): self.check_edit_unit('full') def test_about_overrides(self): + ''' + This test case verifies that a course can use specialized override for about data, e.g. /about/Fall_2012/effort.html + while there is a base definition in /about/effort.html + ''' import_from_xml(modulestore(), 'common/test/data/', ['full']) ms = modulestore('direct') effort = ms.get_item(Location(['i4x','edX','full','about','effort', None])) self.assertEqual(effort.definition['data'],'6 hours') + # this one should be in a non-override folder + effort = ms.get_item(Location(['i4x','edX','full','about','end_date', None])) + self.assertEqual(effort.definition['data'],'TBD') + def test_clone_course(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) From 9f73294959dd6b642e5ac5d2d26f016c489b1aa9 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 14 Jan 2013 12:23:36 -0500 Subject: [PATCH 04/10] add end_date.html file --- common/test/data/full/about/end_date.html | 1 + 1 file changed, 1 insertion(+) create mode 100644 common/test/data/full/about/end_date.html diff --git a/common/test/data/full/about/end_date.html b/common/test/data/full/about/end_date.html new file mode 100644 index 0000000000..2fd9f95700 --- /dev/null +++ b/common/test/data/full/about/end_date.html @@ -0,0 +1 @@ +TBD \ No newline at end of file From 0e11deffcf8279501a70ae3047bfd19fdc12c018 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 14 Jan 2013 12:31:50 -0500 Subject: [PATCH 05/10] Fix staff and peer grading settings to not fail catastrophically --- lms/envs/aws.py | 4 ++-- lms/envs/common.py | 20 ++++++++++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 0516bddc56..98c65e7cb4 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -76,8 +76,8 @@ DATABASES = AUTH_TOKENS['DATABASES'] XQUEUE_INTERFACE = AUTH_TOKENS['XQUEUE_INTERFACE'] -STAFF_GRADING_INTERFACE = AUTH_TOKENS.get('STAFF_GRADING_INTERFACE') - +STAFF_GRADING_INTERFACE = AUTH_TOKENS.get('STAFF_GRADING_INTERFACE', STAFF_GRADING_INTERFACE) +PEER_GRADING_INTERFACE = AUTH_TOKENS.get('STAFF_GRADING_INTERFACE', PEER_GRADING_INTERFACE) PEARSON_TEST_USER = "pearsontest" PEARSON_TEST_PASSWORD = AUTH_TOKENS.get("PEARSON_TEST_PASSWORD") diff --git a/lms/envs/common.py b/lms/envs/common.py index 88cf09502d..0364a8b6f8 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -329,12 +329,28 @@ WIKI_LINK_DEFAULT_LEVEL = 2 ################################# Staff grading config ##################### -STAFF_GRADING_INTERFACE = None +#By setting up the default settings with an incorrect user name and password, +# will get an error when attempting to connect +STAFF_GRADING_INTERFACE = { + 'url': 'http://sandbox-grader-001.m.edx.org/staff_grading', + 'username': 'incorrect_user', + 'password': 'incorrect_pass', + } + # Used for testing, debugging MOCK_STAFF_GRADING = False ################################# Peer grading config ##################### -PEER_GRADING_INTERFACE = None + +#By setting up the default settings with an incorrect user name and password, +# will get an error when attempting to connect +PEER_GRADING_INTERFACE = { + 'url': 'http://sandbox-grader-001.m.edx.org/peer_grading', + 'username': 'incorrect_user', + 'password': 'incorrect_pass', + } + +# Used for testing, debugging MOCK_PEER_GRADING = False ################################# Jasmine ################################### From 1316728aaa851ad6666acfa77c90eaf5092eecd5 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Mon, 14 Jan 2013 12:32:31 -0500 Subject: [PATCH 06/10] Minor naming fix --- lms/envs/aws.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 98c65e7cb4..7b8c48f4af 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -77,7 +77,7 @@ DATABASES = AUTH_TOKENS['DATABASES'] XQUEUE_INTERFACE = AUTH_TOKENS['XQUEUE_INTERFACE'] STAFF_GRADING_INTERFACE = AUTH_TOKENS.get('STAFF_GRADING_INTERFACE', STAFF_GRADING_INTERFACE) -PEER_GRADING_INTERFACE = AUTH_TOKENS.get('STAFF_GRADING_INTERFACE', PEER_GRADING_INTERFACE) +PEER_GRADING_INTERFACE = AUTH_TOKENS.get('PEER_GRADING_INTERFACE', PEER_GRADING_INTERFACE) PEARSON_TEST_USER = "pearsontest" PEARSON_TEST_PASSWORD = AUTH_TOKENS.get("PEARSON_TEST_PASSWORD") From 19f79fa36406038031abf62c1bd2b138aa8ce9d1 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 14 Jan 2013 12:40:17 -0500 Subject: [PATCH 07/10] Prune deleted remote branches --- jenkins/quality.sh | 2 ++ jenkins/test.sh | 2 ++ 2 files changed, 4 insertions(+) diff --git a/jenkins/quality.sh b/jenkins/quality.sh index 4cf26d76bf..56217af874 100755 --- a/jenkins/quality.sh +++ b/jenkins/quality.sh @@ -3,6 +3,8 @@ set -e set -x +git remote prune origin + # Reset the submodule, in case it changed git submodule foreach 'git reset --hard HEAD' diff --git a/jenkins/test.sh b/jenkins/test.sh index 8a96024785..7a61e914b7 100755 --- a/jenkins/test.sh +++ b/jenkins/test.sh @@ -15,6 +15,8 @@ function github_mark_failed_on_exit { trap '[ $? == "0" ] || github_status state:failure "failed"' EXIT } +git remote prune origin + github_mark_failed_on_exit github_status state:pending "is running" From 86be73f6a237eed01324a39fcc7b866063fcea1e Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 14 Jan 2013 11:33:59 -0500 Subject: [PATCH 08/10] Make errors during course loading not break the site, but just hide that course instead --- common/lib/xmodule/xmodule/modulestore/xml.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index b7f0c726e4..3e4cd4e899 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -13,6 +13,7 @@ from importlib import import_module from lxml import etree from path import path +from xmodule.error_module import ErrorDescriptor from xmodule.errortracker import make_error_tracker, exc_info_to_str from xmodule.course_module import CourseDescriptor from xmodule.mako_module import MakoDescriptorSystem @@ -167,8 +168,6 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): # Didn't load properly. Fall back on loading as an error # descriptor. This should never error due to formatting. - # Put import here to avoid circular import errors - from xmodule.error_module import ErrorDescriptor msg = "Error loading from xml. " + str(err)[:200] log.warning(msg) @@ -311,7 +310,7 @@ class XMLModuleStore(ModuleStoreBase): log.exception(msg) errorlog.tracker(msg) - if course_descriptor is not None: + if course_descriptor is not None and not isinstance(course_descriptor, ErrorDescriptor): self.courses[course_dir] = course_descriptor self._location_errors[course_descriptor.location] = errorlog self.parent_trackers[course_descriptor.id].make_known(course_descriptor.location) @@ -423,6 +422,10 @@ class XMLModuleStore(ModuleStoreBase): course_descriptor = system.process_xml(etree.tostring(course_data, encoding='unicode')) + # If we fail to load the course, then skip the rest of the loading steps + if isinstance(course_descriptor, ErrorDescriptor): + return course_descriptor + # NOTE: The descriptors end up loading somewhat bottom up, which # breaks metadata inheritance via get_children(). Instead # (actually, in addition to, for now), we do a final inheritance pass From 95478922d06d87baf25979b35ef3c5b930e60124 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 14 Jan 2013 11:34:06 -0500 Subject: [PATCH 09/10] Whitespace fixes --- common/lib/xmodule/xmodule/modulestore/xml.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 3e4cd4e899..17d6f04932 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -474,11 +474,11 @@ class XMLModuleStore(ModuleStoreBase): if category == "static_tab": for tab in course_descriptor.tabs or []: if tab.get('url_slug') == slug: - module.metadata['display_name'] = tab['name'] + module.metadata['display_name'] = tab['name'] module.metadata['data_dir'] = course_dir - self.modules[course_descriptor.id][module.location] = module + self.modules[course_descriptor.id][module.location] = module except Exception, e: - logging.exception("Failed to load {0}. Skipping... Exception: {1}".format(filepath, str(e))) + logging.exception("Failed to load {0}. Skipping... Exception: {1}".format(filepath, str(e))) system.error_tracker("ERROR: " + str(e)) def get_instance(self, course_id, location, depth=0): From 330a2eac80d2f8e235acb8420d3c2a60b132f8ae Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 14 Jan 2013 11:38:34 -0500 Subject: [PATCH 10/10] Don't break the course if the grading policy is broken --- common/lib/xmodule/xmodule/course_module.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 8b4db799cb..0c854a8036 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -228,7 +228,11 @@ class CourseDescriptor(SequenceDescriptor): if policy_dir: paths = [policy_dir + '/grading_policy.json'] + paths - policy = json.loads(cls.read_grading_policy(paths, system)) + try: + policy = json.loads(cls.read_grading_policy(paths, system)) + except ValueError: + system.error_tracker("Unable to decode grading policy as json") + policy = None # cdodge: import the grading policy information that is on disk and put into the # descriptor 'definition' bucket as a dictionary so that it is persisted in the DB